Index: include/clang/Tooling/Core/Replacement.h =================================================================== --- include/clang/Tooling/Core/Replacement.h +++ include/clang/Tooling/Core/Replacement.h @@ -213,6 +213,16 @@ Replacements mergeReplacements(const ReplacementsImpl &Second) const; + /// \brief Tries adding a conflicting deletion `R` into the current set of + /// replacements by merging it with existing replacemnts that are contained in + /// `R` or replacement that containing `R`. `LastOverlap` is the last + /// replacement in the set that overlaps with `R`. + /// On success, all overlapping deletions are replaced with the largest + /// deletion, and the function returns true; otherwise, the set of + /// replacements is not changed, and the function returns false. + bool tryMergeDeletions(const Replacement &R, + ReplacementsImpl::iterator LastOverlap); + ReplacementsImpl Replaces; }; Index: lib/Tooling/Core/Replacement.cpp =================================================================== --- lib/Tooling/Core/Replacement.cpp +++ lib/Tooling/Core/Replacement.cpp @@ -134,8 +134,50 @@ ReplacementText); } -llvm::Error makeConflictReplacementsError(const Replacement &New, - const Replacement &Existing) { +static bool isDeletion(const Replacement &R) { + return R.getLength() > 0 && R.getReplacementText().empty(); +} + +bool Replacements::tryMergeDeletions(const Replacement &R, + ReplacementsImpl::iterator LastOverlap) { + assert(isDeletion(R) && "R must be a deletion replacement."); + if (!isDeletion(*LastOverlap)) + return false; + auto contains = [](const Replacement &R1, const Replacement &R2) -> bool { + return Range(R1.getOffset(), R1.getLength()) + .contains(Range(R2.getOffset(), R2.getLength())); + }; + if (contains(*LastOverlap, R)) + return true; + // Now `LastOverlap` doesn't contain `R`. If `R` doesn't contain + // `LastOverlap` either, they are really conflicting. + if (!contains(R, *LastOverlap)) + return false; + llvm::SmallVector MergedDeletions = { + LastOverlap}; + auto MergedBegin = LastOverlap; + auto MergedEnd = LastOverlap; + // If all existing replacements overlapping with `R` are deletions that are + // contained in `R`, delete all deletions that are contained in `R` and insert + // `R`. + while (LastOverlap-- != Replaces.begin()) { + // Stop checking if `LastOverLap` does not overlap with `R` anymore. + if (LastOverlap->getOffset() + LastOverlap->getLength() <= R.getOffset()) + break; + // Only merge `LastOverlap` into `R` if it is a deletion contained in `R`. + if (!isDeletion(*LastOverlap) || !contains(R, *LastOverlap)) + return false; + MergedBegin = LastOverlap; + } + do { + Replaces.erase(MergedBegin); + } while (MergedBegin++ != MergedEnd); + Replaces.insert(R); + return true; +} + +static llvm::Error makeConflictReplacementsError(const Replacement &New, + const Replacement &Existing) { return llvm::make_error( "New replacement:\n" + New.toString() + "\nconflicts with existing replacement:\n" + Existing.toString(), @@ -205,6 +247,11 @@ // In either case, we can safely insert `R`. Replaces.insert(R); return llvm::Error::success(); + } else if (isDeletion(R) && tryMergeDeletions(R, I)) { + // Special case: if `R` is contained in an existing deletion or contains + // one or more existing replacements, then all deletions can be merged into + // the largest deletion. + return llvm::Error::success(); } return makeConflictReplacementsError(R, *I); } Index: unittests/Tooling/RefactoringTest.cpp =================================================================== --- unittests/Tooling/RefactoringTest.cpp +++ unittests/Tooling/RefactoringTest.cpp @@ -137,6 +137,97 @@ EXPECT_EQ(Replaces.size(), 2u); } +TEST_F(ReplacementTest, MergeNewDeletions) { + Replacements Replaces; + Replacement ContainingReplacement("x.cc", 0, 10, ""); + auto Err = Replaces.add(ContainingReplacement); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement("x.cc", 5, 3, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement("x.cc", 0, 10, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement("x.cc", 5, 5, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + EXPECT_EQ(1u, Replaces.size()); + EXPECT_EQ(*Replaces.begin(), ContainingReplacement); +} + +TEST_F(ReplacementTest, MergeExistingDeletions) { + Replacements Replaces; + auto Err = Replaces.add(Replacement("x.cc", 0, 2, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement("x.cc", 5, 5, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Replacement After = Replacement("x.cc", 10, 5, ""); + Err = Replaces.add(After); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Replacement ContainingReplacement("x.cc", 0, 10, ""); + Err = Replaces.add(ContainingReplacement); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + EXPECT_EQ(2u, Replaces.size()); + EXPECT_EQ(*Replaces.begin(), ContainingReplacement); + EXPECT_EQ(*(++Replaces.begin()), After); +} + +TEST_F(ReplacementTest, InsertionBeforeMergedDeletions) { + Replacements Replaces; + + Replacement Insertion("x.cc", 0, 0, "123"); + auto Err = Replaces.add(Insertion); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement("x.cc", 5, 5, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Replacement Deletion("x.cc", 0, 10, ""); + Err = Replaces.add(Deletion); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + EXPECT_EQ(2u, Replaces.size()); + EXPECT_EQ(*Replaces.begin(), Insertion); + EXPECT_EQ(*(++Replaces.begin()), Deletion); +} + +TEST_F(ReplacementTest, FailedMergeExistingDeletions) { + Replacements Replaces; + Replacement First("x.cc", 0, 2, ""); + auto Err = Replaces.add(First); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Replacement Second("x.cc", 5, 5, ""); + Err = Replaces.add(Second); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement("x.cc", 1, 10, "")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); + + EXPECT_EQ(2u, Replaces.size()); + EXPECT_EQ(*Replaces.begin(), First); + EXPECT_EQ(*(++Replaces.begin()), Second); +} + TEST_F(ReplacementTest, FailAddRegression) { Replacements Replaces; // Create two replacements, where the second one is an insertion of the empty