[Libreoffice-commits] core.git: compilerplugins/clang

Stephan Bergmann sbergman at redhat.com
Tue Nov 7 10:27:22 UTC 2017


 compilerplugins/clang/plugin.cxx        |   43 ----------------------
 compilerplugins/clang/plugin.hxx        |   38 +++++++++++---------
 compilerplugins/clang/pluginhandler.cxx |   60 +++++++++++++++++++++++++++++---
 compilerplugins/clang/pluginhandler.hxx |   26 ++++++++++++-
 4 files changed, 100 insertions(+), 67 deletions(-)

New commits:
commit 07b8711526972e120824d0fb913b01b1baf6a4cb
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Tue Nov 7 11:19:30 2017 +0100

    Memoize ignoreLocation results
    
    ...which, according to callgrind, reduces instruction fetch count spent on
    compiling sw/source/core/layout/paintfrm.cxx (randomly selected because it is
    rather large) by 5% from 41,992,064,226 to 39,861,989,855 (function main() in
    clang-6.0).
    
    This is best done by forwarding ignoreLocation calls from Plugin to the
    PluginHandler signleton, but due to the tight mutual coupling between plugin.hxx
    and pluginhandler.hxx that unfortunately required some reorganization (and two
    outstanding TODO clean-ups of temporarily introduced using declarations in
    plugin.hxx).
    
    Change-Id: Ia4270517d194def7db7ed80cb6894e9c473e9499

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 7d53e71dfd61..f8292ef661f6 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -36,48 +36,7 @@ DiagnosticBuilder Plugin::report( DiagnosticsEngine::Level level, StringRef mess
     return handler.report( level, name, message, compiler, loc );
 }
 
