better solution for ignoreLocation for tree-wide plugins

Change-Id: I7336003e038781d4ef50380fa49f66b5ff19379f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135589
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

14 files changed, 42 insertions, 84 deletions

diff --git a/compilerplugins/clang/constantparam.cxx b/compilerplugins/clang/constantparam.cxx
index 5fd472954a9a..db82bc4df9ce 100644
--- a/compilerplugins/clang/constantparam.cxx
+++ b/compilerplugins/clang/constantparam.cxx
@@ -61,6 +61,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         // ignore some files that make clang crash inside EvaluateAsInt
         std::string fn(handler.getMainFileName());
         loplugin::normalizeDotDotInFilePath(fn);
diff --git a/compilerplugins/clang/expandablemethods.cxx b/compilerplugins/clang/expandablemethods.cxx
index 5b09345d414c..825c09705dd5 100644
--- a/compilerplugins/clang/expandablemethods.cxx
+++ b/compilerplugins/clang/expandablemethods.cxx
@@ -65,6 +65,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
diff --git a/compilerplugins/clang/finalclasses.cxx b/compilerplugins/clang/finalclasses.cxx
index e061bb7c5ef2..327ff92653c0 100644
--- a/compilerplugins/clang/finalclasses.cxx
+++ b/compilerplugins/clang/finalclasses.cxx
@@ -47,6 +47,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
diff --git a/compilerplugins/clang/mergeclasses.cxx b/compilerplugins/clang/mergeclasses.cxx
index a644677caf76..dae8724bd3bf 100644
--- a/compilerplugins/clang/mergeclasses.cxx
+++ b/compilerplugins/clang/mergeclasses.cxx
@@ -56,6 +56,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
diff --git a/compilerplugins/clang/methodcycles.cxx b/compilerplugins/clang/methodcycles.cxx
index 7b58a4bb52b6..24f4562e4b51 100644
--- a/compilerplugins/clang/methodcycles.cxx
+++ b/compilerplugins/clang/methodcycles.cxx
@@ -73,6 +73,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index 2856d5a0e378..7cfa19eb9c35 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -230,16 +230,21 @@ bool PluginHandler::ignoreLocation(SourceLocation loc) {
 
 bool PluginHandler::checkIgnoreLocation(SourceLocation loc)
 {
-    // If a location comes from a PCH, it is not necessary to check it
-    // in every compilation using the PCH, since with Clang we use
-    // -building-pch-with-obj to build a separate precompiled_foo.cxx file
-    // for the PCH, and so it is known that everything in the PCH will
-    // be checked while compiling this file. Skip the checks for all
-    // other files using the PCH.
-    if( !compiler.getSourceManager().isLocalSourceLocation( loc ))
+    // The tree-wide analysis plugins (like unusedmethods) don't want
+    // this logic, they only want to ignore externl code
+    if (!treeWideAnalysisMode)
     {
-        if( !compiler.getLangOpts().BuildingPCHWithObjectFile )
-            return true;
+        // If a location comes from a PCH, it is not necessary to check it
+        // in every compilation using the PCH, since with Clang we use
+        // -building-pch-with-obj to build a separate precompiled_foo.cxx file
+        // for the PCH, and so it is known that everything in the PCH will
+        // be checked while compiling this file. Skip the checks for all
+        // other files using the PCH.
+        if( !compiler.getSourceManager().isLocalSourceLocation( loc ))
+        {
+            if( !compiler.getLangOpts().BuildingPCHWithObjectFile )
+                return true;
+        }
     }
     SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
     if( compiler.getSourceManager().isInSystemHeader( expansionLoc ))
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index f7a399672c5d..e2ca4075d33d 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -68,6 +68,8 @@ public:
     // Is all code that could see `decl` defined in this TU?
     bool isAllRelevantCodeDefined(NamedDecl const * decl);
 
+    void enableTreeWideAnalysisMode() { treeWideAnalysisMode = true; }
+
 private:
     void handleOption( const std::string& option );
     void createPlugins( std::set< std::string > rewriters );
@@ -81,6 +83,8 @@ private:
     std::string warningsOnly;
     bool warningsAsErrors;
     bool debugMode = false;
+    //// Used by the tree-wide analysis plugins like unusedmethods, etc.
+    bool treeWideAnalysisMode = false;
     std::vector<std::pair<char const*, char const*>> mvModifiedRanges;
 
     // Used internally by isAllRelevantCodeDefined:
diff --git a/compilerplugins/clang/singlevalfields.cxx b/compilerplugins/clang/singlevalfields.cxx
index df5abcc007b6..8964d4fa3fde 100644
--- a/compilerplugins/clang/singlevalfields.cxx
+++ b/compilerplugins/clang/singlevalfields.cxx
@@ -94,6 +94,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         if (!isUnitTestMode())
diff --git a/compilerplugins/clang/unnecessaryvirtual.cxx b/compilerplugins/clang/unnecessaryvirtual.cxx
index c3c73923b3ae..ab8b63bd6f6b 100644
--- a/compilerplugins/clang/unnecessaryvirtual.cxx
+++ b/compilerplugins/clang/unnecessaryvirtual.cxx
@@ -62,6 +62,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
diff --git a/compilerplugins/clang/unusedenumconstants.cxx b/compilerplugins/clang/unusedenumconstants.cxx
index 5f91929a1431..692faa0fb5e9 100644
--- a/compilerplugins/clang/unusedenumconstants.cxx
+++ b/compilerplugins/clang/unusedenumconstants.cxx
@@ -67,6 +67,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         if (!isUnitTestMode())
diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx
index 24045483ffe1..dac13318649c 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -163,8 +163,6 @@ public:
 
 private:
     MyFieldInfo niceName(const FieldDecl*);
-    bool ignoreLocation(SourceLocation loc);
-    bool checkIgnoreLocation(SourceLocation loc);
     void checkTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr);
     void checkIfReadFrom(const FieldDecl* fieldDecl, const Expr* memberExpr);
     void checkIfWrittenTo(const FieldDecl* fieldDecl, const Expr* memberExpr);
@@ -184,6 +182,8 @@ private:
 
 void UnusedFields::run()
 {
+    handler.enableTreeWideAnalysisMode();
+
     TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
     if (!isUnitTestMode())
@@ -281,42 +281,6 @@ MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
     return aInfo;
 }
 
-/**
- * Our need to see everything conflicts with the PCH code in pluginhandler::ignoreLocation,
- * so we have to do this ourselves.
- */
-bool UnusedFields::ignoreLocation(SourceLocation loc)
-{
-    static std::unordered_map<SourceLocation, bool> checkedMap;
-    auto it = checkedMap.find(loc);
-    if (it != checkedMap.end())
-        return it->second;
-    bool ignore = checkIgnoreLocation(loc);
-    checkedMap.emplace(loc, ignore);
-    return ignore;
-}
-
-bool UnusedFields::checkIgnoreLocation(SourceLocation loc)
-{
-    // simplified form of the code in PluginHandler::checkIgnoreLocation
-    SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
-    if( compiler.getSourceManager().isInSystemHeader( expansionLoc ))
-        return true;
-    PresumedLoc presumedLoc = compiler.getSourceManager().getPresumedLoc( expansionLoc );
-    if( presumedLoc.isInvalid())
-        return true;
-    const char* bufferName = presumedLoc.getFilename();
-    if (bufferName == NULL
-        || loplugin::hasPathnamePrefix(bufferName, SRCDIR "/external/")
-        || loplugin::hasPathnamePrefix(bufferName, WORKDIR "/"))
-        return true;
-    if( loplugin::hasPathnamePrefix(bufferName, BUILDDIR "/")
-        || loplugin::hasPathnamePrefix(bufferName, SRCDIR "/") )
-        return false; // ok
-    return true;
-}
-
-
 bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl )
 {
     fieldDecl = fieldDecl->getCanonicalDecl();
diff --git a/compilerplugins/clang/unusedmethods.cxx b/compilerplugins/clang/unusedmethods.cxx
index 41b1317d4047..5b10eae7e9df 100644
--- a/compilerplugins/clang/unusedmethods.cxx
+++ b/compilerplugins/clang/unusedmethods.cxx
@@ -84,6 +84,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         StringRef fn(handler.getMainFileName());
         // ignore external code, makes this run faster
         if (fn.contains("UnpackedTarball"))
@@ -133,8 +135,6 @@ private:
     MyFuncInfo niceName(const FunctionDecl* functionDecl);
     std::string toString(SourceLocation loc);
     void functionTouchedFromExpr( const FunctionDecl* calleeFunctionDecl, const Expr* expr );
-    bool ignoreLocation(SourceLocation loc);
-    bool checkIgnoreLocation(SourceLocation loc);
 
     CXXRecordDecl const * currentCxxRecordDecl = nullptr;
     FunctionDecl const * currentFunctionDecl = nullptr;
@@ -197,41 +197,6 @@ MyFuncInfo UnusedMethods::niceName(const FunctionDecl* functionDecl)
     return aInfo;
 }
 
