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

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 21 08:09:11 PDT 2016


ioeric added inline comments.

================
Comment at: include/clang/Tooling/Core/Replacement.h:69
@@ +68,3 @@
+  /// \brief Whether this range equals to \p RHS or not.
+  bool operator==(const Range &RHS) const {
+    return Offset == RHS.getOffset() && Length == RHS.getLength();
----------------
djasper wrote:
> Why is this required?
It is used in unit test to test the equality between two ranges.

================
Comment at: lib/Tooling/Core/Replacement.cpp:287
@@ +286,3 @@
+  std::sort(Ranges.begin(), Ranges.end());
+  auto I = Ranges.begin();
+  unsigned CurBegin = I->getOffset();
----------------
djasper wrote:
> maybe:
> 
>   std::vector<Range> Result;
>   for (const auto &R: Ranges) {
>     if (Result.empty() || !Result.back().overlapsWith(R)) {
>       Result.push_back(R);
>     } else {
>       unsigned NewEnd = std::max(Result.back.getOffset() + Result.back.getLength(), R.getOffset() + R.getLength());
>       Result[Result.size() - 1] = Range(Result.back.getOffset(), NewEnd - Result.back.getOffset());
>     }
>   }
Thanks! Just one minor point: we also want to merge ranges `R1` and `R2` if 
```
R1.getOffset() + R1.getLength() == R2.getOffset()
``` 
For example, if we have `R1(1, 1)` and `R2(2, 1)`, then we can merge them into `R3(1, 2)`.


http://reviews.llvm.org/D21547





More information about the cfe-commits mailing list