Tweak loplugin:implicitboolconversion to allow some more bool -> sal_Bool

...in templated code, to cater for the needs of
<https://gerrit.libreoffice.org/c/core/+/124400> "Prepare for removal of
non-const operator[] from Sequence in testtools".

For one, by defining ImplicitBoolConversion::TraverseInitListExpr, make sure
that Clang versions before and after
<https://github.com/llvm/llvm-project/commit/0a42fe70a566f22599e04a6f1344ca2dc5565e17>
"[AST] Treat semantic form of InitListExpr as implicit code in traversals"
behave the same.  Old versions of Clang would have erroneously reported

Sequence<Sequence<sal_Bool>> s2{ { false } };

(and reported

Sequence<Sequence<sal_Int32>> s4{ { false } };

twice) in compilerplugins/clang/test/implicitboolconversion.cxx when one of the
four combinations of syntactic/semantic visit of the outer/inner InitListExpr
defeated the intended suppression logic in
ImplicitBoolConversion::TraverseCXXStdInitializerListExpr.

And for another, ImplicitBoolConversion::TraverseInitListExpr can subsume the
exising ImplicitBoolConversion::TraverseCXXStdInitializerListExpr.

But for a third, that would still make

Sequence<Wrap2<sal_Bool>> s6{ { false } };

in compilerplugins/clang/test/implicitboolconversion.cxx emit a false warning,
so add a cheesy "TODO" chicken-out special case to
ImplicitBoolConversion::checkCXXConstructExpr for now.

Change-Id: Ib9a1b78a7812feb98c673b75a357af7737168342
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124583
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

2 files changed, 77 insertions, 53 deletions

diff --git a/compilerplugins/clang/implicitboolconversion.cxx b/compilerplugins/clang/implicitboolconversion.cxx
index 14574e3cd420..bc0b74932b71 100644
--- a/compilerplugins/clang/implicitboolconversion.cxx
+++ b/compilerplugins/clang/implicitboolconversion.cxx
@@ -274,7 +274,7 @@ public:
     { return TraverseCompoundAssignOperator(expr); }
 #endif
 
-    bool TraverseCXXStdInitializerListExpr(CXXStdInitializerListExpr * expr);
+    bool TraverseInitListExpr(InitListExpr * expr);
 
     bool TraverseReturnStmt(ReturnStmt * stmt);
 
@@ -626,36 +626,15 @@ bool ImplicitBoolConversion::TraverseCompoundAssignOperator(CompoundAssignOperat
     }
 }
 
