djasper added a comment. Nice :)
================ Comment at: include/clang/Tooling/Core/Replacement.h:62 @@ +61,3 @@ + /// \brief Whether this range is less-than \p RHS or not. + bool operator<(const Range &RHS) const { + if (Offset != RHS.getOffset()) ---------------- Can we just add a lambda that does this to the std::sort operation? I am not sure that this is actually a natural order for all Replacements. ================ 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(); ---------------- Why is this required? ================ Comment at: lib/Tooling/Core/Replacement.cpp:287 @@ +286,3 @@ + std::sort(Ranges.begin(), Ranges.end()); + auto I = Ranges.begin(); + unsigned CurBegin = I->getOffset(); ---------------- 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()); } } ================ Comment at: lib/Tooling/Core/Replacement.cpp:319 @@ +318,3 @@ + const std::vector<Range> &Ranges) { + if (Ranges.empty()) + return calculateChangedRanges(Replaces); ---------------- I'd probably leave these early exits out. I don't think anyone will ever notice the performance difference. ================ Comment at: lib/Tooling/Core/Replacement.cpp:321 @@ +320,3 @@ + return calculateChangedRanges(Replaces); + + if (Replaces.empty()) ---------------- I think you need to do: Ranges = mergeAndSortRanges(Ranges); here http://reviews.llvm.org/D21547 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits