[PATCH] D21547: Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.

[Eric Liu via cfe-commits]

ioeric added inline comments.

================
Comment at: include/clang/Tooling/Core/Replacement.h:253
@@ +252,3 @@
+std::vector<Range>
+calculateRangesAfterReplacements(const Replacements &Replaces,
+                                 const std::vector<Range> &Ranges);
----------------
getShiftedRanges sounds like it's excluding affected ranges of Replaces. I think often time we want both?

================
Comment at: include/clang/Tooling/Core/Replacement.h:237
@@ +236,3 @@
+/// \brief Calculates the new ranges after \p Replaces are applied. These
+/// include both the original \p Ranges and the affected ranges of \p Replaces
+/// in the new code.
----------------
Why would why want to exclude the newly affected ranges? But if replaces overlap with original ranges, do we consider sub-ranges in original ranges being replaced as parts of the shifted ranges in the new code?

================
Comment at: lib/Tooling/Core/Replacement.cpp:305
@@ +304,3 @@
+// Merge and sort overlapping and adjacent ranges in \p Ranges.
+static void mergeAndSortRanges(std::vector<Range> &Ranges) {
+  if (Ranges.empty())
----------------
Shall we pass Ranges by value instead since it's gonna be sorted anyway?

Also, does it make sense to have calculateChangedRanges() call this?

================
Comment at: lib/Tooling/Core/Replacement.cpp:328
@@ +327,3 @@
+std::vector<Range>
+calculateRangesAfterReplacements(const Replacements &Replaces,
+                                 std::vector<Range> Ranges) {
----------------
That actually works very well! Thanks!


http://reviews.llvm.org/D21547