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

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 15 02:58:12 PDT 2016


ioeric added inline comments.

================
Comment at: lib/Tooling/Core/Replacement.cpp:300
@@ +299,3 @@
+Replacements Replacements::merge(const Replacement &R) const {
+  Replacements Rs;
+  llvm::consumeError(Rs.add(R));
----------------
klimek wrote:
> I'd probably add a single-replacement constructor for convenience.
Right...added single-replacement constructor and removed merge(R).

================
Comment at: lib/Tooling/Core/Replacement.cpp:306
@@ +305,3 @@
+// Merge and sort overlapping ranges in \p Ranges.
+static std::vector<Range> mergeAndSortRanges(std::vector<Range> Ranges) {
+  std::sort(Ranges.begin(), Ranges.end(),
----------------
klimek wrote:
> So, this doesn't do the same as Replacements::merge, right? I think we're getting into a bit confusing terminology - perhaps we can find a better name for this?
Changed to `combineAndSortRanges`...not sure if this is better though


https://reviews.llvm.org/D21748





More information about the cfe-commits mailing list