From 281fa6ac6a7df48dd65f5019d5a74148a096e253 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Mon, 17 Oct 2016 15:34:04 +0200 Subject: some cleanups to the unusedmethods loplugin - publicDefinitionSet is unnecessary, the python post-processor does not need it - remove the traversing of templates, clang will do that for us since we have set the shouldVisitTemplateInstantiations() method Change-Id: I0e96dad0b1cc941fe6c4a2e9227e86d8c3f1d85a --- compilerplugins/clang/inlineablemethods.cxx | 2 +- compilerplugins/clang/unusedmethods.cxx | 81 ++++++++++++++--------------- compilerplugins/clang/unusedmethods.py | 41 +++------------ 3 files changed, 47 insertions(+), 77 deletions(-) (limited to 'compilerplugins') diff --git a/compilerplugins/clang/inlineablemethods.cxx b/compilerplugins/clang/inlineablemethods.cxx index 3dd58d352c45..df98463a3547 100644 --- a/compilerplugins/clang/inlineablemethods.cxx +++ b/compilerplugins/clang/inlineablemethods.cxx @@ -325,7 +325,7 @@ bool InlineableMethods::isCalleeFunctionInteresting(const FunctionDecl* function return true; } -loplugin::Plugin::Registration< InlineableMethods > X("inlineablemethods", true); +loplugin::Plugin::Registration< InlineableMethods > X("inlineablemethods", false); } diff --git a/compilerplugins/clang/unusedmethods.cxx b/compilerplugins/clang/unusedmethods.cxx index 3c5498d79651..80af02484a32 100644 --- a/compilerplugins/clang/unusedmethods.cxx +++ b/compilerplugins/clang/unusedmethods.cxx @@ -12,6 +12,8 @@ #include #include #include +#include + #include "clang/AST/Attr.h" @@ -42,8 +44,6 @@ to auto-remove the method declarations Note that the actual process may involve a fair amount of undoing, hand editing, and general messing around to get it to work :-) -TODO deal with calls to superclass/member constructors from other constructors, so - we can find unused constructors */ namespace { @@ -69,8 +69,7 @@ static std::set callSet; static std::set definitionSet; // for the "unused return type" analysis static std::set usedReturnSet; -// for the "unnecessary public" analysis -static std::set publicDefinitionSet; +// for the "can be private" analysis static std::set calledFromOutsideSet; @@ -96,7 +95,7 @@ public: // for the "unused return type" analysis for (const MyFuncInfo & s : usedReturnSet) output += "usedReturn:\t" + s.returnType + "\t" + s.nameAndParams + "\n"; - // for the "unnecessary public" analysis + // for the "method can be private" analysis for (const MyFuncInfo & s : calledFromOutsideSet) output += "outside:\t" + s.returnType + "\t" + s.nameAndParams + "\n"; ofstream myfile; @@ -116,6 +115,8 @@ private: void logCallToRootMethods(const FunctionDecl* functionDecl, std::set& funcSet); MyFuncInfo niceName(const FunctionDecl* functionDecl); std::string fullyQualifiedName(const FunctionDecl* functionDecl); + std::string toString(SourceLocation loc); + void functionTouchedFromExpr( const FunctionDecl* calleeFunctionDecl, const Expr* expr ); }; MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl) @@ -163,14 +164,20 @@ MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl) aInfo.nameAndParams += " const"; } - SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( functionDecl->getLocation() ); - StringRef name = compiler.getSourceManager().getFilename(expansionLoc); - aInfo.sourceLocation = std::string(name.substr(strlen(SRCDIR)+1)) + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc)); - normalizeDotDotInFilePath(aInfo.sourceLocation); + aInfo.sourceLocation = toString( functionDecl->getLocation() ); return aInfo; } +std::string UnusedMethods::toString(SourceLocation loc) +{ + SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc ); + StringRef name = compiler.getSourceManager().getFilename(expansionLoc); + std::string sourceLocation = std::string(name.substr(strlen(SRCDIR)+1)) + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc)); + normalizeDotDotInFilePath(sourceLocation); + return sourceLocation; +} + std::string UnusedMethods::fullyQualifiedName(const FunctionDecl* functionDecl) { std::string ret = compat::getReturnType(*functionDecl).getCanonicalType().getAsString(); @@ -197,13 +204,13 @@ std::string UnusedMethods::fullyQualifiedName(const FunctionDecl* functionDecl) return ret; } +// For virtual/overriding methods, we need to pretend we called the root method(s), +// so that they get marked as used. void UnusedMethods::logCallToRootMethods(const FunctionDecl* functionDecl, std::set& funcSet) { functionDecl = functionDecl->getCanonicalDecl(); bool bCalledSuperMethod = false; if (isa(functionDecl)) { - // For virtual/overriding methods, we need to pretend we called the root method(s), - // so that they get marked as used. const CXXMethodDecl* methodDecl = dyn_cast(functionDecl); for(CXXMethodDecl::method_iterator it = methodDecl->begin_overridden_methods(); it != methodDecl->end_overridden_methods(); ++it) @@ -222,9 +229,6 @@ void UnusedMethods::logCallToRootMethods(const FunctionDecl* functionDecl, std:: } } -// prevent recursive templates from blowing up the stack -static std::set traversedFunctionSet; - bool UnusedMethods::VisitCallExpr(CallExpr* expr) { // Note that I don't ignore ANYTHING here, because I want to get calls to my code that result @@ -247,18 +251,11 @@ bool UnusedMethods::VisitCallExpr(CallExpr* expr) } gotfunc: - // if we see a call to a function, it may effectively create new code, - // if the function is templated. However, if we are inside a template function, - // calling another function on the same template, the same problem occurs. - // Rather than tracking all of that, just traverse anything we have not already traversed. - if (traversedFunctionSet.insert(fullyQualifiedName(calleeFunctionDecl)).second) - TraverseFunctionDecl(calleeFunctionDecl); - logCallToRootMethods(calleeFunctionDecl, callSet); const Stmt* parent = parentStmt(expr); - // Now do the checks necessary for the "unnecessary public" analysis + // Now do the checks necessary for the "can be private" analysis CXXMethodDecl* calleeMethodDecl = dyn_cast(calleeFunctionDecl); if (calleeMethodDecl && calleeMethodDecl->getAccess() == AS_public) { @@ -304,6 +301,10 @@ bool UnusedMethods::VisitCXXConstructExpr( const CXXConstructExpr* constructExpr const CXXConstructorDecl* constructorDecl = constructExpr->getConstructor(); constructorDecl = constructorDecl->getCanonicalDecl(); + if (!constructorDecl->getLocation().isValid() || ignoreLocation(constructorDecl)) { + return true; + } + logCallToRootMethods(constructorDecl, callSet); return true; @@ -311,16 +312,10 @@ bool UnusedMethods::VisitCXXConstructExpr( const CXXConstructExpr* constructExpr bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl ) { - functionDecl = functionDecl->getCanonicalDecl(); - const CXXMethodDecl* methodDecl = dyn_cast_or_null(functionDecl); - - // ignore method overrides, since the call will show up as being directed to the root method - if (methodDecl && (methodDecl->size_overridden_methods() != 0 || methodDecl->hasAttr())) { - return true; - } + const FunctionDecl* canonicalFunctionDecl = functionDecl->getCanonicalDecl(); // ignore stuff that forms part of the stable URE interface if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc( - functionDecl->getCanonicalDecl()->getNameInfo().getLoc()))) { + canonicalFunctionDecl->getNameInfo().getLoc()))) { return true; } if (isa(functionDecl)) { @@ -332,15 +327,17 @@ bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl ) if (isa(functionDecl) && dyn_cast(functionDecl)->isCopyConstructor()) { return true; } - - if( functionDecl->getLocation().isValid() && !ignoreLocation( functionDecl ) - && !functionDecl->isExternC()) - { - MyFuncInfo funcInfo = niceName(functionDecl); + if (!canonicalFunctionDecl->getLocation().isValid() || ignoreLocation(canonicalFunctionDecl)) { + return true; + } + // ignore method overrides, since the call will show up as being directed to the root method + const CXXMethodDecl* methodDecl = dyn_cast(functionDecl); + if (methodDecl && (methodDecl->size_overridden_methods() != 0 || methodDecl->hasAttr())) { + return true; + } + if (!functionDecl->isExternC()) { + MyFuncInfo funcInfo = niceName(canonicalFunctionDecl); definitionSet.insert(funcInfo); - if (functionDecl->getAccess() == AS_public) { - publicDefinitionSet.insert(funcInfo); - } } return true; } @@ -348,12 +345,12 @@ bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl ) // this catches places that take the address of a method bool UnusedMethods::VisitDeclRefExpr( const DeclRefExpr* declRefExpr ) { - const Decl* functionDecl = declRefExpr->getDecl(); - if (!isa(functionDecl)) { + const FunctionDecl* functionDecl = dyn_cast(declRefExpr->getDecl()); + if (!functionDecl) { return true; } - logCallToRootMethods(dyn_cast(functionDecl)->getCanonicalDecl(), callSet); - logCallToRootMethods(dyn_cast(functionDecl)->getCanonicalDecl(), usedReturnSet); + logCallToRootMethods(functionDecl->getCanonicalDecl(), callSet); + logCallToRootMethods(functionDecl->getCanonicalDecl(), usedReturnSet); return true; } diff --git a/compilerplugins/clang/unusedmethods.py b/compilerplugins/clang/unusedmethods.py index 41044d37f5ef..1e2a67424dd6 100755 --- a/compilerplugins/clang/unusedmethods.py +++ b/compilerplugins/clang/unusedmethods.py @@ -14,7 +14,7 @@ definitionToSourceLocationMap = dict() # for the "unused methods" analysis callSet = set() # set of tuple(return_type, name_and_params) -# for the "unnecessary public" analysis +# for the "method can be private" analysis publicDefinitionSet = set() # set of tuple(return_type, name_and_params) calledFromOutsideSet = set() # set of tuple(return_type, name_and_params) @@ -25,23 +25,6 @@ usedReturnSet = set() # set of tuple(return_type, name_and_params) # things we need to exclude for reasons like : # - it's a weird template thingy that confuses the plugin unusedMethodsExclusionSet = set([ - "double basegfx::DoubleTraits::maxVal()", - "double basegfx::DoubleTraits::minVal()", - "double basegfx::DoubleTraits::neutral()", - "int basegfx::Int32Traits::maxVal()", - "int basegfx::Int32Traits::minVal()", - "int basegfx::Int32Traits::neutral()", - "unsigned long UniqueIndexImpl::Insert(void *)", - "class XMLPropertyBackpatcher & XMLTextImportHelper::GetFootnoteBP()", - "class XMLPropertyBackpatcher & XMLTextImportHelper::GetSequenceIdBP()", - "void XclExpPivotCache::SaveXml(class XclExpXmlStream &)", - - - # TODO track instantiations of template class constructors - "void comphelper::IEventProcessor::release()", - "void SotMutexHolder::acquire()", - "void SotMutexHolder::release()", - # only used by Windows build "_Bool basegfx::B2ITuple::equalZero() const", "class basegfx::B2DPolyPolygon basegfx::unotools::UnoPolyPolygon::getPolyPolygonUnsafe() const", @@ -72,10 +55,6 @@ unusedMethodsExclusionSet = set([ "_Bool ScImportExport::ImportData(const class rtl::OUString &,const class com::sun::star::uno::Any &)", "void* ScannerManager::GetData()", "void ScannerManager::SetData(void *)", - # instantiated from templates, not sure why it is not being picked up - "class basegfx::B2DPolygon OutputDevice::PixelToLogic(const class basegfx::B2DPolygon &,const class MapMode &) const", - "type-parameter-0-0 * detail::cloner::clone(type-parameter-0-0 *const)", - "const class rtl::OUString writerperfect::DocumentHandlerFor::name()", # only used by OSX build "void StyleSettings::SetHideDisabledMenuItems(_Bool)", # debugging methods @@ -83,12 +62,6 @@ unusedMethodsExclusionSet = set([ "void oox::PropertyMap::dumpCode(class com::sun::star::uno::Reference)", "void oox::PropertyMap::dumpData(class com::sun::star::uno::Reference)", "class std::basic_string, class std::allocator > writerfilter::ooxml::OOXMLPropertySet::toString()", - # deep template magic in SW - "Ring * sw::Ring::Ring_node_traits::get_next(const Ring *)", - "Ring * sw::Ring::Ring_node_traits::get_previous(const Ring *)", - "void sw::Ring::Ring_node_traits::set_next(Ring *,Ring *)", - "void sw::Ring::Ring_node_traits::set_previous(Ring *,Ring *)", - "type-parameter-0-0 checking_cast(type-parameter-0-0,type-parameter-0-0)", # I need to teach the plugin that for loops with range expressions call begin() and end() "class __gnu_debug::_Safe_iterator > >, class std::__debug::vector > > SwSortedObjs::begin() const", "class __gnu_debug::_Safe_iterator > >, class std::__debug::vector > > SwSortedObjs::end() const", @@ -158,6 +131,8 @@ with io.open("loplugin.unusedmethods.log", "rb", buffering=1024*1024) as txt: returnType = tokens[1] nameAndParams = tokens[2] calledFromOutsideSet.add((normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams))) + else: + print( "unknown line: " + line) # Invert the definitionToSourceLocationMap. # If we see more than one method at the same sourceLocation, it's being autogenerated as part of a template @@ -283,7 +258,7 @@ for d in definitionSet: tmp1set.add((method, location)) # print out the results, sorted by name and line number -with open("loplugin.unusedmethods.report-methods", "wt") as f: +with open("loplugin.unusedmethods.report-unused-methods", "wt") as f: for t in sort_set_by_natural_key(tmp1set): f.write(t[1] + "\n") f.write(" " + t[0] + "\n") @@ -343,14 +318,14 @@ for d in definitionSet: tmp2set.add((method, location)) # print output, sorted by name and line number -with open("loplugin.unusedmethods.report-returns", "wt") as f: +with open("loplugin.unusedmethods.report-unused-returns", "wt") as f: for t in sort_set_by_natural_key(tmp2set): f.write(t[1] + "\n") f.write(" " + t[0] + "\n") # -------------------------------------------------------------------------------------------- -# "unnecessary public" analysis +# "method can be private" analysis # -------------------------------------------------------------------------------------------- tmp3set = set() @@ -366,10 +341,8 @@ for d in publicDefinitionSet: tmp3set.add((method, definitionToSourceLocationMap[d])) # print output, sorted by name and line number -with open("loplugin.unusedmethods.report-public", "wt") as f: +with open("loplugin.unusedmethods.report-can-be-private", "wt") as f: for t in sort_set_by_natural_key(tmp3set): f.write(t[1] + "\n") f.write(" " + t[0] + "\n") - - -- cgit