[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