From 20a9e101d909cb1953101e5962b74731e1265400 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Wed, 14 Sep 2016 09:01:06 +0200 Subject: loplugin:constantparam clean up the plugin a little, and try to catch params which are default constructed, which doesn't seem to be working yet Change-Id: Ife45f18502a45cd26306424b7432c55fcbb0fd12 Reviewed-on: https://gerrit.libreoffice.org/28861 Reviewed-by: Noel Grandin Tested-by: Noel Grandin --- compilerplugins/clang/constantparam.cxx | 87 +++++++++++++-------------------- compilerplugins/clang/constantparam.py | 43 +++++++--------- 2 files changed, 54 insertions(+), 76 deletions(-) (limited to 'compilerplugins') diff --git a/compilerplugins/clang/constantparam.cxx b/compilerplugins/clang/constantparam.cxx index 58a2c212eead..61428ac60dff 100644 --- a/compilerplugins/clang/constantparam.cxx +++ b/compilerplugins/clang/constantparam.cxx @@ -33,6 +33,7 @@ struct MyCallSiteInfo std::string returnType; std::string nameAndParams; std::string paramName; + std::string paramType; int paramIndex; // because in some declarations the names are empty std::string callValue; std::string sourceLocation; @@ -55,27 +56,6 @@ public: virtual void run() override { - // there is a crash here that I can't seem to workaround - // clang-3.8: /home/noel/clang/src/tools/clang/lib/AST/Type.cpp:1878: bool clang::Type::isConstantSizeType() const: Assertion `!isDependentType() && "This doesn't make sense for dependent types"' failed. - FileID mainFileID = compiler.getSourceManager().getMainFileID(); - static const char* prefix = SRCDIR "/oox/source"; - const FileEntry * fe = compiler.getSourceManager().getFileEntryForID(mainFileID); - if (strncmp(prefix, fe->getDir()->getName(), strlen(prefix)) == 0) { - return; - } - if (strcmp(fe->getDir()->getName(), SRCDIR "/sw/source/filter/ww8") == 0) - { - return; - } - if (strcmp(fe->getDir()->getName(), SRCDIR "/sc/source/filter/oox") == 0) - { - return; - } - if (strcmp(fe->getDir()->getName(), SRCDIR "/sd/source/filter/eppt") == 0) - { - return; - } - TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes @@ -84,7 +64,7 @@ public: std::string output; for (const MyCallSiteInfo & s : callSet) output += s.returnType + "\t" + s.nameAndParams + "\t" + s.sourceLocation + "\t" - + s.paramName + "\t" + s.callValue + "\n"; + + s.paramName + "\t" + s.paramType + "\t" + s.callValue + "\n"; ofstream myfile; myfile.open( SRCDIR "/loplugin.constantparam.log", ios::app | ios::out); myfile << output; @@ -98,11 +78,11 @@ public: bool VisitDeclRefExpr( const DeclRefExpr* ); bool VisitCXXConstructExpr( const CXXConstructExpr* ); private: - MyCallSiteInfo niceName(const FunctionDecl* functionDecl, int paramIndex, llvm::StringRef paramName, const std::string& callValue); + void addToCallSet(const FunctionDecl* functionDecl, int paramIndex, llvm::StringRef paramName, const std::string& callValue); std::string getCallValue(const Expr* arg); }; -MyCallSiteInfo ConstantParam::niceName(const FunctionDecl* functionDecl, int paramIndex, llvm::StringRef paramName, const std::string& callValue) +void ConstantParam::addToCallSet(const FunctionDecl* functionDecl, int paramIndex, llvm::StringRef paramName, const std::string& callValue) { if (functionDecl->getInstantiatedFromMemberFunction()) functionDecl = functionDecl->getInstantiatedFromMemberFunction(); @@ -114,6 +94,21 @@ MyCallSiteInfo ConstantParam::niceName(const FunctionDecl* functionDecl, int par functionDecl = functionDecl->getTemplateInstantiationPattern(); #endif + + if (!functionDecl->getNameInfo().getLoc().isValid()) + return; + if (ignoreLocation(functionDecl)) + return; + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(functionDecl->getLocation()))) + return; + SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( functionDecl->getLocation() ); + StringRef filename = compiler.getSourceManager().getFilename(expansionLoc); + if (!filename.startswith(SRCDIR)) + return; + filename = filename.substr(strlen(SRCDIR)+1); + + MyCallSiteInfo aInfo; aInfo.returnType = compat::getReturnType(*functionDecl).getCanonicalType().getAsString(); @@ -137,13 +132,13 @@ MyCallSiteInfo ConstantParam::niceName(const FunctionDecl* functionDecl, int par } aInfo.paramName = paramName; aInfo.paramIndex = paramIndex; + if (paramIndex < (int)functionDecl->getNumParams()) + aInfo.paramType = functionDecl->getParamDecl(paramIndex)->getType().getCanonicalType().getAsString(); aInfo.callValue = callValue; - 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)); + aInfo.sourceLocation = filename.str() + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc)); - return aInfo; + callSet.insert(aInfo); } std::string ConstantParam::getCallValue(const Expr* arg) @@ -165,6 +160,15 @@ std::string ConstantParam::getCallValue(const Expr* arg) if (isa(arg)) { return "0"; } + if (isa(arg)) + { + const CXXBindTemporaryExpr* strippedArg = dyn_cast_or_null(arg->IgnoreParenCasts()); + if (strippedArg && isa(strippedArg->getSubExpr()) + && dyn_cast(strippedArg->getSubExpr())->getNumArgs() == 0) + { + return "defaultConstruct"; + } + } return "unknown2"; } @@ -202,15 +206,6 @@ bool ConstantParam::VisitCallExpr(const CallExpr * callExpr) { functionDecl = functionDecl->getTemplateInstantiationPattern(); #endif - if (!functionDecl->getNameInfo().getLoc().isValid() || ignoreLocation(functionDecl)) { - return true; - } - // ignore stuff that forms part of the stable URE interface - if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc( - functionDecl->getNameInfo().getLoc()))) { - return true; - } - unsigned len = std::max(callExpr->getNumArgs(), functionDecl->getNumParams()); for (unsigned i = 0; i < len; ++i) { const Expr* valExpr; @@ -225,8 +220,7 @@ bool ConstantParam::VisitCallExpr(const CallExpr * callExpr) { std::string paramName = i < functionDecl->getNumParams() ? functionDecl->getParamDecl(i)->getName() : llvm::StringRef("###" + std::to_string(i)); - MyCallSiteInfo funcInfo = niceName(functionDecl, i, paramName, callValue); - callSet.insert(funcInfo); + addToCallSet(functionDecl, i, paramName, callValue); } return true; } @@ -241,8 +235,7 @@ bool ConstantParam::VisitDeclRefExpr( const DeclRefExpr* declRefExpr ) const FunctionDecl* functionDecl = dyn_cast(decl); for (unsigned i = 0; i < functionDecl->getNumParams(); ++i) { - MyCallSiteInfo funcInfo = niceName(functionDecl, i, functionDecl->getParamDecl(i)->getName(), "unknown3"); - callSet.insert(funcInfo); + addToCallSet(functionDecl, i, functionDecl->getParamDecl(i)->getName(), "unknown3"); } return true; } @@ -252,15 +245,6 @@ bool ConstantParam::VisitCXXConstructExpr( const CXXConstructExpr* constructExpr const CXXConstructorDecl* constructorDecl = constructExpr->getConstructor(); constructorDecl = constructorDecl->getCanonicalDecl(); - // ignore stuff that forms part of the stable URE interface - if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc( - constructorDecl->getNameInfo().getLoc()))) { - return true; - } - if (!constructorDecl->getNameInfo().getLoc().isValid() || ignoreLocation(constructorDecl)) { - return true; - } - unsigned len = std::max(constructExpr->getNumArgs(), constructorDecl->getNumParams()); for (unsigned i = 0; i < len; ++i) { const Expr* valExpr; @@ -275,8 +259,7 @@ bool ConstantParam::VisitCXXConstructExpr( const CXXConstructExpr* constructExpr std::string paramName = i < constructorDecl->getNumParams() ? constructorDecl->getParamDecl(i)->getName() : llvm::StringRef("###" + std::to_string(i)); - MyCallSiteInfo funcInfo = niceName(constructorDecl, i, paramName, callValue); - callSet.insert(funcInfo); + addToCallSet(constructorDecl, i, paramName, callValue); } return true; } diff --git a/compilerplugins/clang/constantparam.py b/compilerplugins/clang/constantparam.py index ae2b2433ae54..ed5acc449968 100755 --- a/compilerplugins/clang/constantparam.py +++ b/compilerplugins/clang/constantparam.py @@ -4,9 +4,7 @@ import sys import re import io -definitionSet = set() -definitionToSourceLocationMap = dict() -callParamSet = dict() +callDict = dict() # 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,23 +15,20 @@ def normalizeTypeParams( line ): # reading as binary (since we known it is pure ascii) is much faster than reading as unicode with io.open("loplugin.constantparam.log", "rb", buffering=1024*1024) as txt: for line in txt: - idx1 = line.find("\t") - idx2 = line.find("\t",idx1+1) - idx3 = line.find("\t",idx2+1) - idx4 = line.find("\t",idx3+1) - returnType = normalizeTypeParams(line[:idx1]) - nameAndParams = normalizeTypeParams(line[idx1+1:idx2]) - sourceLocation = line[idx2+1:idx3] - paramName = line[idx3+1:idx4] - callValue = line[idx4+1:].strip() - callInfo = (returnType, nameAndParams, paramName) - if not callInfo in callParamSet: - callParamSet[callInfo] = set() - callParamSet[callInfo].add(callValue) - definitionToSourceLocationMap[callInfo] = sourceLocation + tokens = line.strip().split("\t") + returnType = normalizeTypeParams(tokens[0]) + nameAndParams = normalizeTypeParams(tokens[1]) + sourceLocation = tokens[2] + paramName = tokens[3] + paramType = normalizeTypeParams(tokens[4]) + callValue = tokens[5] + callInfo = (returnType, nameAndParams, paramName, paramType, sourceLocation) + if not callInfo in callDict: + callDict[callInfo] = set() + callDict[callInfo].add(callValue) tmp1list = list() -for callInfo, callValues in callParamSet.iteritems(): +for callInfo, callValues in callDict.iteritems(): nameAndParams = callInfo[1] if len(callValues) != 1: continue @@ -43,22 +38,22 @@ for callInfo, callValues in callParamSet.iteritems(): # try and ignore setter methods if ("," not in nameAndParams) and (("::set" in nameAndParams) or ("::Set" in nameAndParams)): continue - v0 = callInfo[0] + " " + callInfo[1] - v1 = callInfo[2] + " " + callValue - v2 = definitionToSourceLocationMap[callInfo] + v0 = callInfo[4] + v1 = callInfo[0] + " " + callInfo[1] + v2 = callInfo[3] + " " + callInfo[2] + " " + callValue tmp1list.append((v0,v1,v2)) # sort results by filename:lineno def natural_sort_key(s, _nsre=re.compile('([0-9]+)')): return [int(text) if text.isdigit() else text.lower() for text in re.split(_nsre, s)] -tmp1list.sort(key=lambda v: natural_sort_key(v[2])) +tmp1list.sort(key=lambda v: natural_sort_key(v[0])) # print out the results with open("loplugin.constantparam.report", "wt") as f: for v in tmp1list: - f.write(v[2] + "\n") - f.write(" " + v[0] + "\n") + f.write(v[0] + "\n") f.write(" " + v[1] + "\n") + f.write(" " + v[2] + "\n") -- cgit