compilerplugins/clang/implicitboolconversion.cxx | 92 +++++++-----------
compilerplugins/clang/test/implicitboolconversion.cxx | 38 +++++++
2 files changed, 77 insertions(+), 53 deletions(-)
New commits:
commit 6f26a7246456d8989e83d658a211b5d2608568f5
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Tue Nov 2 08:41:42 2021 +0100
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Tue Nov 2 20:10:46 2021 +0100
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 <sberg...@redhat.com>
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: */
