[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