some cleanups to the unusedmethods loplugin

- publicDefinitionSet is unnecessary, the python post-processor does not
need it
- remove the traversing of templates, clang will do that for us since we
have set the shouldVisitTemplateInstantiations() method

Change-Id: I0e96dad0b1cc941fe6c4a2e9227e86d8c3f1d85a

3 files changed, 47 insertions, 77 deletions

diff --git a/compilerplugins/clang/inlineablemethods.cxx b/compilerplugins/clang/inlineablemethods.cxx
index 3dd58d352c45..df98463a3547 100644
--- a/compilerplugins/clang/inlineablemethods.cxx
+++ b/compilerplugins/clang/inlineablemethods.cxx
@@ -325,7 +325,7 @@ bool InlineableMethods::isCalleeFunctionInteresting(const FunctionDecl* function
     return true;
 }
 
-loplugin::Plugin::Registration< InlineableMethods > X("inlineablemethods", true);
+loplugin::Plugin::Registration< InlineableMethods > X("inlineablemethods", false);
 
 }
 
diff --git a/compilerplugins/clang/unusedmethods.cxx b/compilerplugins/clang/unusedmethods.cxx
index 3c5498d79651..80af02484a32 100644
--- a/compilerplugins/clang/unusedmethods.cxx
+++ b/compilerplugins/clang/unusedmethods.cxx
@@ -12,6 +12,8 @@
 #include <iostream>
 #include <fstream>
 #include <set>
+#include <unordered_map>
+
 
 #include "clang/AST/Attr.h"
 
@@ -42,8 +44,6 @@ to auto-remove the method declarations
 Note that the actual process may involve a fair amount of undoing, hand editing, and general messing around
 to get it to work :-)
 
-TODO deal with calls to superclass/member constructors from other constructors, so
-     we can find unused constructors
 */
 
 namespace {
@@ -69,8 +69,7 @@ static std::set<MyFuncInfo> callSet;
 static std::set<MyFuncInfo> definitionSet;
 // for the "unused return type" analysis
 static std::set<MyFuncInfo> usedReturnSet;
-// for the "unnecessary public" analysis
-static std::set<MyFuncInfo> publicDefinitionSet;
+// for the "can be private" analysis
 static std::set<MyFuncInfo> calledFromOutsideSet;
 
 
@@ -96,7 +95,7 @@ public:
         // for the "unused return type" analysis
         for (const MyFuncInfo & s : usedReturnSet)
             output += "usedReturn:\t" + s.returnType + "\t" + s.nameAndParams + "\n";
-        // for the "unnecessary public" analysis
+        // for the "method can be private" analysis
         for (const MyFuncInfo & s : calledFromOutsideSet)
             output += "outside:\t" + s.returnType + "\t" + s.nameAndParams + "\n";
         ofstream myfile;
@@ -116,6 +115,8 @@ private:
     void logCallToRootMethods(const FunctionDecl* functionDecl, std::set<MyFuncInfo>& funcSet);
     MyFuncInfo niceName(const FunctionDecl* functionDecl);
     std::string fullyQualifiedName(const FunctionDecl* functionDecl);
+    std::string toString(SourceLocation loc);
+    void functionTouchedFromExpr( const FunctionDecl* calleeFunctionDecl, const Expr* expr );
 };
 
 MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl)
@@ -163,14 +164,20 @@ MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl)
         aInfo.nameAndParams += " const";
     }
 
-    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));
-    normalizeDotDotInFilePath(aInfo.sourceLocation);
+    aInfo.sourceLocation = toString( functionDecl->getLocation() );
 
     return aInfo;
 }
 
+std::string UnusedMethods::toString(SourceLocation loc)
+{
+    SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
+    StringRef name = compiler.getSourceManager().getFilename(expansionLoc);
+    std::string sourceLocation = std::string(name.substr(strlen(SRCDIR)+1)) + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc));
+    normalizeDotDotInFilePath(sourceLocation);
+    return sourceLocation;
+}
+
 std::string UnusedMethods::fullyQualifiedName(const FunctionDecl* functionDecl)
 {
     std::string ret = compat::getReturnType(*functionDecl).getCanonicalType().getAsString();
@@ -197,13 +204,13 @@ std::string UnusedMethods::fullyQualifiedName(const FunctionDecl* functionDecl)
     return ret;
 }
 
