klimek created this revision.
klimek added reviewers: djasper, bkramer, ioeric.
klimek added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===================================================================
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,6 +115,26 @@
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Replaces.clear();
+ // Test overlap with an existing insertion.
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
TEST_F(ReplacementTest, CanApplyReplacements) {
FileID ID = Context.createInMemoryFile("input.cpp",
"line1\nline2\nline3\nline4");
Index: lib/Tooling/Core/Replacement.cpp
===================================================================
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -138,25 +138,53 @@
}
llvm::Error Replacements::add(const Replacement &R) {
- if (R.getOffset() != UINT_MAX)
- for (auto Replace : Replaces) {
- if (R.getFilePath() != Replace.getFilePath())
- return llvm::make_error<llvm::StringError>(
- "All replacements must have the same file path. New replacement: " +
- R.getFilePath() + ", existing replacements: " +
- Replace.getFilePath() + "\n",
- llvm::inconvertibleErrorCode());
- if (R.getOffset() == Replace.getOffset() ||
- Range(R.getOffset(), R.getLength())
- .overlapsWith(Range(Replace.getOffset(), Replace.getLength())))
- return llvm::make_error<llvm::StringError>(
- "New replacement:\n" + R.toString() +
- "\nconflicts with existing replacement:\n" + Replace.toString(),
- llvm::inconvertibleErrorCode());
- }
+ // Check the file path.
+ if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
+ return llvm::make_error<llvm::StringError>(
+ "All replacements must have the same file path. New replacement: " +
+ R.getFilePath() + ", existing replacements: " +
+ Replaces.begin()->getFilePath() + "\n",
+ llvm::inconvertibleErrorCode());
+
+ // Special-case header insertions.
+ if (R.getOffset() == UINT_MAX) {
+ Replaces.insert(R);
+ return llvm::Error::success();
+ }
- Replaces.insert(R);
- return llvm::Error::success();
+ // This replacement cannot conflict with replacements that end before
+ // this replacement starts or start after this replacement ends.
+ // We also know that there currently are no overlapping replacements.
+ // Thus, we know that all replacements that start after the end of the current
+ // replacement cannot overlap.
+ Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
+ auto I = Replaces.upper_bound(AtEnd);
+ // I is the smallest iterator whose entry cannot overlap.
+ // If that is begin(), there are no overlaps.
+ if (I == Replaces.begin()) {
+ Replaces.insert(R);
+ return llvm::Error::success();
+ }
+ --I;
+ // If the previous entry's end is left of or touches R's start, or
+ // if the previous entry's start is right of or touches R's end, there is no
+ // overlap.
+ // The exception is if one of them is an insertion; in that case, the offsets
+ // must be equal.
+ if (I->getOffset() != R.getOffset() &&
+ (I->getOffset() + I->getLength() <= R.getOffset() ||
+ R.getOffset() + R.getLength() <= I->getOffset())) {
+ Replaces.insert(R);
+ return llvm::Error::success();
+ }
+ // Otherwise there must be overlap.
+ assert(R.getOffset() == I->getOffset() ||
+ Range(R.getOffset(), R.getLength())
+ .overlapsWith(Range(I->getOffset(), I->getLength())));
+ return llvm::make_error<llvm::StringError>(
+ "New replacement:\n" + R.toString() +
+ "\nconflicts with existing replacement:\n" + I->toString(),
+ llvm::inconvertibleErrorCode());
}
namespace {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
