implicitboolconversion: also warn about mixing bool/sal_Bool in == etc.

...as MSVC would warn about those anyway (at least about cases that do not
compare against literal sal_True/sal_False, but warning even about those helped
clean up lots of redundant

if (foo == true)

instead of just

if (foo)

etc. across the code base).

Change-Id: Iad4b335c35c5411070f04f87f974db7942c288d4

1 files changed, 16 insertions, 13 deletions

diff --git a/compilerplugins/clang/implicitboolconversion.cxx b/compilerplugins/clang/implicitboolconversion.cxx
index bb5062668e6f..0c45bf0372e0 100644
--- a/compilerplugins/clang/implicitboolconversion.cxx
+++ b/compilerplugins/clang/implicitboolconversion.cxx
@@ -27,11 +27,14 @@ template<> struct std::iterator_traits<ExprIterator> {
 
 namespace {
 
-bool isBool(Expr const * expr) {
+bool isBool(Expr const * expr, bool allowTypedefs = true) {
     QualType t1 { expr->getType() };
     if (t1->isBooleanType()) {
         return true;
     }
+    if (!allowTypedefs) {
+        return false;
+    }
 // css::uno::Sequence<sal_Bool> s(1);s[0]=false /*unotools/source/config/configitem.cxx*/:
 if(t1->isSpecificBuiltinType(BuiltinType::UChar))return true;
     TypedefType const * t2 = t1->getAs<TypedefType>();
@@ -265,9 +268,9 @@ bool ImplicitBoolConversion::TraverseBinLT(BinaryOperator * expr) {
     assert(!nested.empty());
     for (auto i: nested.top()) {
         if (!((i == expr->getLHS()->IgnoreParens()
-               && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+               && isBool(expr->getRHS()->IgnoreParenImpCasts(), false))
               || (i == expr->getRHS()->IgnoreParens()
-                  && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+                  && isBool(expr->getLHS()->IgnoreParenImpCasts(), false))))
         {
             reportWarning(i);
         }
@@ -282,9 +285,9 @@ bool ImplicitBoolConversion::TraverseBinLE(BinaryOperator * expr) {
     assert(!nested.empty());
     for (auto i: nested.top()) {
         if (!((i == expr->getLHS()->IgnoreParens()
-               && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+               && isBool(expr->getRHS()->IgnoreParenImpCasts(), false))
               || (i == expr->getRHS()->IgnoreParens()
-                  && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+                  && isBool(expr->getLHS()->IgnoreParenImpCasts(), false))))
         {
             reportWarning(i);
         }
@@ -299,9 +302,9 @@ bool ImplicitBoolConversion::TraverseBinGT(BinaryOperator * expr) {
     assert(!nested.empty());
     for (auto i: nested.top()) {
         if (!((i == expr->getLHS()->IgnoreParens()
-               && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+               && isBool(expr->getRHS()->IgnoreParenImpCasts(), false))
               || (i == expr->getRHS()->IgnoreParens()
-                  && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+                  && isBool(expr->getLHS()->IgnoreParenImpCasts(), false))))
         {
             reportWarning(i);
         }
@@ -316,9 +319,9 @@ bool ImplicitBoolConversion::TraverseBinGE(BinaryOperator * expr) {
     assert(!nested.empty());
     for (auto i: nested.top()) {
         if (!((i == expr->getLHS()->IgnoreParens()
-               && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+               && isBool(expr->getRHS()->IgnoreParenImpCasts(), false))
               || (i == expr->getRHS()->IgnoreParens()
-                  && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+                  && isBool(expr->getLHS()->IgnoreParenImpCasts(), false))))
         {
             reportWarning(i);
         }
@@ -333,9 +336,9 @@ bool ImplicitBoolConversion::TraverseBinEQ(BinaryOperator * expr) {
     assert(!nested.empty());
     for (auto i: nested.top()) {
         if (!((i == expr->getLHS()->IgnoreParens()
-               && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+               && isBool(expr->getRHS()->IgnoreParenImpCasts(), false))
               || (i == expr->getRHS()->IgnoreParens()
-                  && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+                  && isBool(expr->getLHS()->IgnoreParenImpCasts(), false))))
         {
             reportWarning(i);
         }
@@ -350,9 +353,9 @@ bool ImplicitBoolConversion::TraverseBinNE(BinaryOperator * expr) {
     assert(!nested.empty());
     for (auto i: nested.top()) {
         if (!((i == expr->getLHS()->IgnoreParens()
-               && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+               && isBool(expr->getRHS()->IgnoreParenImpCasts(), false))
               || (i == expr->getRHS()->IgnoreParens()
-                  && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+                  && isBool(expr->getLHS()->IgnoreParenImpCasts(), false))))
         {
             reportWarning(i);
         }