Skip to content

Commit f6c7973

Browse files
jrose-applejrose-apple
authored
Teach -verify-apply-fixes to handle overlapping fix-its (#23970)
...by coalescing duplicates and dropping conflicts. Both cases can happen with "expected-error 2 {{...}}": we might get multiple fix-its providing the same new message, or one message might have diverged into two, giving us incompatible changes.
1 parent d4a5740 commit f6c7973

File tree

4 files changed

+239
-0
lines changed

4 files changed

+239
-0
lines changed

‎include/swift/Basic/STLExtras.h

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,44 @@ class RelationalOperationsBase {
834834
}
835835
};
836836

837+
/// Removes all runs of values that match \p pred from the range of \p begin
838+
/// to \p end.
839+
///
840+
/// This is similar to std::unique, but std::unique leaves the first value in
841+
/// place in a run of matching values, whereas this code removes all of them.
842+
///
843+
/// \returns The new end iterator for the container. You should erase elements
844+
/// between this value and the existing end of the container.
845+
template <typename Iterator, typename BinaryPredicate>
846+
Iterator removeAdjacentIf(const Iterator first, const Iterator last,
847+
BinaryPredicate pred) {
848+
using element_reference_t =
849+
typename std::iterator_traits<Iterator>::reference;
850+
851+
auto nextOverlap = std::adjacent_find(first, last, pred);
852+
auto insertionPoint = nextOverlap;
853+
while (nextOverlap != last) {
854+
// We want to erase *all* the matching elements. There could be three or
855+
// more of them. Search for the end of the run.
856+
auto lastOverlapInRun =
857+
std::adjacent_find(std::next(nextOverlap), last,
858+
[&pred](element_reference_t left,
859+
element_reference_t right) -> bool {
860+
return !pred(left, right);
861+
});
862+
// If we get the end iterator back, that means all remaining elements match.
863+
// If we don't, that means (lastOverlapInRun+1) is part of a different run.
864+
if (lastOverlapInRun != last)
865+
++lastOverlapInRun;
866+
867+
nextOverlap = std::adjacent_find(lastOverlapInRun, last, pred);
868+
insertionPoint = std::move(lastOverlapInRun, nextOverlap, insertionPoint);
869+
}
870+
871+
return insertionPoint;
872+
}
873+
874+
837875
} // end namespace swift
838876

839877
#endif // SWIFT_BASIC_INTERLEAVE_H

‎lib/Frontend/DiagnosticVerifier.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,23 @@ void DiagnosticVerifier::autoApplyFixes(unsigned BufferID,
663663
return lhs.getRange().Start.getPointer()
664664
< rhs.getRange().Start.getPointer();
665665
});
666+
// Coalesce identical fix-its. This happens most often with "expected-error 2"
667+
// syntax.
668+
FixIts.erase(std::unique(FixIts.begin(), FixIts.end(),
669+
[](const llvm::SMFixIt &lhs,
670+
const llvm::SMFixIt &rhs) -> bool {
671+
return lhs.getRange().Start == rhs.getRange().Start &&
672+
lhs.getRange().End == rhs.getRange().End &&
673+
lhs.getText() == rhs.getText();
674+
}), FixIts.end());
675+
// Filter out overlapping fix-its. This allows the compiler to apply changes
676+
// to the easy parts of the file, and leave in the tricky cases for the
677+
// developer to handle manually.
678+
FixIts.erase(swift::removeAdjacentIf(FixIts.begin(), FixIts.end(),
679+
[](const llvm::SMFixIt &lhs,
680+
const llvm::SMFixIt &rhs) {
681+
return lhs.getRange().End.getPointer() > rhs.getRange().Start.getPointer();
682+
}), FixIts.end());
666683

667684
// Get the contents of the original source file.
668685
auto memBuffer = SM.getLLVMSourceMgr().getMemoryBuffer(BufferID);

