Author: klimek Date: Wed Aug 3 10:12:00 2016 New Revision: 277603 URL: http://llvm.org/viewvc/llvm-project?rev=277603&view=rev Log: Fix bug in conflict check for Replacements::add().
We would not detect conflicts when inserting insertions at the same offset as previously contained replacements. Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp cfe/trunk/unittests/Tooling/RefactoringTest.cpp Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=277603&r1=277602&r2=277603&view=diff ============================================================================== --- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original) +++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Wed Aug 3 10:12:00 2016 @@ -159,15 +159,16 @@ llvm::Error Replacements::add(const Repl // replacement cannot overlap. Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, ""); - // Find the first entry that starts after the end of R. - // We cannot use upper_bound for that, as there might be an element equal to - // AtEnd in Replaces, and AtEnd does not overlap. - // Instead, we use lower_bound and special-case finding AtEnd below. + // Find the first entry that starts after or at the end of R. Note that + // entries that start at the end can still be conflicting if R is an + // insertion. auto I = Replaces.lower_bound(AtEnd); - // If *I == R (which can only happen if R == AtEnd) the first entry that - // starts after R is (I+1). - if (I != Replaces.end() && *I == R) + // If it starts at the same offset as R (can only happen if R is an + // insertion), we have a conflict. In that case, increase I to fall through + // to the conflict check. + if (I != Replaces.end() && R.getOffset() == I->getOffset()) ++I; + // I is the smallest iterator whose entry cannot overlap. // If that is begin(), there are no overlaps. if (I == Replaces.begin()) { Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=277603&r1=277602&r2=277603&view=diff ============================================================================== --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original) +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Wed Aug 3 10:12:00 2016 @@ -157,6 +157,26 @@ TEST_F(ReplacementTest, FailAddRegressio llvm::consumeError(std::move(Err)); } +TEST_F(ReplacementTest, FailAddInsertAtOffsetOfReplacement) { + Replacements Replaces; + auto Err = Replaces.add(Replacement("x.cc", 10, 2, "")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + Err = Replaces.add(Replacement("x.cc", 10, 0, "")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); +} + +TEST_F(ReplacementTest, FailAddInsertAtOtherInsert) { + Replacements Replaces; + auto Err = Replaces.add(Replacement("x.cc", 10, 0, "a")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + Err = Replaces.add(Replacement("x.cc", 10, 0, "b")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); +} + TEST_F(ReplacementTest, CanApplyReplacements) { FileID ID = Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits