[PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
Manuel Klimek via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 3 05:59:50 PDT 2016
klimek updated this revision to Diff 66651.
klimek added a comment.
Remove re-implementation of overlaps check.
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,46 @@
}
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 does not overlap, we know that entries before it
+ // can also not overlap.
+ if (R.getOffset() != I->getOffset() &&
+ !Range(R.getOffset(), R.getLength())
+ .overlapsWith(Range(I->getOffset(), I->getLength()))) {
+ Replaces.insert(R);
+ return llvm::Error::success();
+ }
+ return llvm::make_error<llvm::StringError>(
+ "New replacement:\n" + R.toString() +
+ "\nconflicts with existing replacement:\n" + I->toString(),
+ llvm::inconvertibleErrorCode());
}
namespace {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23119.66651.patch
Type: text/x-patch
Size: 4073 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160803/f298dbe8/attachment.bin>
More information about the cfe-commits
mailing list