‎unittests/Basic/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ add_swift_unittest(SwiftBasicTests
2323
PrefixMapTest.cpp
2424
RangeTest.cpp
2525
SourceManagerTest.cpp
26+
STLExtrasTest.cpp
2627
StringExtrasTest.cpp
2728
SuccessorMapTest.cpp
2829
ThreadSafeRefCntPointerTest.cpp

‎unittests/Basic/STLExtrasTest.cpp

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
//===--- STLExtrasTest.cpp ------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "swift/Basic/STLExtras.h"
14+
#include "gtest/gtest.h"
15+
16+
using namespace swift;
17+
18+
TEST(RemoveAdjacentIf, NoRemovals) {
19+
{
20+
int items[] = { 1, 2, 3 };
21+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
22+
std::equal_to<int>());
23+
EXPECT_EQ(result, std::end(items));
24+
}
25+
26+
{
27+
int items[] = { 1 };
28+
// Test an empty range.
29+
auto result = removeAdjacentIf(std::begin(items), std::begin(items),
30+
std::equal_to<int>());
31+
EXPECT_EQ(result, std::begin(items));
32+
}
33+
34+
{
35+
int *null = nullptr;
36+
auto result = removeAdjacentIf(null, null, std::equal_to<int>());
37+
EXPECT_EQ(result, null);
38+
}
39+
}
40+
41+
TEST(RemoveAdjacentIf, OnlyOneRun) {
42+
{
43+
int items[] = { 1, 2, 3, 3, 4, 5, 6 };
44+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
45+
std::equal_to<int>());
46+
EXPECT_EQ(result, &items[5]);
47+
EXPECT_EQ(items[0], 1);
48+
EXPECT_EQ(items[1], 2);
49+
EXPECT_EQ(items[2], 4);
50+
EXPECT_EQ(items[3], 5);
51+
EXPECT_EQ(items[4], 6);
52+
}
53+
54+
{
55+
int items[] = { 1, 2, 3, 3, 3, 4, 5, 6 };
56+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
57+
std::equal_to<int>());
58+
EXPECT_EQ(result, &items[5]);
59+
EXPECT_EQ(items[0], 1);
60+
EXPECT_EQ(items[1], 2);
61+
EXPECT_EQ(items[2], 4);
62+
EXPECT_EQ(items[3], 5);
63+
EXPECT_EQ(items[4], 6);
64+
}
65+
66+
{
67+
int items[] = { 1, 2, 3, 3, 3, 3, 4, 5, 6 };
68+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
69+
std::equal_to<int>());
70+
EXPECT_EQ(result, &items[5]);
71+
EXPECT_EQ(items[0], 1);
72+
EXPECT_EQ(items[1], 2);
73+
EXPECT_EQ(items[2], 4);
74+
EXPECT_EQ(items[3], 5);
75+
EXPECT_EQ(items[4], 6);
76+
}
77+
78+
{
79+
int items[] = { 1, 2, 3, 3, 3, 3 };
80+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
81+
std::equal_to<int>());
82+
EXPECT_EQ(result, &items[2]);
83+
EXPECT_EQ(items[0], 1);
84+
EXPECT_EQ(items[1], 2);
85+
}
86+
87+
{
88+
int items[] = { 3, 3, 3, 3, 4, 5, 6 };
89+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
90+
std::equal_to<int>());
91+
EXPECT_EQ(result, &items[3]);
92+
EXPECT_EQ(items[0], 4);
93+
EXPECT_EQ(items[1], 5);
94+
EXPECT_EQ(items[2], 6);
95+
}
96+
97+
{
98+
int items[] = { 1, 1, 1, 1 };
99+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
100+
std::equal_to<int>());
101+
EXPECT_EQ(result, &items[0]);
102+
}
103+
}
104+
105+
106+
TEST(RemoveAdjacentIf, MultipleRuns) {
107+
{
108+
int items[] = { 1, 2, 3, 3, 4, 5, 6, 7, 7, 8, 9 };
109+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
110+
std::equal_to<int>());
111+
EXPECT_EQ(result, &items[7]);
112+
EXPECT_EQ(items[0], 1);
113+
EXPECT_EQ(items[1], 2);
114+
EXPECT_EQ(items[2], 4);
115+
EXPECT_EQ(items[3], 5);
116+
EXPECT_EQ(items[4], 6);
117+
EXPECT_EQ(items[5], 8);
118+
EXPECT_EQ(items[6], 9);
119+
}
120+
121+
{
122+
int items[] = { 1, 2, 3, 3, 3, 4, 5, 6, 7, 7, 7, 8, 9 };
123+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
124+
std::equal_to<int>());
125+
EXPECT_EQ(result, &items[7]);
126+
EXPECT_EQ(items[0], 1);
127+
EXPECT_EQ(items[1], 2);
128+
EXPECT_EQ(items[2], 4);
129+
EXPECT_EQ(items[3], 5);
130+
EXPECT_EQ(items[4], 6);
131+
EXPECT_EQ(items[5], 8);
132+
EXPECT_EQ(items[6], 9);
133+
}
134+
135+
{
136+
int items[] = { 1, 2, 3, 3, 3, 3, 4, 5, 6, 7, 7, 7, 7, 8, 9 };
137+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
138+
std::equal_to<int>());
139+
EXPECT_EQ(result, &items[7]);
140+
EXPECT_EQ(items[0], 1);
141+
EXPECT_EQ(items[1], 2);
142+
EXPECT_EQ(items[2], 4);
143+
EXPECT_EQ(items[3], 5);
144+
EXPECT_EQ(items[4], 6);
145+
EXPECT_EQ(items[5], 8);
146+
EXPECT_EQ(items[6], 9);
147+
}
148+
149+
{
150+
int items[] = { 1, 2, 3, 3, 3, 3, 7, 7 };
151+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
152+
std::equal_to<int>());
153+
EXPECT_EQ(result, &items[2]);
154+
EXPECT_EQ(items[0], 1);
155+
EXPECT_EQ(items[1], 2);
156+
}
157+
158+
{
159+
int items[] = { 3, 3, 3, 3, 4, 5, 6, 7, 7 };
160+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
161+
std::equal_to<int>());
162+
EXPECT_EQ(result, &items[3]);
163+
EXPECT_EQ(items[0], 4);
164+
EXPECT_EQ(items[1], 5);
165+
EXPECT_EQ(items[2], 6);
166+
}
167+
168+
{
169+
int items[] = { 3, 3, 3, 3, 7, 7, 8, 9 };
170+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
171+
std::equal_to<int>());
172+
EXPECT_EQ(result, &items[2]);
173+
EXPECT_EQ(items[0], 8);
174+
EXPECT_EQ(items[1], 9);
175+
}
176+
177+
{
178+
int items[] = { 1, 1, 1, 1, 2, 2 };
179+
auto result = removeAdjacentIf(std::begin(items), std::end(items),
180+
std::equal_to<int>());
181+
EXPECT_EQ(result, &items[0]);
182+
}
183+
}

0 commit comments

Comments
 (0)