From 7604798c68651927fb923ec1c0f6cda70122fc07 Mon Sep 17 00:00:00 2001
From: Manuel Klimek <klimek@google.com>
Date: Wed, 3 Aug 2016 15:12:00 +0000
Subject: [PATCH] Fix bug in conflict check for Replacements::add().

We would not detect conflicts when inserting insertions at the same
offset as previously contained replacements.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@277603 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Tooling/Core/Replacement.cpp      | 15 ++++++++-------
 unittests/Tooling/RefactoringTest.cpp | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/lib/Tooling/Core/Replacement.cpp b/lib/Tooling/Core/Replacement.cpp
index b990ed2083..d498f43352 100644
--- a/lib/Tooling/Core/Replacement.cpp
+++ b/lib/Tooling/Core/Replacement.cpp
@@ -159,15 +159,16 @@ llvm::Error Replacements::add(const Replacement &R) {
   // 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()) {
diff --git a/unittests/Tooling/RefactoringTest.cpp b/unittests/Tooling/RefactoringTest.cpp
index b06123ed62..5dd5c02c57 100644
--- a/unittests/Tooling/RefactoringTest.cpp
+++ b/unittests/Tooling/RefactoringTest.cpp
@@ -157,6 +157,26 @@ TEST_F(ReplacementTest, FailAddRegression) {
   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");
-- 
2.40.0