diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2016-10-22 10:04:52 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2016-10-28 12:56:17 +0000 |
commit | 8a22bc93e0988188a87c0a787a9b32a7f74da84d (patch) | |
tree | 5b0c9bd79ee88be0754687fe552729e8470f5db2 /compilerplugins | |
parent | tdf#89329: use unique_ptr for pImpl in fpicker/ (diff) | |
download | core-8a22bc93e0988188a87c0a787a9b32a7f74da84d.tar.gz core-8a22bc93e0988188a87c0a787a9b32a7f74da84d.zip |
update unnecessaryoverride plugin to find pure forwarding methods
which can be replaced with using declarations.
Is there a more efficient way to code the search? Seems to slow the
build down a little.
Change-Id: I08cda21fa70dce6572e1acc71bf5e6df36bb951f
Reviewed-on: https://gerrit.libreoffice.org/30157
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'compilerplugins')
-rw-r--r-- | compilerplugins/clang/unnecessaryoverride.cxx | 93 |
1 files changed, 86 insertions, 7 deletions
diff --git a/compilerplugins/clang/unnecessaryoverride.cxx b/compilerplugins/clang/unnecessaryoverride.cxx index b031c50b7b87..7136cc632739 100644 --- a/compilerplugins/clang/unnecessaryoverride.cxx +++ b/compilerplugins/clang/unnecessaryoverride.cxx @@ -13,6 +13,7 @@ #include <fstream> #include <set> +#include <clang/AST/CXXInheritance.h> #include "compat.hxx" #include "plugin.hxx" @@ -57,6 +58,15 @@ public: } bool VisitCXXMethodDecl(const CXXMethodDecl *); + +private: + const CXXMethodDecl * findOverriddenOrSimilarMethodInSuperclasses(const CXXMethodDecl *); + bool BaseCheckCallback( + const CXXRecordDecl *BaseDefinition + #if CLANG_VERSION < 30800 + , void * + #endif + ); }; bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) @@ -64,11 +74,18 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) if (ignoreLocation(methodDecl->getCanonicalDecl()) || !methodDecl->doesThisDeclarationHaveABody()) { return true; } - // if we are overriding more than one method, then this is a disambiguating override - if (!methodDecl->isVirtual() || methodDecl->size_overridden_methods() != 1 - || (*methodDecl->begin_overridden_methods())->isPure()) { + if (isa<CXXConstructorDecl>(methodDecl) || isa<CXXDestructorDecl>(methodDecl)) { return true; } + + // if we are overriding more than one method, then this is a disambiguating override + if (methodDecl->isVirtual()) { + if (methodDecl->size_overridden_methods() != 1 + || (*methodDecl->begin_overridden_methods())->isPure()) + { + return true; + } + } if (dyn_cast<CXXDestructorDecl>(methodDecl)) { return true; } @@ -90,15 +107,16 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) return true; - const CXXMethodDecl* overriddenMethodDecl = *methodDecl->begin_overridden_methods(); + const CXXMethodDecl* overriddenMethodDecl = findOverriddenOrSimilarMethodInSuperclasses(methodDecl); + if (!overriddenMethodDecl) { + return true; + } if (compat::getReturnType(*methodDecl).getCanonicalType() != compat::getReturnType(*overriddenMethodDecl).getCanonicalType()) { return true; } - if (methodDecl->getAccess() == AS_public && overriddenMethodDecl->getAccess() == AS_protected) - return true; //TODO: check for identical exception specifications @@ -179,9 +197,10 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) } report( - DiagnosticsEngine::Warning, "%0 virtual function just calls %1 parent", + DiagnosticsEngine::Warning, "%0%1 function just calls %2 parent", methodDecl->getSourceRange().getBegin()) << methodDecl->getAccess() + << (methodDecl->isVirtual() ? " virtual" : "") << overriddenMethodDecl->getAccess() << methodDecl->getSourceRange(); if (methodDecl->getCanonicalDecl()->getLocation() != methodDecl->getLocation()) { @@ -195,6 +214,66 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl) return true; } +const CXXMethodDecl* UnnecessaryOverride::findOverriddenOrSimilarMethodInSuperclasses(const CXXMethodDecl* methodDecl) +{ + if (methodDecl->isVirtual()) { + return *methodDecl->begin_overridden_methods(); + } + if (!methodDecl->getDeclName().isIdentifier()) { + return nullptr; + } + + std::vector<const CXXMethodDecl*> maSimilarMethods; + + auto BaseMatchesCallback = [&](const CXXBaseSpecifier *cxxBaseSpecifier, CXXBasePath& ) + { + if (cxxBaseSpecifier->getAccessSpecifier() != AS_public && cxxBaseSpecifier->getAccessSpecifier() != AS_protected) + return false; + if (!cxxBaseSpecifier->getType().getTypePtr()) + return false; + const CXXRecordDecl* baseCXXRecordDecl = cxxBaseSpecifier->getType()->getAsCXXRecordDecl(); + if (!baseCXXRecordDecl) + return false; + if (baseCXXRecordDecl->isInvalidDecl()) + return false; + for (const CXXMethodDecl* baseMethod : baseCXXRecordDecl->methods()) + { + if (!baseMethod->getDeclName().isIdentifier() || methodDecl->getName() != baseMethod->getName()) { + continue; + } + if (compat::getReturnType(*methodDecl).getCanonicalType() + != compat::getReturnType(*baseMethod).getCanonicalType()) + { + continue; + } + if (methodDecl->param_size() != baseMethod->param_size()) + continue; + if (methodDecl->getNumParams() != baseMethod->getNumParams()) + continue; + bool bParamsMatch = true; + for (unsigned i=0; i<methodDecl->param_size(); ++i) + { + if (methodDecl->parameters()[i]->getType() != baseMethod->parameters()[i]->getType()) + { + bParamsMatch = false; + break; + } + } + if (bParamsMatch) + maSimilarMethods.push_back(baseMethod); + } + return false; + }; + + CXXBasePaths aPaths; + methodDecl->getParent()->lookupInBases(BaseMatchesCallback, aPaths); + + if (maSimilarMethods.size() == 1) { + return maSimilarMethods[0]; + } + return nullptr; +} + loplugin::Plugin::Registration< UnnecessaryOverride > X("unnecessaryoverride", true); |