+// For virtual/overriding methods, we need to pretend we called the root method(s),
+// so that they get marked as used.
 void UnusedMethods::logCallToRootMethods(const FunctionDecl* functionDecl, std::set<MyFuncInfo>& funcSet)
 {
     functionDecl = functionDecl->getCanonicalDecl();
     bool bCalledSuperMethod = false;
     if (isa<CXXMethodDecl>(functionDecl)) {
-        // For virtual/overriding methods, we need to pretend we called the root method(s),
-        // so that they get marked as used.
         const CXXMethodDecl* methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
         for(CXXMethodDecl::method_iterator it = methodDecl->begin_overridden_methods();
             it != methodDecl->end_overridden_methods(); ++it)
@@ -222,9 +229,6 @@ void UnusedMethods::logCallToRootMethods(const FunctionDecl* functionDecl, std::
     }
 }
 
-// prevent recursive templates from blowing up the stack
-static std::set<std::string> traversedFunctionSet;
-
 bool UnusedMethods::VisitCallExpr(CallExpr* expr)
 {
     // Note that I don't ignore ANYTHING here, because I want to get calls to my code that result
@@ -247,18 +251,11 @@ bool UnusedMethods::VisitCallExpr(CallExpr* expr)
     }
 
 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);
-
     logCallToRootMethods(calleeFunctionDecl, callSet);
 
     const Stmt* parent = parentStmt(expr);
 
-    // Now do the checks necessary for the "unnecessary public" analysis
+    // Now do the checks necessary for the "can be private" analysis
     CXXMethodDecl* calleeMethodDecl = dyn_cast<CXXMethodDecl>(calleeFunctionDecl);
     if (calleeMethodDecl && calleeMethodDecl->getAccess() == AS_public)
     {
@@ -304,6 +301,10 @@ bool UnusedMethods::VisitCXXConstructExpr( const CXXConstructExpr* constructExpr
     const CXXConstructorDecl* constructorDecl = constructExpr->getConstructor();
     constructorDecl = constructorDecl->getCanonicalDecl();
 
+    if (!constructorDecl->getLocation().isValid() || ignoreLocation(constructorDecl)) {
+        return true;
+    }
+
     logCallToRootMethods(constructorDecl, callSet);
 
     return true;
@@ -311,16 +312,10 @@ bool UnusedMethods::VisitCXXConstructExpr( const CXXConstructExpr* constructExpr
 
 bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl )
 {
-    functionDecl = functionDecl->getCanonicalDecl();
-    const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(functionDecl);
-
-    // ignore method overrides, since the call will show up as being directed to the root method
-    if (methodDecl && (methodDecl->size_overridden_methods() != 0 || methodDecl->hasAttr<OverrideAttr>())) {
-        return true;
-    }
+    const FunctionDecl* canonicalFunctionDecl = functionDecl->getCanonicalDecl();
     // ignore stuff that forms part of the stable URE interface
     if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(
-                              functionDecl->getCanonicalDecl()->getNameInfo().getLoc()))) {
+                              canonicalFunctionDecl->getNameInfo().getLoc()))) {
         return true;
     }
     if (isa<CXXDestructorDecl>(functionDecl)) {
@@ -332,15 +327,17 @@ bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl )
     if (isa<CXXConstructorDecl>(functionDecl) && dyn_cast<CXXConstructorDecl>(functionDecl)->isCopyConstructor()) {
         return true;
     }
