can-be-private analysis needs to ignore virtual methods

Change-Id: I1e2f28ed550ff9751912f70e3eec511ddc5b75cf

2 files changed, 32 insertions, 9 deletions

diff --git a/compilerplugins/clang/unusedmethods.cxx b/compilerplugins/clang/unusedmethods.cxx
index 98f19ac4a00f..6a2d8d2ca2b4 100644
--- a/compilerplugins/clang/unusedmethods.cxx
+++ b/compilerplugins/clang/unusedmethods.cxx
@@ -54,6 +54,7 @@ struct MyFuncInfo
     std::string returnType;
     std::string nameAndParams;
     std::string sourceLocation;
+    std::string virtualness;
 
 };
 bool operator < (const MyFuncInfo &lhs, const MyFuncInfo &rhs)
@@ -88,7 +89,8 @@ public:
 
         std::string output;
         for (const MyFuncInfo & s : definitionSet)
-            output += "definition:\t" + s.access + "\t" + s.returnType + "\t" + s.nameAndParams + "\t" + s.sourceLocation + "\n";
+            output += "definition:\t" + s.access + "\t" + s.returnType + "\t" + s.nameAndParams
+                      + "\t" + s.sourceLocation + "\t" + s.virtualness + "\n";
         // for the "unused method" analysis
         for (const MyFuncInfo & s : callSet)
             output += "call:\t" + s.returnType + "\t" + s.nameAndParams + "\n";
@@ -144,10 +146,12 @@ MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl)
         aInfo.returnType = "";
     }
 
-    if (isa<CXXMethodDecl>(functionDecl)) {
-        const CXXRecordDecl* recordDecl = dyn_cast<CXXMethodDecl>(functionDecl)->getParent();
+    if (const CXXMethodDecl* methodDecl = dyn_cast<CXXMethodDecl>(functionDecl)) {
+        const CXXRecordDecl* recordDecl = methodDecl->getParent();
         aInfo.nameAndParams += recordDecl->getQualifiedNameAsString();
         aInfo.nameAndParams += "::";
+        if (methodDecl->isVirtual())
+            aInfo.virtualness = "virtual";
     }
     aInfo.nameAndParams += functionDecl->getNameAsString() + "(";
     bool bFirst = true;
@@ -230,12 +234,12 @@ gotfunc:
 
     // Now do the checks necessary for the "can be private" analysis
     CXXMethodDecl* calleeMethodDecl = dyn_cast<CXXMethodDecl>(calleeFunctionDecl);
-    if (calleeMethodDecl && calleeMethodDecl->getAccess() == AS_public)
+    if (calleeMethodDecl && calleeMethodDecl->getAccess() != AS_private)
     {
-        const FunctionDecl* parentFunction = parentFunctionDecl(expr);
-        if (parentFunction && parentFunction != calleeFunctionDecl) {
-            if (!ignoreLocation(parentFunction)) {
-                calledFromOutsideSet.insert(niceName(parentFunction));
+        const FunctionDecl* parentFunctionOfCallSite = parentFunctionDecl(expr);
+        if (parentFunctionOfCallSite != calleeFunctionDecl) {
+            if (!parentFunctionOfCallSite || !ignoreLocation(parentFunctionOfCallSite)) {
+                calledFromOutsideSet.insert(niceName(calleeFunctionDecl));
             }
         }
     }
@@ -316,7 +320,6 @@ bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl )
     return true;
 }
 
-// this catches places that take the address of a method
 bool UnusedMethods::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
 {
     const FunctionDecl* functionDecl = dyn_cast<FunctionDecl>(declRefExpr->getDecl());
@@ -325,6 +328,19 @@ bool UnusedMethods::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
     }
     logCallToRootMethods(functionDecl->getCanonicalDecl(), callSet);
     logCallToRootMethods(functionDecl->getCanonicalDecl(), usedReturnSet);
+
+    // Now do the checks necessary for the "can be private" analysis
+    const CXXMethodDecl* methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
+    if (methodDecl && methodDecl->getAccess() != AS_private)
+    {
+        const FunctionDecl* parentFunctionOfCallSite = parentFunctionDecl(declRefExpr);
+        if (parentFunctionOfCallSite != functionDecl) {
+            if (!parentFunctionOfCallSite || !ignoreLocation(parentFunctionOfCallSite)) {
+                calledFromOutsideSet.insert(niceName(functionDecl));
+            }
+        }
+    }
+
     return true;
 }
 
diff --git a/compilerplugins/clang/unusedmethods.py b/compilerplugins/clang/unusedmethods.py
index bf05c2fdf739..53cc93495555 100755
--- a/compilerplugins/clang/unusedmethods.py
+++ b/compilerplugins/clang/unusedmethods.py
@@ -17,6 +17,7 @@ callSet = set() # set of tuple(return_type, name_and_params)
 # 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)
+virtualSet = set() # set of tuple(return_type, name_and_params)
 
 # for the "unused return types" analysis
 usedReturnSet = set() # set of tuple(return_type, name_and_params)
@@ -114,11 +115,15 @@ with io.open("loplugin.unusedmethods.log", "rb", buffering=1024*1024) as txt:
             returnType = tokens[2]
             nameAndParams = tokens[3]
             sourceLocation = tokens[4]
+            virtual = ""
+            if len(tokens)>=6: virtual = tokens[5]
             funcInfo = (normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams))
             definitionSet.add(funcInfo)
             if access == "public":
                 publicDefinitionSet.add(funcInfo)
             definitionToSourceLocationMap[funcInfo] = sourceLocation
+            if virtual == "virtual":
+                virtualSet.add(funcInfo)
         elif tokens[0] == "call:":
             returnType = tokens[1]
             nameAndParams = tokens[2]
@@ -333,6 +338,8 @@ for d in publicDefinitionSet:
     method = d[0] + " " + d[1]
     if d in calledFromOutsideSet:
         continue
+    if d in virtualSet:
+        continue
     # TODO ignore constructors for now, my called-from-outside analysis doesn't work here
     if d[0] == "":
         continue