Don't attempt to actually do double code removals

...that easily works around the problem that in a rewriter rewriting types of
VarDecls like

   T x, y;

it would try to replace T twice.  Also, keep the list of removals globally with
the (global) rewriter.

Change-Id: I55b8d11986c2a29e09ff40132fd114a0cc48dc90

4 files changed, 27 insertions, 12 deletions

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 626e55a624ac..88841ea308a3 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -25,7 +25,7 @@ namespace loplugin
 {
 
 Plugin::Plugin( const InstantiationData& data )
-    : compiler( data.compiler ), name( data.name ), handler( data.handler )
+    : compiler( data.compiler ), handler( data.handler ), name( data.name )
     {
     }
 
@@ -192,9 +192,11 @@ bool RewritePlugin::removeText( CharSourceRange range, RewriteOptions opts )
     assert( rewriter );
     if( rewriter->getRangeSize( range, opts ) == -1 )
         return reportEditFailure( range.getBegin());
-    if( removals.find( range.getBegin()) != removals.end())
+    if( !handler.addRemoval( range.getBegin() ) )
+        {
         report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", range.getBegin());
-    removals.insert( range.getBegin());
+        return true;
+        }
     if( opts.flags & RemoveWholeStatement || opts.flags & RemoveAllWhitespace )
         {
         if( !adjustRangeForOptions( &range, opts ))
@@ -249,9 +251,11 @@ bool RewritePlugin::adjustRangeForOptions( CharSourceRange* range, RewriteOption
 bool RewritePlugin::replaceText( SourceLocation Start, unsigned OrigLength, StringRef NewStr )
     {
     assert( rewriter );
-    if( OrigLength != 0 && removals.find( Start ) != removals.end())
+    if( OrigLength != 0 && !handler.addRemoval( Start ) )
+        {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", Start );
-    removals.insert( Start );
+        return true;
+        }
     if( rewriter->ReplaceText( Start, OrigLength, NewStr ))
         return reportEditFailure( Start );
     return true;
@@ -262,9 +266,11 @@ bool RewritePlugin::replaceText( SourceRange range, StringRef NewStr )
     assert( rewriter );
     if( rewriter->getRangeSize( range ) == -1 )
         return reportEditFailure( range.getBegin());
-    if( removals.find( range.getBegin()) != removals.end())
+    if( !handler.addRemoval( range.getBegin() ) )
+        {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
-    removals.insert( range.getBegin());
+        return true;
+        }
     if( rewriter->ReplaceText( range, NewStr ))
         return reportEditFailure( range.getBegin());
     return true;
@@ -275,9 +281,11 @@ bool RewritePlugin::replaceText( SourceRange range, SourceRange replacementRange
     assert( rewriter );
     if( rewriter->getRangeSize( range ) == -1 )
         return reportEditFailure( range.getBegin());
-    if( removals.find( range.getBegin()) != removals.end())
+    if( !handler.addRemoval( range.getBegin() ) )
+        {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
-    removals.insert( range.getBegin());
+        return true;
+        }
     if( rewriter->ReplaceText( range, replacementRange ))
         return reportEditFailure( range.getBegin());
     return true;
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index ff868bf1718f..796ebef74a11 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -20,7 +20,6 @@
 #include <clang/Basic/SourceManager.h>
 #include <clang/Frontend/CompilerInstance.h>
 #include <clang/Lex/Preprocessor.h>
-#include <set>
 #include <unordered_map>
 
 #if __clang_major__ < 3 || __clang_major__ == 3 && __clang_minor__ < 2
@@ -67,6 +66,7 @@ class Plugin
         bool ignoreLocation( const Decl* decl );
         bool ignoreLocation( const Stmt* stmt );
         CompilerInstance& compiler;
+        PluginHandler& handler;
         /**
          Returns the parent of the given AST node. Clang's internal AST representation doesn't provide this information,
          it can only provide children, but getting the parent is often useful for inspecting a part of the AST.
@@ -78,7 +78,6 @@ class Plugin
         template< typename T > static Plugin* createHelper( const InstantiationData& data );
         enum { isRewriter = false };
         const char* name;
-        PluginHandler& handler;
         static unordered_map< const Stmt*, const Stmt* > parents;
         static void buildParents( CompilerInstance& compiler );
     };
@@ -139,7 +138,6 @@ class RewritePlugin
         enum { isRewriter = true };
         bool reportEditFailure( SourceLocation loc );
         bool adjustRangeForOptions( CharSourceRange* range, RewriteOptions options );
-        set< SourceLocation > removals;
     };
 
 /**
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index d6595a3d502d..4518dd5cbca5 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -159,6 +159,11 @@ DiagnosticBuilder PluginHandler::report( DiagnosticsEngine::Level level, StringR
     return report( level, nullptr, message, compiler, loc );
     }
 
+bool PluginHandler::addRemoval( SourceLocation loc )
+    {
+    return removals.insert( loc ).second;
+    }
+
 void PluginHandler::HandleTranslationUnit( ASTContext& context )
     {
     if( context.getDiagnostics().hasErrorOccurred())
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index dcfac71abdee..48cee8e4a402 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -14,6 +14,8 @@
 
 #include "plugin.hxx"
 
+#include <set>
+
 #include <clang/AST/ASTConsumer.h>
 #include <clang/Frontend/FrontendAction.h>
 
@@ -33,12 +35,14 @@ class PluginHandler
         static void registerPlugin( Plugin* (*create)( const Plugin::InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault );
         DiagnosticBuilder report( DiagnosticsEngine::Level level, const char * plugin, StringRef message,
             CompilerInstance& compiler, SourceLocation loc = SourceLocation());
+        bool addRemoval( SourceLocation loc );
     private:
         void handleOption( const string& option );
         void createPlugins( set< string > rewriters );
         DiagnosticBuilder report( DiagnosticsEngine::Level level, StringRef message, SourceLocation loc = SourceLocation());
         CompilerInstance& compiler;
         Rewriter rewriter;
+        set< SourceLocation > removals;
         string scope;
         string warningsOnly;
     };