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

Stephan Bergmann sbergman at redhat.com
Fri Mar 23 12:02:48 UTC 2018


 compilerplugins/clang/plugin.cxx        |   14 +-------------
 compilerplugins/clang/plugin.hxx        |    4 ----
 compilerplugins/clang/pluginhandler.cxx |   16 +---------------
 compilerplugins/clang/pluginhandler.hxx |    2 --
 4 files changed, 2 insertions(+), 34 deletions(-)

New commits:
commit 119d8137695e38c16e9fad9f3ce8a774f58e4b9a
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Fri Mar 23 10:49:57 2018 +0100

    Revert "Allow compiler plugins for online"
    
    This reverts commit b39e627be45f847554f11fdac040b6f4da4054ba.  The assumed (see
    comment at <https://gerrit.libreoffice.org/#/c/46769/4/compilerplugins/clang/
    plugin.cxx at 633>) performance bottleneck of isSamePathname -> getAbsolutePath
    does show up in Flamegraph inspections of LO builds.  But changing (non-member
    function) isSamePathname to only call getAbsolutePath if (PluginHandlder member
    function) isLOOLMode is true would need some code reorg, and Online development
    doesn't seem to make too much actual use of the plugin, so conclusion on IRC was
    to revert.
    
    Change-Id: I0f04fdcc87087dac516630ed5e48361f5ea332ca
    Reviewed-on: https://gerrit.libreoffice.org/51774
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 07f1edecf4c7..56d40e337bf9 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -18,8 +18,6 @@
 #include <clang/Basic/FileManager.h>
 #include <clang/Lex/Lexer.h>
 
-#include <llvm/Support/Path.h>
-
 #include "pluginhandler.hxx"
 
 /*
@@ -653,20 +651,10 @@ bool hasPathnamePrefix(StringRef pathname, StringRef prefix)
         [](StringRef p, StringRef a) { return p.startswith(a); });
 }
 
-std::string getAbsolutePath(StringRef path)
-{
-    llvm::SmallString<1024> absPath(path);
-    llvm::sys::fs::make_absolute(absPath);
-    llvm::sys::path::remove_dots(absPath, true);
-    return absPath.str().str();
-}
-
 bool isSamePathname(StringRef pathname, StringRef other)
 {
-    std::string absPathname = getAbsolutePath(pathname);
-    std::string absOther = getAbsolutePath(other);
     return checkPathname(
-        absPathname, absOther, [](StringRef p, StringRef a) { return p == a; });
+        pathname, other, [](StringRef p, StringRef a) { return p == a; });
 }
 
 } // namespace
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 95983c00060d..5299e7f2eac3 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -80,7 +80,6 @@ protected:
     bool isInUnoIncludeFile(const FunctionDecl*) const;
 
     bool isDebugMode() const { return handler.isDebugMode(); }
-    bool isLOOLMode() const { return handler.isLOOLMode(); }
 
     static bool isUnitTestMode();
 
@@ -230,9 +229,6 @@ void normalizeDotDotInFilePath(std::string&);
 // prefix may also contain backslashes:
 bool hasPathnamePrefix(StringRef pathname, StringRef prefix);
 
-// get the absolute path for a given path
-std::string getAbsolutePath(StringRef path);
-
 // Same as pathname == other, except on Windows, where pathname and other may
 // also contain backslashes:
 bool isSamePathname(StringRef pathname, StringRef other);
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index 959941fb72a9..3f169972f6a2 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -18,7 +18,6 @@
 #include <clang/Frontend/CompilerInstance.h>
 #include <clang/Frontend/FrontendPluginRegistry.h>
 #include <clang/Lex/PPCallbacks.h>
-
 #include <stdio.h>
 
 #if defined _WIN32
@@ -117,8 +116,6 @@ void PluginHandler::handleOption( const std::string& option )
         unitTestMode = true;
     else if (option == "debug")
         debugMode = true;
-    else if ( option.substr(0, 15) == "lool-base-path=" )
-        loolBasePath = option.substr(15);
     else
         report( DiagnosticsEngine::Fatal, "unknown option %0" ) << option;
 }
@@ -196,7 +193,7 @@ bool PluginHandler::checkIgnoreLocation(SourceLocation loc)
     if( compiler.getSourceManager().isInSystemHeader( expansionLoc ))
         return true;
     const char* bufferName = compiler.getSourceManager().getPresumedLoc( expansionLoc ).getFilename();
-    if (bufferName == nullptr
+    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
@@ -225,11 +222,6 @@ bool PluginHandler::checkIgnoreLocation(SourceLocation loc)
         if (hasPathnamePrefix(s, WORKDIR))
             return true;
     }
-    if ( isLOOLMode() ) {
-        std::string absPath = getAbsolutePath(bufferName);
-        if ( StringRef(absPath).startswith(loolBasePath) )
-            return false;
-    }
     if( hasPathnamePrefix(bufferName, BUILDDIR)
         || hasPathnamePrefix(bufferName, SRCDIR) )
         return false; // ok
@@ -305,12 +297,6 @@ void PluginHandler::HandleTranslationUnit( ASTContext& context )
             pathWarning = "modified source in build dir : %0";
         else if( name.startswith(SRCDIR "/") )
             ; // ok
-        else if ( isLOOLMode() )
-        {
-            std::string absPath = getAbsolutePath(name);
-            if ( !StringRef(absPath).startswith(loolBasePath) )
-                bSkip = true;
-        }
         else
         {
             pathWarning = "modified source in unknown location, not modifying : %0";
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index 890b0cf53c3d..0b53d7043f00 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -55,7 +55,6 @@ public:
             CompilerInstance& compiler, SourceLocation loc = SourceLocation());
     bool ignoreLocation(SourceLocation loc);
     bool isDebugMode() const { return debugMode; }
-    bool isLOOLMode() const { return !loolBasePath.empty(); }
     static bool isUnitTestMode();
     // If we overlap with a previous area we modified, we cannot perform this change
     // without corrupting the source
@@ -72,7 +71,6 @@ private:
     Rewriter rewriter;
     std::string scope;
     std::string warningsOnly;
-    std::string loolBasePath;
     bool warningsAsErrors;
     bool debugMode = false;
     std::vector<std::pair<char const*, char const*>> mvModifiedRanges;


More information about the Libreoffice-commits mailing list