mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make checkIdenticalDefaultArguments more precise

Browse Source 
...when creating objects involves copy/move constructors

Change-Id: I0c7ccb85b7dcb584502a48817d7d2abfde25aaf2
Reviewed-on: https://gerrit.libreoffice.org/44733
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This commit is contained in:
Stephan Bergmann
2017-11-14 19:17:07 +01:00
parent 4283092eb2
commit cab6e68369
3 changed files with 38 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
compilerplugins/clang/plugin.cxx
View File

@@ -30,7 +30,7 @@ namespace {
Expr const * skipImplicit(Expr const * expr) { Expr const * skipImplicit(Expr const * expr) {
if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) { if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) {
expr = e->GetTemporaryExpr(); expr = e->GetTemporaryExpr()->IgnoreImpCasts();
} }
if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) { if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) {
expr = e->getSubExpr(); expr = e->getSubExpr();
@@ -271,18 +271,26 @@ Plugin::IdenticalDefaultArgumentsResult Plugin::checkIdenticalDefaultArguments(
} }
} }
// catch params with defaults like "= OUString()" // catch params with defaults like "= OUString()"
if (auto const e1 = dyn_cast<CXXConstructExpr>(skipImplicit(desugared1))) { for (Expr const * e1 = desugared1, * e2 = desugared2;;) {
if (auto const e2 = dyn_cast<CXXConstructExpr>( auto const ce1 = dyn_cast<CXXConstructExpr>(skipImplicit(e1));
skipImplicit(desugared2))) auto const ce2 = dyn_cast<CXXConstructExpr>(skipImplicit(e2));
{ if (ce1 == nullptr || ce2 == nullptr) {
if ((e1->getConstructor()->getCanonicalDecl() break;
!= e2->getConstructor()->getCanonicalDecl())) }
if (ce1->getConstructor()->getCanonicalDecl() != ce2->getConstructor()->getCanonicalDecl())
{ {
return IdenticalDefaultArgumentsResult::No; return IdenticalDefaultArgumentsResult::No;
} }
if (e1->getNumArgs() == 0 && e2->getNumArgs() == 0) { if (ce1->isElidable() && ce2->isElidable() && ce1->getNumArgs() == 1
return IdenticalDefaultArgumentsResult::Yes; && 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; return IdenticalDefaultArgumentsResult::Maybe;

7
compilerplugins/clang/test/unnecessaryoverride.cxx
View File

@@ -94,6 +94,7 @@ struct Base2
{ {
void default1(Base const& = SimpleDerived()); void default1(Base const& = SimpleDerived());
void default2(Base const& = SimpleDerived()); void default2(Base const& = SimpleDerived());
void default3(Base = Base());
}; };
struct Derived2 : Base2 struct Derived2 : Base2
@@ -105,6 +106,12 @@ struct Derived2 : Base2
{ {
Base2::default2(x); 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: */ /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

15
compilerplugins/clang/unnecessaryoverride.cxx
View File

@@ -289,10 +289,12 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
if (!compoundStmt || compoundStmt->size() != 1) if (!compoundStmt || compoundStmt->size() != 1)
return true; return true;
const CXXMemberCallExpr* callExpr; const CXXMemberCallExpr* callExpr = nullptr;
if (compat::getReturnType(*methodDecl).getCanonicalType()->isVoidType()) 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 else
{ {
@@ -355,8 +357,13 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
if (!expr2) if (!expr2)
return true; return true;
for (unsigned i = 0; i<callExpr->getNumArgs(); ++i) { for (unsigned i = 0; i<callExpr->getNumArgs(); ++i) {
// ignore ImplicitCastExpr auto e = callExpr->getArg(i)->IgnoreImplicit();
const DeclRefExpr * declRefExpr = dyn_cast<DeclRefExpr>(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)) if (!declRefExpr || declRefExpr->getDecl() != methodDecl->getParamDecl(i))
return true; return true;
} }