implicitboolconversion: warn about implicit conversion of call args to bool

...to be able to find problems like 6e0bdf04add338b7d5b29fc7b3fc9f08cfd5e96f
"sal_Bool arg of SetUseImagesInMenus was abused to squeeze '2' through it"
earlier when converting occurrences of sal_Bool to bool.

Restricting this check to function call arguments avoids too much noise while
hopefully still catching all the relevant problems.

(This check partially overlaps the pointertobool check, so implicit conversions
from pointers to bool call arguments will now generate two loplugin warnings,
but that's harmless.)

Change-Id: I0b03b1d1615aaf8bc18e7a84c56fff3ef9903508

1 files changed, 41 insertions, 11 deletions

diff --git a/compilerplugins/clang/implicitboolconversion.cxx b/compilerplugins/clang/implicitboolconversion.cxx
index 0327f9fbbef0..6503ca8d2860 100644
--- a/compilerplugins/clang/implicitboolconversion.cxx
+++ b/compilerplugins/clang/implicitboolconversion.cxx
@@ -26,6 +26,14 @@ template<> struct std::iterator_traits<ExprIterator> {
     typedef std::random_access_iterator_tag iterator_category;
 };
 
+template<> struct std::iterator_traits<ConstExprIterator> {
+    typedef std::ptrdiff_t difference_type;
+    typedef Expr const * value_type;
+    typedef Expr const ** pointer;
+    typedef Expr const & reference;
+    typedef std::random_access_iterator_tag iterator_category;
+};
+
 namespace {
 
 bool isBool(Expr const * expr, bool allowTypedefs = true) {
@@ -147,11 +155,13 @@ private:
     void reportWarning(ImplicitCastExpr const * expr);
 
     std::stack<std::vector<ImplicitCastExpr const *>> nested;
+    std::stack<CallExpr const *> calls;
     bool externCIntFunctionDefinition = false;
 };
 
 bool ImplicitBoolConversion::TraverseCallExpr(CallExpr * expr) {
     nested.push(std::vector<ImplicitCastExpr const *>());
+    calls.push(expr);
     bool ret = RecursiveASTVisitor::TraverseCallExpr(expr);
     Decl const * d = expr->getCalleeDecl();
     bool ext = false;
@@ -203,6 +213,7 @@ bool ImplicitBoolConversion::TraverseCallExpr(CallExpr * expr) {
             reportWarning(i);
         }
     }
+    calls.pop();
     nested.pop();
     return ret;
 }
@@ -540,21 +551,40 @@ bool ImplicitBoolConversion::VisitImplicitCastExpr(
         } else {
             nested.top().push_back(expr);
         }
-    } else {
-        ExplicitCastExpr const * sub = dyn_cast<ExplicitCastExpr>(
-            expr->getSubExpr()->IgnoreParenImpCasts());
-        if (sub != nullptr
-            && (sub->getSubExpr()->IgnoreParenImpCasts()->getType().IgnoreParens()
-                == expr->getType().IgnoreParens())
-            && isBool(sub->getSubExpr()->IgnoreParenImpCasts()))
+        return true;
+    }
+    ExplicitCastExpr const * sub = dyn_cast<ExplicitCastExpr>(
+        expr->getSubExpr()->IgnoreParenImpCasts());
+    if (sub != nullptr
+        && (sub->getSubExpr()->IgnoreParenImpCasts()->getType().IgnoreParens()
+            == expr->getType().IgnoreParens())
+        && isBool(sub->getSubExpr()->IgnoreParenImpCasts()))
+    {
+        report(
+            DiagnosticsEngine::Warning,
+            "explicit conversion (%0) from %1 to %2 implicitly cast back to %3",
+            expr->getLocStart())
+            << sub->getCastKindName()
+            << sub->getSubExpr()->IgnoreParenImpCasts()->getType()
+            << sub->getType() << expr->getType() << expr->getSourceRange();
+        return true;
+    }
+    if (expr->getType()->isBooleanType() && !isBool(expr->getSubExpr())
+        && !calls.empty())
+    {
+        CallExpr const * call = calls.top();
+        if (std::find_if(
+                call->arg_begin(), call->arg_end(),
+                [expr](Expr const * e) { return expr == e->IgnoreParens(); })
+            != call->arg_end())
         {
             report(
                 DiagnosticsEngine::Warning,
-                "explicit conversion (%0) from %1 to %2 implicitly cast back to %3",
+                "implicit conversion (%0) of call argument from %1 to %2",
                 expr->getLocStart())
-                << sub->getCastKindName()
-                << sub->getSubExpr()->IgnoreParenImpCasts()->getType()
-                << sub->getType() << expr->getType() << expr->getSourceRange();
+                << expr->getCastKindName() << expr->getSubExpr()->getType()
+                << expr->getType() << expr->getSourceRange();
+            return true;
         }
     }
     return true;