From 6f26a7246456d8989e83d658a211b5d2608568f5 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Tue, 2 Nov 2021 08:41:42 +0100 Subject: Tweak loplugin:implicitboolconversion to allow some more bool -> sal_Bool ...in templated code, to cater for the needs of "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 "[AST] Treat semantic form of InitListExpr as implicit code in traversals" behave the same. Old versions of Clang would have erroneously reported Sequence> s2{ { false } }; (and reported Sequence> 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> 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 --- compilerplugins/clang/implicitboolconversion.cxx | 92 +++++++++------------- .../clang/test/implicitboolconversion.cxx | 38 +++++++++ 2 files changed, 77 insertions(+), 53 deletions(-) (limited to 'compilerplugins') 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; 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(t)) { - t = et->desugar(); - } - auto ts = t->getAs(); - if (ts == nullptr - || !ts->getArg(0).getAsType()->isSpecificBuiltinType( - clang::BuiltinType::UChar)) - { - return RecursiveASTVisitor::TraverseCXXStdInitializerListExpr(expr); - } - // Avoid warnings for code like - // - // Sequence arBool({true, false, true}); - // - auto e = dyn_cast( - ignoreParenAndTemporaryMaterialization(expr->getSubExpr())); - if (e == nullptr) { - return RecursiveASTVisitor::TraverseCXXStdInitializerListExpr(expr); - } +bool ImplicitBoolConversion::TraverseInitListExpr(InitListExpr * expr) { nested.push(std::vector()); - 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(); - 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 #include +#include #include +template struct Sequence +{ + Sequence(std::initializer_list); +}; + +template struct Wrap1 +{ + T element; +}; + +template 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 s1{ false }; + (void)s1; + Sequence> s2{ { false } }; + (void)s2; + // expected-error@+1 {{implicit conversion (IntegralCast) from 'bool' to 'const int' [loplugin:implicitboolconversion]}} + Sequence s3{ false }; + (void)s3; + // expected-error@+1 {{implicit conversion (IntegralCast) from 'bool' to 'const int' [loplugin:implicitboolconversion]}} + Sequence> s4{ { false } }; + (void)s4; + Wrap1 w1{ false }; + (void)w1; + Sequence> s5{ { false } }; + (void)s5; + Wrap2 w2{ false }; + (void)w2; + Sequence> s6{ { false } }; + (void)s6; } /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ -- cgit