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 <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 54 insertions, 76 deletions

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<CXXNullPtrLiteralExpr>(arg)) {
         return "0";
     }
+    if (isa<MaterializeTemporaryExpr>(arg))
+    {
+        const CXXBindTemporaryExpr* strippedArg = dyn_cast_or_null<CXXBindTemporaryExpr>(arg->IgnoreParenCasts());
+        if (strippedArg && isa<CXXTemporaryObjectExpr>(strippedArg->getSubExpr())
+            && dyn_cast<CXXTemporaryObjectExpr>(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<FunctionDecl>(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")