loplugin:unnecessaryoverride
Change-Id: I08c55a3023ec2e8990098eeb60e91cd18556e7ae Reviewed-on: https://gerrit.libreoffice.org/29656 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
@@ -39,6 +39,19 @@ public:
|
||||
return;
|
||||
if (fn == SRCDIR "/forms/source/component/Time.cxx")
|
||||
return;
|
||||
if (fn == SRCDIR "/svx/source/dialog/hyperdlg.cxx")
|
||||
return;
|
||||
if (fn == SRCDIR "/svx/source/dialog/rubydialog.cxx")
|
||||
return;
|
||||
if (fn.startswith(SRCDIR "/canvas"))
|
||||
return;
|
||||
if (fn == SRCDIR "/sc/source/ui/view/spelldialog.cxx")
|
||||
return;
|
||||
if (fn == SRCDIR "/sd/source/ui/dlg/SpellDialogChildWindow.cxx")
|
||||
return;
|
||||
// HAVE_ODBC_ADMINISTRATION
|
||||
if (fn == SRCDIR "/dbaccess/source/ui/dlg/dsselect.cxx")
|
||||
return;
|
||||
|
||||
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
|
||||
}
|
||||
@@ -72,6 +85,10 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
|
||||
// entertaining template magic
|
||||
if (aFileName == SRCDIR "/sc/source/ui/vba/vbaformatcondition.cxx")
|
||||
return true;
|
||||
// not sure what is going on here, but removing the override causes a crash
|
||||
if (methodDecl->getQualifiedNameAsString() == "SwXTextDocument::queryAdapter")
|
||||
return true;
|
||||
|
||||
|
||||
const CXXMethodDecl* overriddenMethodDecl = *methodDecl->begin_overridden_methods();
|
||||
|
||||
@@ -80,62 +97,78 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
if (methodDecl->getAccess() == AS_public && overriddenMethodDecl->getAccess() == AS_protected)
|
||||
return true;
|
||||
|
||||
//TODO: check for identical exception specifications
|
||||
|
||||
const CompoundStmt* compoundStmt = dyn_cast<CompoundStmt>(methodDecl->getBody());
|
||||
if (!compoundStmt || compoundStmt->size() != 1)
|
||||
return true;
|
||||
auto returnStmt = dyn_cast<ReturnStmt>(*compoundStmt->body_begin());
|
||||
if (returnStmt == nullptr) {
|
||||
return true;
|
||||
}
|
||||
auto returnExpr = returnStmt->getRetValue();
|
||||
if (returnExpr == nullptr) {
|
||||
return true;
|
||||
}
|
||||
returnExpr = returnExpr->IgnoreImplicit();
|
||||
|
||||
// In something like
|
||||
//
|
||||
// Reference< XResultSet > SAL_CALL OPreparedStatement::executeQuery(
|
||||
// const rtl::OUString& sql)
|
||||
// throw(SQLException, RuntimeException, std::exception)
|
||||
// {
|
||||
// return OCommonStatement::executeQuery( sql );
|
||||
// }
|
||||
//
|
||||
// look down through all the
|
||||
//
|
||||
// ReturnStmt
|
||||
// `-ExprWithCleanups
|
||||
// `-CXXConstructExpr
|
||||
// `-MaterializeTemporaryExpr
|
||||
// `-ImplicitCastExpr
|
||||
// `-CXXBindTemporaryExpr
|
||||
// `-CXXMemberCallExpr
|
||||
//
|
||||
// where the fact that the overriding and overridden function have identical
|
||||
// return types makes us confident that all we need to check here is whether
|
||||
// there's an (arbitrary, one-argument) CXXConstructorExpr and
|
||||
// CXXBindTemporaryExpr in between:
|
||||
if (auto ctorExpr = dyn_cast<CXXConstructExpr>(returnExpr)) {
|
||||
if (ctorExpr->getNumArgs() == 1) {
|
||||
if (auto tempExpr = dyn_cast<CXXBindTemporaryExpr>(
|
||||
ctorExpr->getArg(0)->IgnoreImplicit()))
|
||||
{
|
||||
returnExpr = tempExpr->getSubExpr();
|
||||
const CXXMemberCallExpr* callExpr;
|
||||
if (compat::getReturnType(*methodDecl).getCanonicalType()->isVoidType())
|
||||
{
|
||||
callExpr = dyn_cast<CXXMemberCallExpr>(*compoundStmt->body_begin());
|
||||
}
|
||||
else
|
||||
{
|
||||
auto returnStmt = dyn_cast<ReturnStmt>(*compoundStmt->body_begin());
|
||||
if (returnStmt == nullptr) {
|
||||
return true;
|
||||
}
|
||||
auto returnExpr = returnStmt->getRetValue();
|
||||
if (returnExpr == nullptr) {
|
||||
return true;
|
||||
}
|
||||
returnExpr = returnExpr->IgnoreImplicit();
|
||||
|
||||
// In something like
|
||||
//
|
||||
// Reference< XResultSet > SAL_CALL OPreparedStatement::executeQuery(
|
||||
// const rtl::OUString& sql)
|
||||
// throw(SQLException, RuntimeException, std::exception)
|
||||
// {
|
||||
// return OCommonStatement::executeQuery( sql );
|
||||
// }
|
||||
//
|
||||
// look down through all the
|
||||
//
|
||||
// ReturnStmt
|
||||
// `-ExprWithCleanups
|
||||
// `-CXXConstructExpr
|
||||
// `-MaterializeTemporaryExpr
|
||||
// `-ImplicitCastExpr
|
||||
// `-CXXBindTemporaryExpr
|
||||
// `-CXXMemberCallExpr
|
||||
//
|
||||
// where the fact that the overriding and overridden function have identical
|
||||
// return types makes us confident that all we need to check here is whether
|
||||
// there's an (arbitrary, one-argument) CXXConstructorExpr and
|
||||
// CXXBindTemporaryExpr in between:
|
||||
if (auto ctorExpr = dyn_cast<CXXConstructExpr>(returnExpr)) {
|
||||
if (ctorExpr->getNumArgs() == 1) {
|
||||
auto tempExpr1 = ctorExpr->getArg(0)->IgnoreImplicit();
|
||||
if (auto tempExpr2 = dyn_cast<CXXBindTemporaryExpr>(tempExpr1))
|
||||
{
|
||||
returnExpr = tempExpr2->getSubExpr();
|
||||
}
|
||||
else if (auto tempExpr2 = dyn_cast<CXXMemberCallExpr>(tempExpr1))
|
||||
{
|
||||
returnExpr = tempExpr2;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
callExpr = dyn_cast<CXXMemberCallExpr>(returnExpr->IgnoreParenImpCasts());
|
||||
}
|
||||
|
||||
const CXXMemberCallExpr* callExpr = dyn_cast<CXXMemberCallExpr>(
|
||||
returnExpr->IgnoreParenImpCasts());
|
||||
if (!callExpr || callExpr->getMethodDecl() != overriddenMethodDecl)
|
||||
return true;
|
||||
const ImplicitCastExpr* expr1 = dyn_cast_or_null<ImplicitCastExpr>(callExpr->getImplicitObjectArgument());
|
||||
const Expr* expr1 = callExpr->getImplicitObjectArgument()->IgnoreImpCasts();
|
||||
if (!expr1)
|
||||
return true;
|
||||
const CXXThisExpr* expr2 = dyn_cast_or_null<CXXThisExpr>(expr1->getSubExpr());
|
||||
const CXXThisExpr* expr2 = dyn_cast_or_null<CXXThisExpr>(expr1);
|
||||
if (!expr2)
|
||||
return true;
|
||||
for (unsigned i = 0; i<callExpr->getNumArgs(); ++i) {
|
||||
@@ -146,9 +179,10 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
|
||||
}
|
||||
|
||||
report(
|
||||
DiagnosticsEngine::Warning, "%0 virtual function just calls %1 parent",
|
||||
methodDecl->getSourceRange().getBegin())
|
||||
<< methodDecl->getAccess() << overriddenMethodDecl->getAccess()
|
||||
DiagnosticsEngine::Warning, "%0 virtual function just calls %1 parent",
|
||||
methodDecl->getSourceRange().getBegin())
|
||||
<< methodDecl->getAccess()
|
||||
<< overriddenMethodDecl->getAccess()
|
||||
<< methodDecl->getSourceRange();
|
||||
if (methodDecl->getCanonicalDecl()->getLocation() != methodDecl->getLocation()) {
|
||||
const CXXMethodDecl* pOther = methodDecl->getCanonicalDecl();
|
||||
|
Reference in New Issue
Block a user