-bool Plugin::ignoreLocation( SourceLocation loc )
-{
-    SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
-    if( compiler.getSourceManager().isInSystemHeader( expansionLoc ))
-        return true;
-    const char* bufferName = compiler.getSourceManager().getPresumedLoc( expansionLoc ).getFilename();
-    if (bufferName == NULL
-        || hasPathnamePrefix(bufferName, SRCDIR "/external/")
-        || isSamePathname(bufferName, SRCDIR "/sdext/source/pdfimport/wrapper/keyword_list") )
-            // workdir/CustomTarget/sdext/pdfimport/hash.cxx is generated from
-            // sdext/source/pdfimport/wrapper/keyword_list by gperf, which
-            // inserts various #line directives denoting the latter into the
-            // former, but fails to add a #line directive returning back to
-            // hash.cxx itself before the gperf generated boilerplate, so
-            // compilers erroneously consider errors in the boilerplate to come
-            // from keyword_list instead of hash.cxx (for Clang on Linux/macOS
-            // this is not an issue due to the '#pragma GCC system_header'
-            // generated into the start of hash.cxx, #if'ed for __GNUC__, but
-            // for clang-cl it is an issue)
-        return true;
-    if( hasPathnamePrefix(bufferName, WORKDIR) )
-    {
-        // workdir/CustomTarget/vcl/unx/kde4/tst_exclude_socket_notifiers.moc
-        // includes
-        // "../../../../../vcl/unx/kde4/tst_exclude_socket_notifiers.hxx",
-        // making the latter file erroneously match here; so strip any ".."
-        // segments:
-        if (strstr(bufferName, "/..") == nullptr) {
-            return true;
-        }
-        std::string s(bufferName);
-        normalizeDotDotInFilePath(s);
-        if (hasPathnamePrefix(s, WORKDIR))
-            return true;
-    }
-    if( hasPathnamePrefix(bufferName, BUILDDIR)
-        || hasPathnamePrefix(bufferName, SRCDIR) )
-        return false; // ok
-    return true;
-}
-
-void Plugin::normalizeDotDotInFilePath( std::string & s )
+void normalizeDotDotInFilePath( std::string & s )
 {
     for (std::string::size_type i = 0;;)
     {
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 89d9999255df..0b9da5e6e9a5 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -24,13 +24,21 @@
 
 #include <clang/Rewrite/Core/Rewriter.h>
 
+#include "pluginhandler.hxx"
+
 using namespace clang;
 using namespace llvm;
 
 namespace loplugin
 {
 
-class PluginHandler;
+struct InstantiationData
+{
+    const char* name;
+    PluginHandler& handler;
+    CompilerInstance& compiler;
+    Rewriter* rewriter;
+};
 
 /**
     Base class for plugins.
@@ -41,14 +49,6 @@ class PluginHandler;
 class Plugin
 {
 public:
-    struct InstantiationData
-    {
-        const char* name;
-        PluginHandler& handler;
-        CompilerInstance& compiler;
-        Rewriter* rewriter;
-    };
-
     explicit Plugin( const InstantiationData& data );
     virtual ~Plugin() {}
     virtual void run() = 0;
@@ -58,9 +58,10 @@ public:
     SourceLocation locationAfterToken( SourceLocation location );
 protected:
     DiagnosticBuilder report( DiagnosticsEngine::Level level, StringRef message, SourceLocation loc = SourceLocation()) const;
-    bool ignoreLocation( SourceLocation loc );
-    bool ignoreLocation( const Decl* decl );
-    bool ignoreLocation( const Stmt* stmt );
+    bool ignoreLocation( SourceLocation loc ) const
+    { return handler.ignoreLocation(loc); }
+    bool ignoreLocation( const Decl* decl ) const;
+    bool ignoreLocation( const Stmt* stmt ) const;
     CompilerInstance& compiler;
     PluginHandler& handler;
     /**
@@ -77,8 +78,6 @@ protected:
     bool isInUnoIncludeFile(SourceLocation spellingLocation) const;
     bool isInUnoIncludeFile(const FunctionDecl*) const;
 
-    static void normalizeDotDotInFilePath(std::string&);
-
     static bool isUnitTestMode();
 
     bool containsPreprocessingConditionalInclusion(SourceRange range);
@@ -170,17 +169,17 @@ public:
 class RegistrationCreate
 {
 public:
-    template< typename T, bool > static T* create( const Plugin::InstantiationData& data );
+    template< typename T, bool > static T* create( const InstantiationData& data );
 };
 
 inline
-bool Plugin::ignoreLocation( const Decl* decl )
+bool Plugin::ignoreLocation( const Decl* decl ) const
 {
     return ignoreLocation( decl->getLocation());
 }
 
 inline
-bool Plugin::ignoreLocation( const Stmt* stmt )
+bool Plugin::ignoreLocation( const Stmt* stmt ) const
 {
     // Invalid location can happen at least for ImplicitCastExpr of
     // ImplicitParam 'self' in Objective C method declarations:
@@ -215,6 +214,8 @@ RewritePlugin::RewriteOption operator|( RewritePlugin::RewriteOption option1, Re
     return static_cast< RewritePlugin::RewriteOption >( int( option1 ) | int( option2 ));
 }
 
+void normalizeDotDotInFilePath(std::string&);
+
 // Same as pathname.startswith(prefix), except on Windows, where pathname and
 // prefix may also contain backslashes:
 bool hasPathnamePrefix(StringRef pathname, StringRef prefix);
@@ -225,6 +226,9 @@ bool isSamePathname(StringRef pathname, StringRef other);
 
 } // namespace
 
+using loplugin::InstantiationData; //TODO
+using loplugin::normalizeDotDotInFilePath; //TODO
+
 #endif // COMPILEPLUGIN_H
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index d82a369cb119..ff1c6b6922cb 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -11,6 +11,7 @@
 
 #include <memory>
 #include "compat.hxx"
+#include "plugin.hxx"
 #include "pluginhandler.hxx"
 
 #include <clang/Frontend/CompilerInstance.h>
@@ -40,7 +41,7 @@ namespace loplugin
 
 struct PluginData
 {
-    Plugin* (*create)( const Plugin::InstantiationData& );
+    Plugin* (*create)( const InstantiationData& );
     Plugin* object;
     const char* optionName;
     bool isPPCallback;
@@ -126,17 +127,17 @@ void PluginHandler::createPlugins( std::set< std::string > rewriters )
         if (unitTestMode && mainFileName.find(plugins[ i ].optionName) == StringRef::npos)
             continue;
         if( rewriters.erase( name ) != 0 )
-            plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { name, *this, compiler, &rewriter } );
+            plugins[ i ].object = plugins[ i ].create( InstantiationData { name, *this, compiler, &rewriter } );
         else if( plugins[ i ].byDefault )
-            plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { name, *this, compiler, NULL } );
+            plugins[ i ].object = plugins[ i ].create( InstantiationData { name, *this, compiler, NULL } );
         else if( unitTestMode && strcmp(name, "unusedmethodsremove") != 0 && strcmp(name, "unusedfieldsremove") != 0)
-            plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { name, *this, compiler, NULL } );
+            plugins[ i ].object = plugins[ i ].create( InstantiationData { name, *this, compiler, NULL } );
     }
     for( auto r: rewriters )
         report( DiagnosticsEngine::Fatal, "unknown plugin tool %0" ) << r;
 }
 
-void PluginHandler::registerPlugin( Plugin* (*create)( const Plugin::InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault )
+void PluginHandler::registerPlugin( Plugin* (*create)( const InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault )
 {
     assert( !bPluginObjectsCreated );
     assert( pluginCount < MAX_PLUGINS );
@@ -175,6 +176,55 @@ DiagnosticBuilder PluginHandler::report( DiagnosticsEngine::Level level, StringR
     return report( level, nullptr, message, compiler, loc );
 }
 
+bool PluginHandler::ignoreLocation(SourceLocation loc) {
+    auto i = ignored_.find(loc);
+    if (i == ignored_.end()) {
+        i = ignored_.emplace(loc, checkIgnoreLocation(loc)).first;
+    }
+    return i->second;
+}
+
+bool PluginHandler::checkIgnoreLocation(SourceLocation loc)
+{
+    SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
+    if( compiler.getSourceManager().isInSystemHeader( expansionLoc ))
+        return true;
+    const char* bufferName = compiler.getSourceManager().getPresumedLoc( expansionLoc ).getFilename();
+    if (bufferName == NULL
+        || hasPathnamePrefix(bufferName, SRCDIR "/external/")
+        || isSamePathname(bufferName, SRCDIR "/sdext/source/pdfimport/wrapper/keyword_list") )
+            // workdir/CustomTarget/sdext/pdfimport/hash.cxx is generated from
+            // sdext/source/pdfimport/wrapper/keyword_list by gperf, which
+            // inserts various #line directives denoting the latter into the
+            // former, but fails to add a #line directive returning back to
+            // hash.cxx itself before the gperf generated boilerplate, so
+            // compilers erroneously consider errors in the boilerplate to come
+            // from keyword_list instead of hash.cxx (for Clang on Linux/macOS
+            // this is not an issue due to the '#pragma GCC system_header'
+            // generated into the start of hash.cxx, #if'ed for __GNUC__, but
+            // for clang-cl it is an issue)
+        return true;
+    if( hasPathnamePrefix(bufferName, WORKDIR) )
+    {
+        // workdir/CustomTarget/vcl/unx/kde4/tst_exclude_socket_notifiers.moc
+        // includes
+        // "../../../../../vcl/unx/kde4/tst_exclude_socket_notifiers.hxx",
+        // making the latter file erroneously match here; so strip any ".."
+        // segments:
+        if (strstr(bufferName, "/..") == nullptr) {
+            return true;
+        }
+        std::string s(bufferName);
+        normalizeDotDotInFilePath(s);
+        if (hasPathnamePrefix(s, WORKDIR))
+            return true;
+    }
+    if( hasPathnamePrefix(bufferName, BUILDDIR)
+        || hasPathnamePrefix(bufferName, SRCDIR) )
+        return false; // ok
+    return true;
+}
+
 bool PluginHandler::addRemoval( SourceLocation loc )
 {
     return removals.insert( loc ).second;
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index 63210fa11df4..bd49f7cfeb5e 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -12,17 +12,34 @@
 #ifndef PLUGINHANDLER_H
 #define PLUGINHANDLER_H
 
+#include <cstddef>
+#include <functional>
 #include <memory>
-#include "plugin.hxx"
-
 #include <set>
+#include <unordered_map>
 
 #include <clang/AST/ASTConsumer.h>
+#include <clang/Frontend/CompilerInstance.h>
 #include <clang/Frontend/FrontendAction.h>
+#include <clang/Rewrite/Core/Rewriter.h>
+
+using namespace clang;
+
+namespace std {
+
+template<> struct hash<::clang::SourceLocation> {
+    size_t operator ()(::clang::SourceLocation loc) const
+    { return loc.getRawEncoding(); }
+};
+
+}
 
 namespace loplugin
 {
 
+class Plugin;
+struct InstantiationData;
+
 /**
  Class that manages all LO modules.
 */
@@ -33,17 +50,20 @@ public:
     PluginHandler( CompilerInstance& compiler, const std::vector< std::string >& args );
     virtual ~PluginHandler();
     virtual void HandleTranslationUnit( ASTContext& context ) override;
-    static void registerPlugin( Plugin* (*create)( const Plugin::InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault );
+    static void registerPlugin( Plugin* (*create)( const InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault );
     DiagnosticBuilder report( DiagnosticsEngine::Level level, const char * plugin, StringRef message,
             CompilerInstance& compiler, SourceLocation loc = SourceLocation());
+    bool ignoreLocation(SourceLocation loc);
     bool addRemoval( SourceLocation loc );
     static bool isUnitTestMode();
 private:
     void handleOption( const std::string& option );
     void createPlugins( std::set< std::string > rewriters );
     DiagnosticBuilder report( DiagnosticsEngine::Level level, StringRef message, SourceLocation loc = SourceLocation());
+    bool checkIgnoreLocation(SourceLocation loc);
     CompilerInstance& compiler;
     StringRef const mainFileName;
+    std::unordered_map<SourceLocation, bool> ignored_;
     Rewriter rewriter;
     std::set< SourceLocation > removals;
     std::string scope;


More information about the Libreoffice-commits mailing list