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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
