r188287 - Fixing a conflict detection bug in tooling::deduplicate

[Edwin Vane]

Author: revane
Date: Tue Aug 13 11:26:44 2013
New Revision: 188287

URL: http://llvm.org/viewvc/llvm-project?rev=188287&view=rev
Log:
Fixing a conflict detection bug in tooling::deduplicate

If a Replacment is contained within the conflict range being built, the
conflict range would be erroneously shortened. Now fixed. Tests updated to
catch this case.


Modified:
    cfe/trunk/lib/Tooling/Refactoring.cpp
    cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring.cpp?rev=188287&r1=188286&r2=188287&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring.cpp Tue Aug 13 11:26:44 2013
@@ -206,8 +206,9 @@ void deduplicate(std::vector<Replacement
     if (ConflictRange.overlapsWith(Current)) {
       // Extend conflicted range
       ConflictRange = Range(ConflictRange.getOffset(),
-                            Current.getOffset() + Current.getLength() -
-                                ConflictRange.getOffset());
+                            std::max(ConflictRange.getLength(),
+                                     Current.getOffset() + Current.getLength() -
+                                         ConflictRange.getOffset()));
       ++ConflictLength;
     } else {
       if (ConflictLength > 1)

Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=188287&r1=188286&r2=188287&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Tue Aug 13 11:26:44 2013
@@ -400,20 +400,25 @@ TEST(DeduplicateTest, detectsConflicts)
     // returned conflict info refers to.
     Input.push_back(Replacement("fileA", 0,  5, " foo "));  // 0
     Input.push_back(Replacement("fileA", 5,  5, " bar "));  // 1
+    Input.push_back(Replacement("fileA", 6,  0, " bar "));  // 3
     Input.push_back(Replacement("fileA", 5,  5, " moo "));  // 2
-    Input.push_back(Replacement("fileA", 15, 5, " golf ")); // 4
-    Input.push_back(Replacement("fileA", 16, 5, " bag "));  // 5
-    Input.push_back(Replacement("fileA", 10, 3, " club ")); // 6
+    Input.push_back(Replacement("fileA", 7,  2, " bar "));  // 4
+    Input.push_back(Replacement("fileA", 15, 5, " golf ")); // 5
+    Input.push_back(Replacement("fileA", 16, 5, " bag "));  // 6
+    Input.push_back(Replacement("fileA", 10, 3, " club ")); // 7
+
+    // #3 is special in that it is completely contained by another conflicting
+    // Replacement. #4 ensures #3 hasn't messed up the conflicting range size.
 
     std::vector<Range> Conflicts;
     deduplicate(Input, Conflicts);
 
     // No duplicates
-    ASSERT_EQ(6u, Input.size());
+    ASSERT_EQ(8u, Input.size());
     ASSERT_EQ(2u, Conflicts.size());
     ASSERT_EQ(1u, Conflicts[0].getOffset());
-    ASSERT_EQ(2u, Conflicts[0].getLength());
-    ASSERT_EQ(4u, Conflicts[1].getOffset());
+    ASSERT_EQ(4u, Conflicts[0].getLength());
+    ASSERT_EQ(6u, Conflicts[1].getOffset());
     ASSERT_EQ(2u, Conflicts[1].getLength());
   }
 }