r277603 - Fix bug in conflict check for Replacements::add().
Manuel Klimek via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 3 08:12:00 PDT 2016
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");
More information about the cfe-commits
mailing list