-bool ImplicitBoolConversion::TraverseCXXStdInitializerListExpr(
-    CXXStdInitializerListExpr * expr)
-{
-    // Must be some std::initializer_list<T>; check whether T is sal_Bool (i.e.,
-    // unsigned char) [TODO: check for real sal_Bool instead]:
-    auto t = expr->getType();
-    if (auto et = dyn_cast<ElaboratedType>(t)) {
-        t = et->desugar();
-    }
-    auto ts = t->getAs<TemplateSpecializationType>();
-    if (ts == nullptr
-        || !ts->getArg(0).getAsType()->isSpecificBuiltinType(
-            clang::BuiltinType::UChar))
-    {
-        return RecursiveASTVisitor::TraverseCXXStdInitializerListExpr(expr);
-    }
-    // Avoid warnings for code like
-    //
-    //  Sequence<sal_Bool> arBool({true, false, true});
-    //
-    auto e = dyn_cast<InitListExpr>(
-        ignoreParenAndTemporaryMaterialization(expr->getSubExpr()));
-    if (e == nullptr) {
-        return RecursiveASTVisitor::TraverseCXXStdInitializerListExpr(expr);
-    }
+bool ImplicitBoolConversion::TraverseInitListExpr(InitListExpr * expr) {
     nested.push(std::vector<ImplicitCastExpr const *>());
-    bool ret = RecursiveASTVisitor::TraverseCXXStdInitializerListExpr(expr);
+    auto const e = expr->isSemanticForm() ? expr : expr->getSemanticForm();
+    auto const ret = TraverseSynOrSemInitListExpr(e, nullptr);
     assert(!nested.empty());
     for (auto i: nested.top()) {
-        if (std::find(e->begin(), e->end(), i) == e->end()) {
+        if (std::find(e->begin(), e->end(), i) == e->end()
+            || !i->getType()->isSpecificBuiltinType(clang::BuiltinType::UChar))
+        {
             reportWarning(i);
         }
     }
@@ -857,31 +836,38 @@ void ImplicitBoolConversion::checkCXXConstructExpr(
         if (j != expr->arg_end()) {
             TemplateSpecializationType const * t1 = expr->getType()->
                 getAs<TemplateSpecializationType>();
-            SubstTemplateTypeParmType const * t2 = nullptr;
-            CXXConstructorDecl const * d = expr->getConstructor();
-            if (d->getNumParams() == expr->getNumArgs()) { //TODO: better check
-                t2 = getAsSubstTemplateTypeParmType(
-                    d->getParamDecl(j - expr->arg_begin())->getType()
-                    .getNonReferenceType());
-            }
-            if (t1 != nullptr && t2 != nullptr) {
-                TemplateDecl const * td
-                    = t1->getTemplateName().getAsTemplateDecl();
-                if (td != nullptr) {
-                    TemplateParameterList const * ps
-                        = td->getTemplateParameters();
-                    auto i = std::find(
-                        ps->begin(), ps->end(),
-                        t2->getReplacedParameter()->getDecl());
-                    if (i != ps->end()) {
-                        if (ps->size() == t1->getNumArgs()) { //TODO
-                            TemplateArgument const & arg = t1->getArg(
-                                i - ps->begin());
-                            if (arg.getKind() == TemplateArgument::Type
-                                && (loplugin::TypeCheck(arg.getAsType())
-                                    .AnyBoolean()))
-                            {
-                                continue;
+            if (t1 == nullptr) {
+                //TODO:
+                if (i->getType()->isSpecificBuiltinType(clang::BuiltinType::UChar)) {
+                    continue;
+                }
+            } else {
+                SubstTemplateTypeParmType const * t2 = nullptr;
+                CXXConstructorDecl const * d = expr->getConstructor();
+                if (d->getNumParams() == expr->getNumArgs()) { //TODO: better check
+                    t2 = getAsSubstTemplateTypeParmType(
+                        d->getParamDecl(j - expr->arg_begin())->getType()
+                        .getNonReferenceType());
+                }
+                if (t2 != nullptr) {
+                    TemplateDecl const * td
+                        = t1->getTemplateName().getAsTemplateDecl();
+                    if (td != nullptr) {
+                        TemplateParameterList const * ps
+                            = td->getTemplateParameters();
+                        auto k = std::find(
+                            ps->begin(), ps->end(),
+                            t2->getReplacedParameter()->getDecl());
+                        if (k != ps->end()) {
+                            if (ps->size() == t1->getNumArgs()) { //TODO
+                                TemplateArgument const & arg = t1->getArg(
+                                    k - ps->begin());
+                                if (arg.getKind() == TemplateArgument::Type
+                                    && (loplugin::TypeCheck(arg.getAsType())
+                                        .AnyBoolean()))
+                                {
+                                    continue;
+                                }
                             }
                         }
                     }
diff --git a/compilerplugins/clang/test/implicitboolconversion.cxx b/compilerplugins/clang/test/implicitboolconversion.cxx
index 8d669ed79959..0c53a1daf627 100644
--- a/compilerplugins/clang/test/implicitboolconversion.cxx
+++ b/compilerplugins/clang/test/implicitboolconversion.cxx
@@ -10,9 +10,29 @@
 #include <sal/config.h>
 
 #include <atomic>
+#include <initializer_list>
 
 #include <sal/types.h>
 
+template <typename T> struct Sequence
+{
+    Sequence(std::initializer_list<T>);
+};
+
+template <typename T> struct Wrap1
+{
+    T element;
+};
+
+template <typename T> struct Wrap2
+{
+    Wrap2(T const& e)
+        : element(e)
+    {
+    }
+    T element;
+};
+
 bool g();
 
 void f()
@@ -32,6 +52,24 @@ void f()
     bool b2 = true;
     b2 &= g();
     (void)b2;
+    Sequence<sal_Bool> s1{ false };
+    (void)s1;
+    Sequence<Sequence<sal_Bool>> s2{ { false } };
+    (void)s2;
+    // expected-error@+1 {{implicit conversion (IntegralCast) from 'bool' to 'const int' [loplugin:implicitboolconversion]}}
+    Sequence<sal_Int32> s3{ false };
+    (void)s3;
+    // expected-error@+1 {{implicit conversion (IntegralCast) from 'bool' to 'const int' [loplugin:implicitboolconversion]}}
+    Sequence<Sequence<sal_Int32>> s4{ { false } };
+    (void)s4;
+    Wrap1<sal_Bool> w1{ false };
+    (void)w1;
+    Sequence<Wrap1<sal_Bool>> s5{ { false } };
+    (void)s5;
+    Wrap2<sal_Bool> w2{ false };
+    (void)w2;
+    Sequence<Wrap2<sal_Bool>> s6{ { false } };
+    (void)s6;
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */