diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2016-11-17 08:39:50 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2016-11-17 08:40:27 +0200 |
commit | 2f7ccd102a6280750ce920e4c4eab66f9a01b9d3 (patch) | |
tree | f1e384c6b76b8fd9846db630ee58cdf6c9085482 /compilerplugins | |
parent | Remove redundant checks on nullptr. (diff) | |
download | core-2f7ccd102a6280750ce920e4c4eab66f9a01b9d3.tar.gz core-2f7ccd102a6280750ce920e4c4eab66f9a01b9d3.zip |
extend unusedfields loplugin to find fields that can be private
and apply the results in xmlscript
Change-Id: Ib126f6e1576639abfd171e99d9561be9715ece2f
Diffstat (limited to 'compilerplugins')
-rw-r--r-- | compilerplugins/clang/unusedfields.cxx | 143 | ||||
-rwxr-xr-x | compilerplugins/clang/unusedfields.py | 97 |
2 files changed, 115 insertions, 125 deletions
diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx index 6e4da40439e1..1bf71cdea13b 100644 --- a/compilerplugins/clang/unusedfields.cxx +++ b/compilerplugins/clang/unusedfields.cxx @@ -47,6 +47,7 @@ struct MyFieldInfo std::string fieldName; std::string fieldType; std::string sourceLocation; + std::string access; }; bool operator < (const MyFieldInfo &lhs, const MyFieldInfo &rhs) { @@ -57,6 +58,7 @@ bool operator < (const MyFieldInfo &lhs, const MyFieldInfo &rhs) // try to limit the voluminous output a little static std::set<MyFieldInfo> touchedSet; +static std::set<MyFieldInfo> touchedFromOutsideSet; static std::set<MyFieldInfo> readFromSet; static std::set<MyFieldInfo> definitionSet; @@ -76,11 +78,13 @@ public: std::string output; for (const MyFieldInfo & s : touchedSet) output += "touch:\t" + s.parentClass + "\t" + s.fieldName + "\n"; + for (const MyFieldInfo & s : touchedFromOutsideSet) + output += "outside:\t" + s.parentClass + "\t" + s.fieldName + "\n"; for (const MyFieldInfo & s : readFromSet) output += "read:\t" + s.parentClass + "\t" + s.fieldName + "\n"; for (const MyFieldInfo & s : definitionSet) { - output += "definition:\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n"; + output += "definition:\t" + s.access + "\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n"; } ofstream myfile; myfile.open( SRCDIR "/loplugin.unusedfields.log", ios::app | ios::out); @@ -89,14 +93,14 @@ public: } bool shouldVisitTemplateInstantiations () const { return true; } + bool shouldVisitImplicitCode() const { return true; } - bool VisitCallExpr(CallExpr* ); bool VisitFieldDecl( const FieldDecl* ); bool VisitMemberExpr( const MemberExpr* ); bool VisitDeclRefExpr( const DeclRefExpr* ); private: MyFieldInfo niceName(const FieldDecl*); - std::string fullyQualifiedName(const FunctionDecl*); + void checkForTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr, const MyFieldInfo& fieldInfo); }; MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl) @@ -111,73 +115,27 @@ MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl) aInfo.sourceLocation = std::string(name.substr(strlen(SRCDIR)+1)) + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc)); normalizeDotDotInFilePath(aInfo.sourceLocation); - return aInfo; -} - -std::string UnusedFields::fullyQualifiedName(const FunctionDecl* functionDecl) -{ - std::string ret = compat::getReturnType(*functionDecl).getCanonicalType().getAsString(); - ret += " "; - if (isa<CXXMethodDecl>(functionDecl)) { - const CXXRecordDecl* recordDecl = dyn_cast<CXXMethodDecl>(functionDecl)->getParent(); - ret += recordDecl->getQualifiedNameAsString(); - ret += "::"; - } - ret += functionDecl->getNameAsString() + "("; - bool bFirst = true; - for (const ParmVarDecl *pParmVarDecl : compat::parameters(*functionDecl)) { - if (bFirst) - bFirst = false; - else - ret += ","; - ret += pParmVarDecl->getType().getCanonicalType().getAsString(); - } - ret += ")"; - if (isa<CXXMethodDecl>(functionDecl) && dyn_cast<CXXMethodDecl>(functionDecl)->isConst()) { - ret += " const"; - } - - return ret; -} - -// prevent recursive templates from blowing up the stack -static std::set<std::string> traversedFunctionSet; - -bool UnusedFields::VisitCallExpr(CallExpr* expr) -{ - // Note that I don't ignore ANYTHING here, because I want to get calls to my code that result - // from template instantiation deep inside the STL and other external code - - FunctionDecl* calleeFunctionDecl = expr->getDirectCallee(); - if (calleeFunctionDecl == nullptr) { - Expr* callee = expr->getCallee()->IgnoreParenImpCasts(); - DeclRefExpr* dr = dyn_cast<DeclRefExpr>(callee); - if (dr) { - calleeFunctionDecl = dyn_cast<FunctionDecl>(dr->getDecl()); - if (calleeFunctionDecl) - goto gotfunc; - } - return true; + switch (fieldDecl->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; } -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); - - return true; + return aInfo; } bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl ) { fieldDecl = fieldDecl->getCanonicalDecl(); - const FieldDecl* canonicalDecl = fieldDecl; - - if( ignoreLocation( fieldDecl )) + if (ignoreLocation( fieldDecl )) { return true; + } + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) { + return true; + } QualType type = fieldDecl->getType(); // unwrap array types @@ -200,7 +158,7 @@ bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl ) return true; } */ - definitionSet.insert(niceName(canonicalDecl)); + definitionSet.insert(niceName(fieldDecl)); return true; } @@ -211,20 +169,22 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr ) if (!fieldDecl) { return true; } + fieldDecl = fieldDecl->getCanonicalDecl(); + if (ignoreLocation(fieldDecl)) { + return true; + } + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) { + return true; + } MyFieldInfo fieldInfo = niceName(fieldDecl); - // ignore move/copy operator, it's self->self - const FunctionDecl* parentFunction = parentFunctionDecl(memberExpr); - const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(parentFunction); - if (!methodDecl || !(methodDecl->isCopyAssignmentOperator() || methodDecl->isMoveAssignmentOperator())) { - touchedSet.insert(fieldInfo); - } + // for the touched-from-outside analysis - // for the write-only analysis + checkForTouchedFromOutside(fieldDecl, memberExpr, fieldInfo); - if (ignoreLocation(memberExpr)) - return true; + // for the write-only analysis const Stmt* child = memberExpr; const Stmt* parent = parentStmt(memberExpr); @@ -298,13 +258,48 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr ) bool UnusedFields::VisitDeclRefExpr( const DeclRefExpr* declRefExpr ) { const Decl* decl = declRefExpr->getDecl(); - if (!isa<FieldDecl>(decl)) { + const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(decl); + if (!fieldDecl) { + return true; + } + fieldDecl = fieldDecl->getCanonicalDecl(); + if (ignoreLocation(fieldDecl)) { return true; } - touchedSet.insert(niceName(dyn_cast<FieldDecl>(decl))); + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) { + return true; + } + MyFieldInfo fieldInfo = niceName(fieldDecl); + touchedSet.insert(fieldInfo); + checkForTouchedFromOutside(fieldDecl, declRefExpr, fieldInfo); return true; } +void UnusedFields::checkForTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr, const MyFieldInfo& fieldInfo) { + const FunctionDecl* memberExprParentFunction = parentFunctionDecl(memberExpr); + const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(memberExprParentFunction); + + // it's touched from somewhere outside a class + if (!methodDecl) { + touchedSet.insert(fieldInfo); + touchedFromOutsideSet.insert(fieldInfo); + return; + } + + auto constructorDecl = dyn_cast<CXXConstructorDecl>(methodDecl); + if (methodDecl->isCopyAssignmentOperator() || methodDecl->isMoveAssignmentOperator()) { + // ignore move/copy operator, it's self->self + } else if (constructorDecl && (constructorDecl->isCopyConstructor() || constructorDecl->isMoveConstructor())) { + // ignore move/copy constructor, it's self->self + } else { + touchedSet.insert(fieldInfo); + if (memberExprParentFunction->getParent() != fieldDecl->getParent()) { + touchedFromOutsideSet.insert(fieldInfo); + } + } +} + loplugin::Plugin::Registration< UnusedFields > X("unusedfields", false); } diff --git a/compilerplugins/clang/unusedfields.py b/compilerplugins/clang/unusedfields.py index 785b7a9a1881..7bf910f62d80 100755 --- a/compilerplugins/clang/unusedfields.py +++ b/compilerplugins/clang/unusedfields.py @@ -5,11 +5,13 @@ import re import io definitionSet = set() +protectedAndPublicDefinitionSet = set() # set of tuple(type, name) definitionToSourceLocationMap = dict() definitionToTypeMap = dict() callSet = set() readFromSet = set() sourceLocationSet = set() +touchedFromOutsideSet = set() # clang does not always use exactly the same numbers in the type-parameter vars it generates # so I need to substitute them to ensure we can match correctly. @@ -17,30 +19,43 @@ normalizeTypeParamsRegex = re.compile(r"type-parameter-\d+-\d+") def normalizeTypeParams( line ): return normalizeTypeParamsRegex.sub("type-parameter-?-?", line) +def parseFieldInfo( tokens ): + if len(tokens) == 3: + return (normalizeTypeParams(tokens[1]), tokens[2]) + else: + return (normalizeTypeParams(tokens[1]), "") + # The parsing here is designed to avoid grabbing stuff which is mixed in from gbuild. # I have not yet found a way of suppressing the gbuild output. with io.open("loplugin.unusedfields.log", "rb", buffering=1024*1024) as txt: for line in txt: tokens = line.strip().split("\t") if tokens[0] == "definition:": - fieldInfo = (normalizeTypeParams(tokens[1]), tokens[2]) + access = tokens[1] + fieldInfo = (normalizeTypeParams(tokens[2]), tokens[3]) + srcLoc = tokens[5] + # ignore external source code + if (srcLoc.startswith("external/")): + continue + # ignore build folder + if (srcLoc.startswith("workdir/")): + continue definitionSet.add(fieldInfo) - definitionToTypeMap[fieldInfo] = tokens[3] - definitionToSourceLocationMap[fieldInfo] = tokens[4] + definitionToTypeMap[fieldInfo] = tokens[4] + if access == "protected" or access == "public": + protectedAndPublicDefinitionSet.add(fieldInfo) + definitionToSourceLocationMap[fieldInfo] = tokens[5] elif tokens[0] == "touch:": - if len(tokens) == 3: - callInfo = (normalizeTypeParams(tokens[1]), tokens[2]) - callSet.add(callInfo) - else: - callInfo = (normalizeTypeParams(tokens[1]), "") - callSet.add(callInfo) + callSet.add(parseFieldInfo(tokens)) + elif tokens[0] == "read:": + readFromSet.add(parseFieldInfo(tokens)) elif tokens[0] == "read:": - if len(tokens) == 3: - readInfo = (normalizeTypeParams(tokens[1]), tokens[2]) - readFromSet.add(readInfo) - else: - readInfo = (normalizeTypeParams(tokens[1]), "") - readFromSet.add(readInfo) + readFromSet.add(parseFieldInfo(tokens)) + elif tokens[0] == "outside:": + touchedFromOutsideSet.add(parseFieldInfo(tokens)) + 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 # and we should just ignore @@ -58,24 +73,6 @@ for d in definitionSet: if d in callSet: continue srcLoc = definitionToSourceLocationMap[d]; - # ignore external source code - if (srcLoc.startswith("external/")): - continue - # ignore build folder - if (srcLoc.startswith("workdir/")): - continue - # ignore our stable/URE/UNO api - if (srcLoc.startswith("include/com/") - or srcLoc.startswith("include/cppu/") - or srcLoc.startswith("include/cppuhelper/") - or srcLoc.startswith("include/osl/") - or srcLoc.startswith("include/rtl/") - or srcLoc.startswith("include/sal/") - or srcLoc.startswith("include/salhelper/") - or srcLoc.startswith("include/systools/") - or srcLoc.startswith("include/typelib/") - or srcLoc.startswith("include/uno/")): - continue # this is all representations of on-disk data structures if (srcLoc.startswith("sc/source/filter/inc/scflt.hxx") or srcLoc.startswith("sw/source/filter/ww8/") @@ -108,24 +105,6 @@ for d in definitionSet: if d in readFromSet: continue srcLoc = definitionToSourceLocationMap[d]; - # ignore external source code - if (srcLoc.startswith("external/")): - continue - # ignore build folder - if (srcLoc.startswith("workdir/")): - continue - # ignore our stable/URE/UNO api - if (srcLoc.startswith("include/com/") - or srcLoc.startswith("include/cppu/") - or srcLoc.startswith("include/cppuhelper/") - or srcLoc.startswith("include/osl/") - or srcLoc.startswith("include/rtl/") - or srcLoc.startswith("include/sal/") - or srcLoc.startswith("include/salhelper/") - or srcLoc.startswith("include/systools/") - or srcLoc.startswith("include/typelib/") - or srcLoc.startswith("include/uno/")): - continue # this is all representations of on-disk data structures if (srcLoc.startswith("sc/source/filter/inc/scflt.hxx") or srcLoc.startswith("sw/source/filter/ww8/") @@ -140,6 +119,17 @@ for d in definitionSet: writeonlySet.add((clazz + " " + definitionToTypeMap[d], srcLoc)) + +canBePrivateSet = set() +for d in protectedAndPublicDefinitionSet: + clazz = d[0] + " " + d[1] + if d in touchedFromOutsideSet: + continue + srcLoc = definitionToSourceLocationMap[d]; + + canBePrivateSet.add((clazz + " " + definitionToTypeMap[d], srcLoc)) + + # sort the results using a "natural order" so sequences like [item1,item2,item10] sort nicely def natural_sort_key(s, _nsre=re.compile('([0-9]+)')): return [int(text) if text.isdigit() else text.lower() @@ -148,6 +138,7 @@ def natural_sort_key(s, _nsre=re.compile('([0-9]+)')): # sort results by name and line number tmp1list = sorted(untouchedSet, key=lambda v: natural_sort_key(v[1])) tmp2list = sorted(writeonlySet, key=lambda v: natural_sort_key(v[1])) +tmp3list = sorted(canBePrivateSet, key=lambda v: natural_sort_key(v[1])) # print out the results with open("loplugin.unusedfields.report-untouched", "wt") as f: @@ -158,5 +149,9 @@ with open("loplugin.unusedfields.report-writeonly", "wt") as f: for t in tmp2list: f.write( t[1] + "\n" ) f.write( " " + t[0] + "\n" ) +with open("loplugin.unusedfields.report-can-be-private", "wt") as f: + for t in tmp3list: + f.write( t[1] + "\n" ) + f.write( " " + t[0] + "\n" ) |