compilerplugins/clang/plugin.cxx | 32 +++++++++++++-------- compilerplugins/clang/test/unnecessaryoverride.cxx | 7 ++++ compilerplugins/clang/unnecessaryoverride.cxx | 15 +++++++-- 3 files changed, 38 insertions(+), 16 deletions(-)
New commits: commit cab6e6836973a9ddfc5ed9df757e07138328c1c3 Author: Stephan Bergmann <sberg...@redhat.com> Date: Tue Nov 14 19:17:07 2017 +0100 Make checkIdenticalDefaultArguments more precise ...when creating objects involves copy/move constructors Change-Id: I0c7ccb85b7dcb584502a48817d7d2abfde25aaf2 Reviewed-on: https://gerrit.libreoffice.org/44733 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Stephan Bergmann <sberg...@redhat.com> diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx index 717d88b23091..80fff7651ae0 100644 --- a/compilerplugins/clang/plugin.cxx +++ b/compilerplugins/clang/plugin.cxx @@ -30,7 +30,7 @@ namespace { Expr const * skipImplicit(Expr const * expr) { if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) { - expr = e->GetTemporaryExpr(); + expr = e->GetTemporaryExpr()->IgnoreImpCasts(); } if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) { expr = e->getSubExpr(); @@ -271,18 +271,26 @@ Plugin::IdenticalDefaultArgumentsResult Plugin::checkIdenticalDefaultArguments( } } // catch params with defaults like "= OUString()" - if (auto const e1 = dyn_cast<CXXConstructExpr>(skipImplicit(desugared1))) { - if (auto const e2 = dyn_cast<CXXConstructExpr>( - skipImplicit(desugared2))) + for (Expr const * e1 = desugared1, * e2 = desugared2;;) { + auto const ce1 = dyn_cast<CXXConstructExpr>(skipImplicit(e1)); + auto const ce2 = dyn_cast<CXXConstructExpr>(skipImplicit(e2)); + if (ce1 == nullptr || ce2 == nullptr) { + break; + } + if (ce1->getConstructor()->getCanonicalDecl() != ce2->getConstructor()->getCanonicalDecl()) { - if ((e1->getConstructor()->getCanonicalDecl() - != e2->getConstructor()->getCanonicalDecl())) - { - return IdenticalDefaultArgumentsResult::No; - } - if (e1->getNumArgs() == 0 && e2->getNumArgs() == 0) { - return IdenticalDefaultArgumentsResult::Yes; - } + return IdenticalDefaultArgumentsResult::No; + } + if (ce1->isElidable() && ce2->isElidable() && ce1->getNumArgs() == 1 + && ce2->getNumArgs() == 1) + { + assert(ce1->getConstructor()->isCopyOrMoveConstructor()); + e1 = ce1->getArg(0)->IgnoreImpCasts(); + e2 = ce2->getArg(0)->IgnoreImpCasts(); + continue; + } + if (ce1->getNumArgs() == 0 && ce2->getNumArgs() == 0) { + return IdenticalDefaultArgumentsResult::Yes; } } return IdenticalDefaultArgumentsResult::Maybe; diff --git a/compilerplugins/clang/test/unnecessaryoverride.cxx b/compilerplugins/clang/test/unnecessaryoverride.cxx index 7abcf971986f..1f5c1f9fb4a7 100644 --- a/compilerplugins/clang/test/unnecessaryoverride.cxx +++ b/compilerplugins/clang/test/unnecessaryoverride.cxx @@ -94,6 +94,7 @@ struct Base2 { void default1(Base const& = SimpleDerived()); void default2(Base const& = SimpleDerived()); + void default3(Base = Base()); }; struct Derived2 : Base2 @@ -105,6 +106,12 @@ struct Derived2 : Base2 { Base2::default2(x); } + void + default3( // expected-error {{public function just calls public parent [loplugin:unnecessaryoverride]}} + Base x = Base()) + { + (Base2::default3(x)); + } }; /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/unnecessaryoverride.cxx b/compilerplugins/clang/unnecessaryoverride.cxx index 13ed723385d8..336c7712a95f 100644 --- a/compilerplugins/clang/unnecessaryoverride.cxx +++ b/compilerplugins/clang/unnecessaryoverride.cxx @@ -289,10 +289,12 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) if (!compoundStmt || compoundStmt->size() != 1) return true; - const CXXMemberCallExpr* callExpr; + const CXXMemberCallExpr* callExpr = nullptr; if (compat::getReturnType(*methodDecl).getCanonicalType()->isVoidType()) { - callExpr = dyn_cast<CXXMemberCallExpr>(*compoundStmt->body_begin()); + if (auto const e = dyn_cast<Expr>(*compoundStmt->body_begin())) { + callExpr = dyn_cast<CXXMemberCallExpr>(e->IgnoreImplicit()->IgnoreParens()); + } } else { @@ -355,8 +357,13 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) if (!expr2) return true; for (unsigned i = 0; i<callExpr->getNumArgs(); ++i) { - // ignore ImplicitCastExpr - const DeclRefExpr * declRefExpr = dyn_cast<DeclRefExpr>(callExpr->getArg(i)->IgnoreImplicit()); + auto e = callExpr->getArg(i)->IgnoreImplicit(); + if (auto const e1 = dyn_cast<CXXConstructExpr>(e)) { + if (e1->getConstructor()->isCopyOrMoveConstructor() && e1->getNumArgs() == 1) { + e = e1->getArg(0)->IgnoreImpCasts(); + } + } + const DeclRefExpr * declRefExpr = dyn_cast<DeclRefExpr>(e); if (!declRefExpr || declRefExpr->getDecl() != methodDecl->getParamDecl(i)) return true; } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits