[PATCH] D21748: Implement tooling::Replacements as a class.

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 20 05:50:48 PDT 2016


klimek added inline comments.

================
Comment at: include/clang/Tooling/Core/Replacement.h:145
@@ -145,4 +144,3 @@
 
-/// \brief A set of Replacements.
-/// FIXME: Change to a vector and deduplicate in the RefactoringTool.
-typedef std::set<Replacement> Replacements;
+/// \brief This maintains a set of replacements that are conflict-free.
+/// Two replacements are considered conflicts if they overlap or have the same
----------------
s/This //

================
Comment at: lib/Tooling/Core/Replacement.cpp:146
@@ +145,3 @@
+        return llvm::make_error<llvm::StringError>(
+            "All replacements must have the SAME FILE PATH! New replacement: " +
+                R.getFilePath() + ", existing replacements: " +
----------------
DO NOT SCREAM!!! :D

================
Comment at: lib/Tooling/Core/Replacement.cpp:306
@@ +305,3 @@
+              return LHS.getLength() < RHS.getLength();
+            });
+  std::vector<Range> Result;
----------------
I believe it's better. Can you add a comment expanding what this is for, so we don't have to re-think about that next time we stumble over it? :)


https://reviews.llvm.org/D21748





More information about the cfe-commits mailing list