diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2019-02-18 11:58:39 +0100 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2019-02-18 14:52:43 +0100 |
commit | 6ba818779d3eeca144ae3278788fa51ccd7d1298 (patch) | |
tree | 0969dc736f82cc0c0c6379c237414e07d5bec10f | |
parent | dbc7fc7587008b376f28605264f45f967680e4ff (diff) |
Make findOperator more precise wrt acceptable parameter types
Change-Id: I0a1ea253d999c9444344188d764134e83ae0c495
Reviewed-on: https://gerrit.libreoffice.org/67959
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
-rw-r--r-- | compilerplugins/clang/simplifybool.cxx | 46 |
1 files changed, 38 insertions, 8 deletions
diff --git a/compilerplugins/clang/simplifybool.cxx b/compilerplugins/clang/simplifybool.cxx index 4f7e418f0655..895e3eb7b464 100644 --- a/compilerplugins/clang/simplifybool.cxx +++ b/compilerplugins/clang/simplifybool.cxx @@ -53,7 +53,12 @@ Expr const * getSubExprOfLogicalNegation(Expr const * expr) { ? nullptr : e->getSubExpr(); } -FunctionDecl const * findOperator(CompilerInstance& compiler, clang::RecordType const * recordType, BinaryOperator::Opcode opcode) { +// Search for an operator with matching parameter types; while this may miss some operators with +// odd parameter types that would actually be used by the compiler, it is overall better to have too +// many false negatives (i.e., miss valid loplugin:simplifybool warnings) than false positives here: +FunctionDecl const * findOperator(CompilerInstance& compiler, clang::RecordType const * recordType, BinaryOperator::Opcode opcode, QualType lhs, QualType rhs) { + auto const clhs = lhs.isNull() ? nullptr : lhs.getCanonicalType().getTypePtr(); + auto const crhs = rhs.getCanonicalType().getTypePtr(); OverloadedOperatorKind over = BinaryOperator::getOverloadedOperator(opcode); CXXRecordDecl const * recordDecl = dyn_cast<CXXRecordDecl>(recordType->getDecl()); if (!recordDecl) @@ -61,7 +66,9 @@ FunctionDecl const * findOperator(CompilerInstance& compiler, clang::RecordType // search for member overloads for (auto it = recordDecl->method_begin(); it != recordDecl->method_end(); ++it) { if (it->getOverloadedOperator() == over) { - return *it; + assert(it->getNumParams() == 1); + if (it->getParamDecl(0)->getType().getCanonicalType().getTypePtr() == crhs) + return *it; } } // search for free function overloads @@ -75,12 +82,22 @@ FunctionDecl const * findOperator(CompilerInstance& compiler, clang::RecordType FunctionDecl const * f = dyn_cast<FunctionDecl>(*d); if (!f || f->getNumParams() != 2) continue; - auto qt = f->getParamDecl(0)->getType(); - auto lvalue = dyn_cast<LValueReferenceType>(qt.getTypePtr()); - if (!lvalue) + if (f->getParamDecl(1)->getType().getCanonicalType().getTypePtr() != crhs) continue; - if (lvalue->getPointeeType().getTypePtr() == recordType) - return f; + auto const p0 = f->getParamDecl(0)->getType().getCanonicalType().getTypePtr(); + if (clhs) { + if (p0 != clhs) + continue; + } else { + if (p0 != recordType) { + auto lvalue = dyn_cast<LValueReferenceType>(p0); + if (!lvalue) + continue; + if (lvalue->getPointeeType().getTypePtr() != recordType) + continue; + } + } + return f; } return nullptr; } @@ -236,7 +253,20 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) { if (!t->isRecordType()) return true; auto recordType = dyn_cast<RecordType>(t); - auto const negOp = findOperator(compiler, recordType, negatedOpcode); + auto const fdecl = binaryOp->getDirectCallee(); + if (!fdecl) // e.g. CXXOperatorCallExpr with UnresolvedLookupExpr + return true; + QualType lhs; + QualType rhs; + if (auto const mdecl = dyn_cast<CXXMethodDecl>(fdecl)) { + assert(fdecl->getNumParams() == 1); + rhs = fdecl->getParamDecl(0)->getType(); + } else { + assert(fdecl->getNumParams() == 2); + lhs = fdecl->getParamDecl(0)->getType(); + rhs = fdecl->getParamDecl(1)->getType(); + } + auto const negOp = findOperator(compiler, recordType, negatedOpcode, lhs, rhs); if (!negOp) return true; // if we are inside a similar operator, ignore, eg. operator!= is often defined by calling !operator== |