From d34d792230251f8d27008522c4e63fb61db32773 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Mon, 1 Feb 2016 14:52:38 +0200 Subject: new loplugin to find public methods that can be private based on the unusedmethods plugin, which I should probably rename at some point Change-Id: If197423c59d4350ea1fdc69e99d24b631d9751b9 --- compilerplugins/clang/unusedmethods.cxx | 94 +++++++++++++++++++++++++++------ compilerplugins/clang/unusedmethods.py | 65 ++++++++++++++++++----- 2 files changed, 131 insertions(+), 28 deletions(-) (limited to 'compilerplugins') diff --git a/compilerplugins/clang/unusedmethods.cxx b/compilerplugins/clang/unusedmethods.cxx index 6f724f8b0e36..b05ffe3dae5d 100644 --- a/compilerplugins/clang/unusedmethods.cxx +++ b/compilerplugins/clang/unusedmethods.cxx @@ -16,8 +16,14 @@ #include "compat.hxx" /** -Dump a list of calls to methods, and a list of method definitions. -Then we will post-process the 2 lists and find the set of unused methods. +This plugin performs 3 different analyses: + +(1) Find unused methods +(2) Find methods whose return types are never evaluated +(3) Find methods which are public, but are never called from outside the class i.e. they can be private + +It does so, by dumping various call/definition/use info to a log file. +Then we will post-process the various lists and find the set of unused methods. Be warned that it produces around 5G of log file. @@ -41,6 +47,7 @@ namespace { struct MyFuncInfo { + std::string access; std::string returnType; std::string nameAndParams; std::string sourceLocation; @@ -58,16 +65,17 @@ struct MyFuncInfo // try to limit the voluminous output a little + +// for the "unused method" analysis static std::set callSet; -static std::set usedReturnSet; static std::set definitionSet; +// for the "unused return type" analysis +static std::set usedReturnSet; +// for the "unnecessary public" analysis +static std::set publicDefinitionSet; +static std::set calledFromOutsideSet; -static bool startswith(const std::string& s, const std::string& prefix) -{ - return s.rfind(prefix,0) == 0; -} - class UnusedMethods: public RecursiveASTVisitor, public loplugin::Plugin { @@ -80,17 +88,19 @@ public: // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes // writing to the same logfile + std::string output; + for (const MyFuncInfo & s : definitionSet) + output += "definition:\t" + s.access + "\t" + s.returnType + "\t" + s.nameAndParams + "\t" + s.sourceLocation + "\n"; + // for the "unused method" analysis for (const MyFuncInfo & s : callSet) output += "call:\t" + s.returnType + "\t" + s.nameAndParams + "\n"; + // for the "unused return type" analysis for (const MyFuncInfo & s : usedReturnSet) output += "usedReturn:\t" + s.returnType + "\t" + s.nameAndParams + "\n"; - for (const MyFuncInfo & s : definitionSet) - { - //treat all UNO interfaces as having been called, since they are part of our external ABI - if (!startswith(s.nameAndParams, "com::sun::star::")) - output += "definition:\t" + s.returnType + "\t" + s.nameAndParams + "\t" + s.sourceLocation + "\n"; - } + // for the "unnecessary public" analysis + for (const MyFuncInfo & s : calledFromOutsideSet) + output += "outside:\t" + s.returnType + "\t" + s.nameAndParams + "\n"; ofstream myfile; myfile.open( SRCDIR "/unusedmethods.log", ios::app | ios::out); myfile << output; @@ -121,6 +131,13 @@ MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl) #endif MyFuncInfo aInfo; + switch (functionDecl->getAccess()) + { + case AS_public: aInfo.access = "public"; break; + case AS_private: aInfo.access = "private"; break; + case AS_protected: aInfo.access = "protected"; break; + default: aInfo.access = "unknown"; break; + } aInfo.returnType = compat::getReturnType(*functionDecl).getCanonicalType().getAsString(); if (isa(functionDecl)) { @@ -201,6 +218,33 @@ void UnusedMethods::logCallToRootMethods(const FunctionDecl* functionDecl, std:: // prevent recursive templates from blowing up the stack static std::set traversedFunctionSet; +const Decl* get_DeclContext_from_Stmt(ASTContext& context, const Stmt& stmt) +{ + auto it = context.getParents(stmt).begin(); + + if (it == context.getParents(stmt).end()) + return nullptr; + + const Decl *aDecl = it->get(); + if (aDecl) + return aDecl; + + const Stmt *aStmt = it->get(); + if (aStmt) + return get_DeclContext_from_Stmt(context, *aStmt); + + return nullptr; +} + +static const FunctionDecl* get_top_FunctionDecl_from_Stmt(ASTContext& context, const Stmt& stmt) +{ + const Decl *decl = get_DeclContext_from_Stmt(context, stmt); + if (decl) + return static_cast(decl->getNonClosureContext()); + + return nullptr; +} + bool UnusedMethods::VisitCallExpr(CallExpr* expr) { // Note that I don't ignore ANYTHING here, because I want to get calls to my code that result @@ -232,10 +276,22 @@ gotfunc: logCallToRootMethods(calleeFunctionDecl, callSet); + const Stmt* parent = parentStmt(expr); + + // Now do the checks necessary for the "unnecessary public" analysis + CXXMethodDecl* calleeMethodDecl = dyn_cast(calleeFunctionDecl); + if (calleeMethodDecl && calleeMethodDecl->getAccess() == AS_public) + { + const FunctionDecl* parentFunctionDecl = get_top_FunctionDecl_from_Stmt(compiler.getASTContext(), *expr); + if (parentFunctionDecl && parentFunctionDecl != calleeFunctionDecl) { + calledFromOutsideSet.insert(niceName(parentFunctionDecl)); + } + } + + // Now do the checks necessary for the "unused return value" analysis if (calleeFunctionDecl->getReturnType()->isVoidType()) { return true; } - const Stmt* parent = parentStmt(expr); if (!parent) { // we will get null parent if it's under a CXXConstructExpr node logCallToRootMethods(calleeFunctionDecl, usedReturnSet); @@ -283,7 +339,13 @@ bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl ) } if( functionDecl->getLocation().isValid() && !ignoreLocation( functionDecl )) - definitionSet.insert(niceName(functionDecl)); + { + MyFuncInfo funcInfo = niceName(functionDecl); + definitionSet.insert(funcInfo); + if (functionDecl->getAccess() == AS_public) { + publicDefinitionSet.insert(funcInfo); + } + } return true; } diff --git a/compilerplugins/clang/unusedmethods.py b/compilerplugins/clang/unusedmethods.py index 2f94f16bd9ca..504a97fc4e32 100755 --- a/compilerplugins/clang/unusedmethods.py +++ b/compilerplugins/clang/unusedmethods.py @@ -5,10 +5,13 @@ import re import io definitionSet = set() +publicDefinitionSet = set() definitionToSourceLocationMap = dict() callSet = set() -returnSet = set() +usedReturnSet = set() sourceLocationSet = set() +calledFromOutsideSet = set() + # things we need to exclude for reasons like : # - it's a weird template thingy that confuses the plugin exclusionSet = set([ @@ -120,19 +123,35 @@ with io.open(sys.argv[1], "rb", buffering=1024*1024) as txt: if line.startswith("definition:\t"): idx1 = line.find("\t",12) idx2 = line.find("\t",idx1+1) - funcInfo = (normalizeTypeParams(line[12:idx1]), normalizeTypeParams(line[idx1+1:idx2])) + idx3 = line.find("\t",idx2+1) + access = line[12:idx1] + returnType = line[idx1+1:idx2] + nameAndParams = line[idx2+1:idx3] + sourceLocation = line[idx3+1:].strip() + funcInfo = (normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams)) definitionSet.add(funcInfo) - definitionToSourceLocationMap[funcInfo] = line[idx2+1:].strip() + if access == "public": + publicDefinitionSet.add(funcInfo) + definitionToSourceLocationMap[funcInfo] = sourceLocation elif line.startswith("call:\t"): idx1 = line.find("\t",6) - callSet.add((normalizeTypeParams(line[6:idx1]), normalizeTypeParams(line[idx1+1:].strip()))) + returnType = line[6:idx1] + nameAndParams = line[idx1+1:].strip() + callSet.add((normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams))) elif line.startswith("usedReturn:\t"): idx1 = line.find("\t",12) - returnSet.add((normalizeTypeParams(line[12:idx1]), normalizeTypeParams(line[idx1+1:].strip()))) + returnType = line[12:idx1] + nameAndParams = line[idx1+1:].strip() + usedReturnSet.add((normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams))) + elif line.startswith("calledFromOutsideSet:\t"): + idx1 = line.find("\t",22) + returnType = line[22:idx1] + nameAndParams = line[idx1+1:].strip() + calledFromOutsideSet.add((normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams))) -# Invert the definitionToSourceLocationMap +# Invert the definitionToSourceLocationMap. # If we see more than one method at the same sourceLocation, it's being autogenerated as part of a template -# and we should just ignore +# and we should just ignore it. sourceLocationToDefinitionMap = {} for k, v in definitionToSourceLocationMap.iteritems(): sourceLocationToDefinitionMap[v] = sourceLocationToDefinitionMap.get(v, []) @@ -253,11 +272,9 @@ tmp1list = sorted(tmp1set, key=lambda v: natural_sort_key(v[1])) tmp2set = set() for d in definitionSet: clazz = d[0] + " " + d[1] - if clazz in exclusionSet: - continue - if d in returnSet: + if d in usedReturnSet: continue - if isOtherConstness(d, returnSet): + if isOtherConstness(d, usedReturnSet): continue if d[0] == "void": continue @@ -287,7 +304,31 @@ for d in definitionSet: # sort results by name and line number tmp2list = sorted(tmp2set, key=lambda v: natural_sort_key(v[1])) -for t in tmp2list: +#for t in tmp2list: +# print t[1] +# print " ", t[0] + + +# ------------------------------------------- +# Do the "unnecessary public" part +# ------------------------------------------- + +tmp3set = set() +for d in publicDefinitionSet: + clazz = d[0] + " " + d[1] + if d in calledFromOutsideSet: + continue + if isOtherConstness(d, calledFromOutsideSet): + continue + # ignore external code + if definitionToSourceLocationMap[d].startswith("external/"): + continue + tmp3set.add((clazz, definitionToSourceLocationMap[d])) + +# sort results by name and line number +tmp3list = sorted(tmp3set, key=lambda v: natural_sort_key(v[1])) + +for t in tmp3list: print t[1] print " ", t[0] -- cgit