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

Noel Grandin noel.grandin at collabora.co.uk
Mon Feb 12 08:36:57 UTC 2018


 compilerplugins/clang/changetoolsgen.cxx |  103 ++++++++++++++++++++-----------
 compilerplugins/clang/plugin.cxx         |   40 +++++++++---
 compilerplugins/clang/pluginhandler.cxx  |    9 ++
 compilerplugins/clang/pluginhandler.hxx  |    2 
 4 files changed, 109 insertions(+), 45 deletions(-)

New commits:
commit 288447449d7c43857f0bee7c3b9325c87e950143
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Mon Feb 12 10:28:14 2018 +0200

    rename loplugin changerectanglegetref to changetoolsgen
    
    Change-Id: I378f64ac0879d4c6ea574b1674e96ffb9cc89732

diff --git a/compilerplugins/clang/changerectanglegetref.cxx b/compilerplugins/clang/changetoolsgen.cxx
similarity index 84%
rename from compilerplugins/clang/changerectanglegetref.cxx
rename to compilerplugins/clang/changetoolsgen.cxx
index f687693ee8cb..93856ca311f6 100644
--- a/compilerplugins/clang/changerectanglegetref.cxx
+++ b/compilerplugins/clang/changetoolsgen.cxx
@@ -14,21 +14,20 @@
 #include <regex>
 
 /**
- * Changes calls to tools::Rectangle methods that return a ref to instead call the setter methods.
+ * Changes calls to tools::Rectangle/Point/Size methods that return a ref to instead call the setter methods.
  *
  * run as:
- *   make COMPILER_PLUGIN_TOOL=changerectanglegetref UPDATE_FILES=all FORCE_COMPILE_ALL=1
+ *   make COMPILER_PLUGIN_TOOL=changetoolsgen UPDATE_FILES=all FORCE_COMPILE_ALL=1
  * or
- *   make <module> COMPILER_PLUGIN_TOOL=changerectanglegetref FORCE_COMPILE_ALL=1
+ *   make <module> COMPILER_PLUGIN_TOOL=changetoolsgen FORCE_COMPILE_ALL=1
  */
 
 namespace
 {
-class ChangeRectangleGetRef : public RecursiveASTVisitor<ChangeRectangleGetRef>,
-                              public loplugin::RewritePlugin
+class ChangeToolsGen : public RecursiveASTVisitor<ChangeToolsGen>, public loplugin::RewritePlugin
 {
 public:
-    explicit ChangeRectangleGetRef(loplugin::InstantiationData const& data)
+    explicit ChangeToolsGen(loplugin::InstantiationData const& data)
         : RewritePlugin(data)
     {
     }
@@ -45,12 +44,9 @@ private:
     std::string extractCode(SourceLocation startLoc, SourceLocation endLoc);
 };
 
-void ChangeRectangleGetRef::run()
-{
-    TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
-}
+void ChangeToolsGen::run() { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
 
-bool ChangeRectangleGetRef::VisitCXXMemberCallExpr(CXXMemberCallExpr const* call)
+bool ChangeToolsGen::VisitCXXMemberCallExpr(CXXMemberCallExpr const* call)
 {
     if (ignoreLocation(call))
         return true;
@@ -140,8 +136,8 @@ bool ChangeRectangleGetRef::VisitCXXMemberCallExpr(CXXMemberCallExpr const* call
     return true;
 }
 
-bool ChangeRectangleGetRef::ChangeAssignment(Stmt const* parent, std::string const& methodName,
-                                             std::string const& setPrefix)
+bool ChangeToolsGen::ChangeAssignment(Stmt const* parent, std::string const& methodName,
+                                      std::string const& setPrefix)
 {
     // Look for expressions like
     //    aRect.Left() = ...;
@@ -167,10 +163,10 @@ bool ChangeRectangleGetRef::ChangeAssignment(Stmt const* parent, std::string con
     return replaceText(startLoc, originalLength, newText);
 }
 
-bool ChangeRectangleGetRef::ChangeBinaryOperator(BinaryOperator const* binaryOp,
-                                                 CXXMemberCallExpr const* call,
-                                                 std::string const& methodName,
-                                                 std::string const& setPrefix)
+bool ChangeToolsGen::ChangeBinaryOperator(BinaryOperator const* binaryOp,
+                                          CXXMemberCallExpr const* call,
+                                          std::string const& methodName,
+                                          std::string const& setPrefix)
 {
     // Look for expressions like
     //    aRect.Left() += ...;
@@ -231,10 +227,10 @@ bool ChangeRectangleGetRef::ChangeBinaryOperator(BinaryOperator const* binaryOp,
     return replaceText(startLoc, originalLength, newText);
 }
 
-bool ChangeRectangleGetRef::ChangeUnaryOperator(UnaryOperator const* unaryOp,
-                                                CXXMemberCallExpr const* call,
-                                                std::string const& methodName,
-                                                std::string const& setPrefix)
+bool ChangeToolsGen::ChangeUnaryOperator(UnaryOperator const* unaryOp,
+                                         CXXMemberCallExpr const* call,
+                                         std::string const& methodName,
+                                         std::string const& setPrefix)
 {
     // Look for expressions like
     //    aRect.Left()++;
@@ -289,7 +285,7 @@ bool ChangeRectangleGetRef::ChangeUnaryOperator(UnaryOperator const* unaryOp,
     }
 }
 
-std::string ChangeRectangleGetRef::extractCode(SourceLocation startLoc, SourceLocation endLoc)
+std::string ChangeToolsGen::extractCode(SourceLocation startLoc, SourceLocation endLoc)
 {
     SourceManager& SM = compiler.getSourceManager();
     const char* p1 = SM.getCharacterData(startLoc);
@@ -298,7 +294,7 @@ std::string ChangeRectangleGetRef::extractCode(SourceLocation startLoc, SourceLo
     return std::string(p1, p2 - p1 + n);
 }
 
-static loplugin::Plugin::Registration<ChangeRectangleGetRef> X("changerectanglegetref", false);
+static loplugin::Plugin::Registration<ChangeToolsGen> X("changetoolsgen", false);
 
 } // namespace
 
commit 78bd5d5fb4ff0545985d8ac7bc02a0454ce13afc
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Mon Feb 12 10:24:44 2018 +0200

    loplugin:changerectanglegetref also fix Point and Size
    
    Change-Id: I373af0a62e3785c4abc2d27b0b31121c9d596ca3

diff --git a/compilerplugins/clang/changerectanglegetref.cxx b/compilerplugins/clang/changerectanglegetref.cxx
index b943e9f4516c..f687693ee8cb 100644
--- a/compilerplugins/clang/changerectanglegetref.cxx
+++ b/compilerplugins/clang/changerectanglegetref.cxx
@@ -36,11 +36,12 @@ public:
     bool VisitCXXMemberCallExpr(CXXMemberCallExpr const* call);
 
 private:
-    bool ChangeAssignment(Stmt const* parent, std::string const& methodName);
+    bool ChangeAssignment(Stmt const* parent, std::string const& methodName,
+                          std::string const& setPrefix);
     bool ChangeBinaryOperator(BinaryOperator const* parent, CXXMemberCallExpr const* call,
-                              std::string const& methodName);
+                              std::string const& methodName, std::string const& setPrefix);
     bool ChangeUnaryOperator(UnaryOperator const* parent, CXXMemberCallExpr const* call,
-                             std::string const& methodName);
+                             std::string const& methodName, std::string const& setPrefix);
     std::string extractCode(SourceLocation startLoc, SourceLocation endLoc);
 };
 
@@ -60,14 +61,47 @@ bool ChangeRectangleGetRef::VisitCXXMemberCallExpr(CXXMemberCallExpr const* call
         return true;
     auto dc = loplugin::DeclCheck(func);
     std::string methodName;
+    std::string setPrefix;
     if (dc.Function("Top").Class("Rectangle").Namespace("tools").GlobalNamespace())
+    {
         methodName = "Top";
+        setPrefix = "Set";
+    }
     else if (dc.Function("Bottom").Class("Rectangle").Namespace("tools").GlobalNamespace())
+    {
         methodName = "Bottom";
+        setPrefix = "Set";
+    }
     else if (dc.Function("Left").Class("Rectangle").Namespace("tools").GlobalNamespace())
+    {
         methodName = "Left";
+        setPrefix = "Set";
+    }
     else if (dc.Function("Right").Class("Rectangle").Namespace("tools").GlobalNamespace())
+    {
         methodName = "Right";
+        setPrefix = "Set";
+    }
+    else if (dc.Function("X").Class("Point").GlobalNamespace())
+    {
+        methodName = "X";
+        setPrefix = "set";
+    }
+    else if (dc.Function("Y").Class("Point").GlobalNamespace())
+    {
+        methodName = "Y";
+        setPrefix = "set";
+    }
+    else if (dc.Function("Width").Class("Size").GlobalNamespace())
+    {
+        methodName = "Width";
+        setPrefix = "set";
+    }
+    else if (dc.Function("Height").Class("Size").GlobalNamespace())
+    {
+        methodName = "Height";
+        setPrefix = "set";
+    }
     else
         return true;
     if (!loplugin::TypeCheck(func->getReturnType()).LvalueReference())
@@ -78,7 +112,7 @@ bool ChangeRectangleGetRef::VisitCXXMemberCallExpr(CXXMemberCallExpr const* call
         return true;
     if (auto unaryOp = dyn_cast<UnaryOperator>(parent))
     {
-        if (!ChangeUnaryOperator(unaryOp, call, methodName))
+        if (!ChangeUnaryOperator(unaryOp, call, methodName, setPrefix))
             report(DiagnosticsEngine::Warning, "Could not fix this one1", call->getLocStart());
         return true;
     }
@@ -92,21 +126,22 @@ bool ChangeRectangleGetRef::VisitCXXMemberCallExpr(CXXMemberCallExpr const* call
     auto opcode = binaryOp->getOpcode();
     if (opcode == BO_Assign)
     {
-        if (!ChangeAssignment(parent, methodName))
+        if (!ChangeAssignment(parent, methodName, setPrefix))
             report(DiagnosticsEngine::Warning, "Could not fix this one4", call->getLocStart());
         return true;
     }
     if (opcode == BO_RemAssign || opcode == BO_AddAssign || opcode == BO_SubAssign
         || opcode == BO_MulAssign || opcode == BO_DivAssign)
     {
-        if (!ChangeBinaryOperator(binaryOp, call, methodName))
+        if (!ChangeBinaryOperator(binaryOp, call, methodName, setPrefix))
             report(DiagnosticsEngine::Warning, "Could not fix this one5", call->getLocStart());
         return true;
     }
     return true;
 }
 
-bool ChangeRectangleGetRef::ChangeAssignment(Stmt const* parent, std::string const& methodName)
+bool ChangeRectangleGetRef::ChangeAssignment(Stmt const* parent, std::string const& methodName,
+                                             std::string const& setPrefix)
 {
     // Look for expressions like
     //    aRect.Left() = ...;
@@ -124,7 +159,7 @@ bool ChangeRectangleGetRef::ChangeAssignment(Stmt const* parent, std::string con
     auto originalLength = callText.size();
 
     auto newText = std::regex_replace(callText, std::regex(methodName + "\\(\\) *="),
-                                      "Set" + methodName + "(");
+                                      setPrefix + methodName + "(");
     if (newText == callText)
         return false;
     newText += " )";
@@ -134,7 +169,8 @@ bool ChangeRectangleGetRef::ChangeAssignment(Stmt const* parent, std::string con
 
 bool ChangeRectangleGetRef::ChangeBinaryOperator(BinaryOperator const* binaryOp,
                                                  CXXMemberCallExpr const* call,
-                                                 std::string const& methodName)
+                                                 std::string const& methodName,
+                                                 std::string const& setPrefix)
 {
     // Look for expressions like
     //    aRect.Left() += ...;
@@ -182,14 +218,14 @@ bool ChangeRectangleGetRef::ChangeBinaryOperator(BinaryOperator const* binaryOp,
     auto implicitObjectText = extractCode(call->getImplicitObjectArgument()->getExprLoc(),
                                           call->getImplicitObjectArgument()->getExprLoc());
     auto newText = std::regex_replace(callText, std::regex(methodName + "\\(\\) *" + regexOpname),
-                                      "Set" + methodName + "( " + implicitObjectText + ".Get"
+                                      setPrefix + methodName + "( " + implicitObjectText + "."
                                           + methodName + "() " + replaceOpname + " ");
     if (newText == callText)
         return false;
     // sometimes we end up with duplicate spaces after the opname
-    newText = std::regex_replace(
-        newText, std::regex("Get" + methodName + "\\(\\) \\" + replaceOpname + "  "),
-        "Get" + methodName + "() " + replaceOpname + " ");
+    newText
+        = std::regex_replace(newText, std::regex(methodName + "\\(\\) \\" + replaceOpname + "  "),
+                             methodName + "() " + replaceOpname + " ");
     newText += " )";
 
     return replaceText(startLoc, originalLength, newText);
@@ -197,7 +233,8 @@ bool ChangeRectangleGetRef::ChangeBinaryOperator(BinaryOperator const* binaryOp,
 
 bool ChangeRectangleGetRef::ChangeUnaryOperator(UnaryOperator const* unaryOp,
                                                 CXXMemberCallExpr const* call,
-                                                std::string const& methodName)
+                                                std::string const& methodName,
+                                                std::string const& setPrefix)
 {
     // Look for expressions like
     //    aRect.Left()++;
@@ -238,16 +275,16 @@ bool ChangeRectangleGetRef::ChangeUnaryOperator(UnaryOperator const* unaryOp,
     {
         auto newText
             = std::regex_replace(callText, std::regex(methodName + "\\(\\) *" + regexOpname),
-                                 "Set" + methodName + "( " + replaceOpname + implicitObjectText
-                                     + ".Get" + methodName + "()");
+                                 setPrefix + methodName + "( " + replaceOpname + implicitObjectText
+                                     + "." + methodName + "()");
         return replaceText(startLoc, originalLength, newText);
     }
     else
     {
         auto newText
             = std::regex_replace(callText, std::regex(regexOpname + " *" + methodName + "\\(\\)"),
-                                 "Set" + methodName + "( " + replaceOpname + implicitObjectText
-                                     + ".Get" + methodName + "()");
+                                 setPrefix + methodName + "( " + replaceOpname + implicitObjectText
+                                     + "." + methodName + "()");
         return replaceText(startLoc, originalLength, newText);
     }
 }
commit fc80919370169748864cbbeee8b0c9bbc5d82376
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Mon Feb 12 10:23:59 2018 +0200

    fix loplugin rewriter source range checking
    
    after
        commit 94ab8e4360a2a7a932656e99f718244321d0f923
        Date:   Fri Feb 9 15:28:41 2018 +0200
        improve loplugin rewriter double source modification detection
    
    Change-Id: Ibf0a64fe4cc3dd6bf5ae16672b3d748a842196e4

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index ef2b7667de85..984c9e13d759 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -397,9 +397,15 @@ bool RewritePlugin::insertText( SourceLocation Loc, StringRef Str, bool InsertAf
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertText( Loc, Str, InsertAfter, indentNewLines ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -408,9 +414,15 @@ bool RewritePlugin::insertTextAfter( SourceLocation Loc, StringRef Str )
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertTextAfter( Loc, Str ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -419,9 +431,15 @@ bool RewritePlugin::insertTextAfterToken( SourceLocation Loc, StringRef Str )
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertTextAfterToken( Loc, Str ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -430,9 +448,15 @@ bool RewritePlugin::insertTextBefore( SourceLocation Loc, StringRef Str )
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertTextBefore( Loc, Str ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -457,7 +481,7 @@ bool RewritePlugin::removeText( CharSourceRange range, RewriteOptions opts )
     if( !handler.checkOverlap( range.getAsRange() ) )
     {
         report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", range.getBegin());
-        return true;
+        return false;
     }
     if( opts.flags & RemoveWholeStatement || opts.flags & RemoveAllWhitespace )
     {
@@ -520,7 +544,7 @@ bool RewritePlugin::replaceText( SourceLocation Start, unsigned OrigLength, Stri
     if( OrigLength != 0 && !handler.checkOverlap( Range ) )
     {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", Start );
-        return true;
+        return false;
     }
     if( rewriter->ReplaceText( Start, OrigLength, NewStr ))
         return reportEditFailure( Start );
@@ -538,7 +562,7 @@ bool RewritePlugin::replaceText( SourceRange range, StringRef NewStr )
     if( !handler.checkOverlap( range ) )
     {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
-        return true;
+        return false;
     }
     if( rewriter->ReplaceText( range, NewStr ))
         return reportEditFailure( range.getBegin());
@@ -556,7 +580,7 @@ bool RewritePlugin::replaceText( SourceRange range, SourceRange replacementRange
     if( !handler.checkOverlap( range ) )
     {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
-        return true;
+        return false;
     }
     if( rewriter->ReplaceText( range, replacementRange ))
         return reportEditFailure( range.getBegin());
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index 3e78030993be..959941fb72a9 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -250,10 +250,17 @@ bool PluginHandler::checkOverlap(SourceRange range)
         if (p1 <= rPair.second && rPair.first <= p2)
             return false;
     }
-    mvModifiedRanges.emplace_back(p1, p2);
     return true;
 }
 
+void PluginHandler::addSourceModification(SourceRange range)
+{
+    SourceManager& SM = compiler.getSourceManager();
+    char const  *p1 = SM.getCharacterData( range.getBegin() );
+    char const *p2 = SM.getCharacterData( range.getEnd() );
+    mvModifiedRanges.emplace_back(p1, p2);
+}
+
 void PluginHandler::HandleTranslationUnit( ASTContext& context )
 {
     if( context.getDiagnostics().hasErrorOccurred())
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index 05e8ce3502c7..890b0cf53c3d 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -60,7 +60,7 @@ public:
     // If we overlap with a previous area we modified, we cannot perform this change
     // without corrupting the source
     bool checkOverlap(SourceRange range);
-    bool addSourceModification(SourceRange range);
+    void addSourceModification(SourceRange range);
 private:
     void handleOption( const std::string& option );
     void createPlugins( std::set< std::string > rewriters );


More information about the Libreoffice-commits mailing list