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