-
-    if( functionDecl->getLocation().isValid() && !ignoreLocation( functionDecl )
-        && !functionDecl->isExternC())
-    {
-        MyFuncInfo funcInfo = niceName(functionDecl);
+    if (!canonicalFunctionDecl->getLocation().isValid() || ignoreLocation(canonicalFunctionDecl)) {
+        return true;
+    }
+    // ignore method overrides, since the call will show up as being directed to the root method
+    const CXXMethodDecl* methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
+    if (methodDecl && (methodDecl->size_overridden_methods() != 0 || methodDecl->hasAttr<OverrideAttr>())) {
+        return true;
+    }
+    if (!functionDecl->isExternC()) {
+        MyFuncInfo funcInfo = niceName(canonicalFunctionDecl);
         definitionSet.insert(funcInfo);
-        if (functionDecl->getAccess() == AS_public) {
-            publicDefinitionSet.insert(funcInfo);
-        }
     }
     return true;
 }
@@ -348,12 +345,12 @@ bool UnusedMethods::VisitFunctionDecl( const FunctionDecl* functionDecl )
 // this catches places that take the address of a method
 bool UnusedMethods::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
 {
-    const Decl* functionDecl = declRefExpr->getDecl();
-    if (!isa<FunctionDecl>(functionDecl)) {
+    const FunctionDecl* functionDecl = dyn_cast<FunctionDecl>(declRefExpr->getDecl());
+    if (!functionDecl) {
         return true;
     }
-    logCallToRootMethods(dyn_cast<FunctionDecl>(functionDecl)->getCanonicalDecl(), callSet);
-    logCallToRootMethods(dyn_cast<FunctionDecl>(functionDecl)->getCanonicalDecl(), usedReturnSet);
+    logCallToRootMethods(functionDecl->getCanonicalDecl(), callSet);
+    logCallToRootMethods(functionDecl->getCanonicalDecl(), usedReturnSet);
     return true;
 }
 
diff --git a/compilerplugins/clang/unusedmethods.py b/compilerplugins/clang/unusedmethods.py
index 41044d37f5ef..1e2a67424dd6 100755
--- a/compilerplugins/clang/unusedmethods.py
+++ b/compilerplugins/clang/unusedmethods.py
@@ -14,7 +14,7 @@ definitionToSourceLocationMap = dict()
 # for the "unused methods" analysis
 callSet = set() # set of tuple(return_type, name_and_params)
 
-# for the "unnecessary public" analysis
+# 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)
 
@@ -25,23 +25,6 @@ usedReturnSet = set() # set of tuple(return_type, name_and_params)
 # things we need to exclude for reasons like :
 # - it's a weird template thingy that confuses the plugin
 unusedMethodsExclusionSet = set([
-    "double basegfx::DoubleTraits::maxVal()",
-    "double basegfx::DoubleTraits::minVal()",
-    "double basegfx::DoubleTraits::neutral()",
-    "int basegfx::Int32Traits::maxVal()",
-    "int basegfx::Int32Traits::minVal()",
-    "int basegfx::Int32Traits::neutral()",
-    "unsigned long UniqueIndexImpl::Insert(void *)",
-    "class XMLPropertyBackpatcher<short> & XMLTextImportHelper::GetFootnoteBP()",
-    "class XMLPropertyBackpatcher<short> & XMLTextImportHelper::GetSequenceIdBP()",
-    "void XclExpPivotCache::SaveXml(class XclExpXmlStream &)",
-
-
-    # TODO track instantiations of template class constructors
-    "void comphelper::IEventProcessor::release()",
-    "void SotMutexHolder::acquire()",
-    "void SotMutexHolder::release()",
-
     # only used by Windows build
     "_Bool basegfx::B2ITuple::equalZero() const",
     "class basegfx::B2DPolyPolygon basegfx::unotools::UnoPolyPolygon::getPolyPolygonUnsafe() const",
@@ -72,10 +55,6 @@ unusedMethodsExclusionSet = set([
     "_Bool ScImportExport::ImportData(const class rtl::OUString &,const class com::sun::star::uno::Any &)",
 	"void* ScannerManager::GetData()",
 	"void ScannerManager::SetData(void *)",
-    # instantiated from templates, not sure why it is not being picked up
-    "class basegfx::B2DPolygon OutputDevice::PixelToLogic(const class basegfx::B2DPolygon &,const class MapMode &) const",
-    "type-parameter-0-0 * detail::cloner::clone(type-parameter-0-0 *const)",
-    "const class rtl::OUString writerperfect::DocumentHandlerFor::name()",
     # only used by OSX build
     "void StyleSettings::SetHideDisabledMenuItems(_Bool)",
     # debugging methods
@@ -83,12 +62,6 @@ unusedMethodsExclusionSet = set([
     "void oox::PropertyMap::dumpCode(class com::sun::star::uno::Reference<class com::sun::star::beans::XPropertySet>)",
     "void oox::PropertyMap::dumpData(class com::sun::star::uno::Reference<class com::sun::star::beans::XPropertySet>)",
     "class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char> > writerfilter::ooxml::OOXMLPropertySet::toString()",
-    # deep template magic in SW
-    "Ring<value_type> * sw::Ring::Ring_node_traits::get_next(const Ring<value_type> *)",
-    "Ring<value_type> * sw::Ring::Ring_node_traits::get_previous(const Ring<value_type> *)",
-    "void sw::Ring::Ring_node_traits::set_next(Ring<value_type> *,Ring<value_type> *)",
-    "void sw::Ring::Ring_node_traits::set_previous(Ring<value_type> *,Ring<value_type> *)",
-    "type-parameter-0-0 checking_cast(type-parameter-0-0,type-parameter-0-0)",
     # I need to teach the plugin that for loops with range expressions call begin() and end()
     "class __gnu_debug::_Safe_iterator<class __gnu_cxx::__normal_iterator<class SwAnchoredObject *const *, class std::__cxx1998::vector<class SwAnchoredObject *, class std::allocator<class SwAnchoredObject *> > >, class std::__debug::vector<class SwAnchoredObject *, class std::allocator<class SwAnchoredObject *> > > SwSortedObjs::begin() const",
     "class __gnu_debug::_Safe_iterator<class __gnu_cxx::__normal_iterator<class SwAnchoredObject *const *, class std::__cxx1998::vector<class SwAnchoredObject *, class std::allocator<class SwAnchoredObject *> > >, class std::__debug::vector<class SwAnchoredObject *, class std::allocator<class SwAnchoredObject *> > > SwSortedObjs::end() const",
@@ -158,6 +131,8 @@ with io.open("loplugin.unusedmethods.log", "rb", buffering=1024*1024) as txt:
             returnType = tokens[1]
             nameAndParams = tokens[2]
             calledFromOutsideSet.add((normalizeTypeParams(returnType), normalizeTypeParams(nameAndParams)))
+        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
@@ -283,7 +258,7 @@ for d in definitionSet:
     tmp1set.add((method, location))
 
 # print out the results, sorted by name and line number
-with open("loplugin.unusedmethods.report-methods", "wt") as f:
+with open("loplugin.unusedmethods.report-unused-methods", "wt") as f:
     for t in sort_set_by_natural_key(tmp1set):
         f.write(t[1] + "\n")
         f.write("    " + t[0] + "\n")
@@ -343,14 +318,14 @@ for d in definitionSet:
     tmp2set.add((method, location))
 
 # print output, sorted by name and line number
-with open("loplugin.unusedmethods.report-returns", "wt") as f:
+with open("loplugin.unusedmethods.report-unused-returns", "wt") as f:
     for t in sort_set_by_natural_key(tmp2set):
         f.write(t[1] + "\n")
         f.write("    " +  t[0] + "\n")
 
 
 # --------------------------------------------------------------------------------------------
-# "unnecessary public" analysis
+# "method can be private" analysis
 # --------------------------------------------------------------------------------------------
 
 tmp3set = set()
@@ -366,10 +341,8 @@ for d in publicDefinitionSet:
     tmp3set.add((method, definitionToSourceLocationMap[d]))
 
 # print output, sorted by name and line number
-with open("loplugin.unusedmethods.report-public", "wt") as f:
+with open("loplugin.unusedmethods.report-can-be-private", "wt") as f:
     for t in sort_set_by_natural_key(tmp3set):
         f.write(t[1] + "\n")
         f.write("    " + t[0] + "\n")
 
-        
-