-/**
- * Our need to see everything conflicts with the PCH code in pluginhandler::ignoreLocation,
- * so we have to do this ourselves.
- */
-bool UnusedMethods::ignoreLocation(SourceLocation loc)
-{
-    static std::unordered_map<SourceLocation, bool> checkedMap;
-    auto it = checkedMap.find(loc);
-    if (it != checkedMap.end())
-        return it->second;
-    bool ignore = checkIgnoreLocation(loc);
-    checkedMap.emplace(loc, ignore);
-    return ignore;
-}
-
-bool UnusedMethods::checkIgnoreLocation(SourceLocation loc)
-{
-    // simplified form of the code in PluginHandler::checkIgnoreLocation
-    SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
-    if( compiler.getSourceManager().isInSystemHeader( expansionLoc ))
-        return true;
-    PresumedLoc presumedLoc = compiler.getSourceManager().getPresumedLoc( expansionLoc );
-    if( presumedLoc.isInvalid())
-        return true;
-    const char* bufferName = presumedLoc.getFilename();
-    if (bufferName == NULL
-        || loplugin::hasPathnamePrefix(bufferName, SRCDIR "/external/")
-        || loplugin::hasPathnamePrefix(bufferName, WORKDIR "/"))
-        return true;
-    if( loplugin::hasPathnamePrefix(bufferName, BUILDDIR "/")
-        || loplugin::hasPathnamePrefix(bufferName, SRCDIR "/") )
-        return false; // ok
-    return true;
-}
-
 std::string UnusedMethods::toString(SourceLocation loc)
 {
     SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
diff --git a/compilerplugins/clang/unusedvarsglobal.cxx b/compilerplugins/clang/unusedvarsglobal.cxx
index 50ac67d4059e..ebbe1ed06bf9 100644
--- a/compilerplugins/clang/unusedvarsglobal.cxx
+++ b/compilerplugins/clang/unusedvarsglobal.cxx
@@ -175,6 +175,8 @@ private:
 
 void UnusedVarsGlobal::run()
 {
+    handler.enableTreeWideAnalysisMode();
+
     TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
     if (!isUnitTestMode())
diff --git a/compilerplugins/clang/virtualdead.cxx b/compilerplugins/clang/virtualdead.cxx
index 1a63363aaaf2..e3cdf2b45018 100644
--- a/compilerplugins/clang/virtualdead.cxx
+++ b/compilerplugins/clang/virtualdead.cxx
@@ -68,6 +68,8 @@ public:
 
     virtual void run() override
     {
+        handler.enableTreeWideAnalysisMode();
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 
         // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes