Skip to content

Commit 31a71c8

Browse files
AnthonyLatsisAnthonyLatsis
authored
Merge pull request #63545 from AnthonyLatsis/robust-fixit-verifications
DiagnosticVerifier: Implement defaulting behavior for lines in expected fix-it replacement range
2 parents 1e4e7f7 + 3998fdf commit 31a71c8

39 files changed

+293
-280
lines changed

‎include/swift/Frontend/DiagnosticVerifier.h

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,10 @@ struct ExpectedFixIt;
3838

3939
/// A range expressed in terms of line-and-column pairs.
4040
struct LineColumnRange {
41-
static constexpr unsigned NoValue = ~0u;
42-
4341
unsigned StartLine, StartCol;
4442
unsigned EndLine, EndCol;
4543

46-
LineColumnRange()
47-
: StartLine(NoValue), StartCol(NoValue), EndLine(NoValue),
48-
EndCol(NoValue) {}
44+
LineColumnRange() : StartLine(0), StartCol(0), EndLine(0), EndCol(0) {}
4945
};
5046

5147
class CapturedFixItInfo final {
@@ -63,9 +59,7 @@ class CapturedFixItInfo final {
6359
/// Obtain the line-column range corresponding to the fix-it's
6460
/// replacement range.
6561
const LineColumnRange &getLineColumnRange(const SourceManager &SM,
66-
unsigned BufferID,
67-
bool ComputeStartLocLine,
68-
bool ComputeEndLocLine) const;
62+
unsigned BufferID) const;
6963
};
7064

7165
struct CapturedDiagnosticInfo {

‎lib/Frontend/DiagnosticVerifier.cpp

Lines changed: 67 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,30 @@ struct ExpectedFixIt {
3535
};
3636
} // end namespace swift
3737

38-
constexpr unsigned LineColumnRange::NoValue;
39-
4038
const LineColumnRange &
4139
CapturedFixItInfo::getLineColumnRange(const SourceManager &SM,
42-
unsigned BufferID,
43-
bool ComputeStartLocLine,
44-
bool ComputeEndLocLine) const {
45-
if (LineColRange.StartLine == LineColumnRange::NoValue &&
46-
ComputeStartLocLine) {
47-
std::tie(LineColRange.StartLine, LineColRange.StartCol) =
48-
SM.getPresumedLineAndColumnForLoc(getSourceRange().getStart(),
49-
BufferID);
50-
} else if (LineColRange.StartCol == LineColumnRange::NoValue) {
51-
LineColRange.StartCol =
52-
SM.getColumnInBuffer(getSourceRange().getStart(), BufferID);
40+
unsigned BufferID) const {
41+
if (LineColRange.StartLine != 0) {
42+
// Already computed.
43+
return LineColRange;
5344
}
5445

55-
if (LineColRange.EndLine == LineColumnRange::NoValue && ComputeEndLocLine) {
46+
auto SrcRange = FixIt.getRange();
47+
48+
std::tie(LineColRange.StartLine, LineColRange.StartCol) =
49+
SM.getPresumedLineAndColumnForLoc(SrcRange.getStart(), BufferID);
50+
51+
// We don't have to compute much if the end location is on the same line.
52+
if (SrcRange.getByteLength() == 0) {
53+
LineColRange.EndLine = LineColRange.StartLine;
54+
LineColRange.EndCol = LineColRange.StartCol;
55+
} else if (SM.extractText(SrcRange, BufferID).find_first_of("\n\r") ==
56+
StringRef::npos) {
57+
LineColRange.EndLine = LineColRange.StartLine;
58+
LineColRange.EndCol = LineColRange.StartCol + SrcRange.getByteLength();
59+
} else {
5660
std::tie(LineColRange.EndLine, LineColRange.EndCol) =
57-
SM.getPresumedLineAndColumnForLoc(FixIt.getRange().getEnd(), BufferID);
58-
} else if (LineColRange.EndCol == LineColumnRange::NoValue) {
59-
LineColRange.EndCol =
60-
SM.getColumnInBuffer(getSourceRange().getEnd(), BufferID);
61+
SM.getPresumedLineAndColumnForLoc(SrcRange.getEnd(), BufferID);
6162
}
6263

6364
return LineColRange;
@@ -293,23 +294,11 @@ bool DiagnosticVerifier::checkForFixIt(
293294
if (ActualFixIt.getText() != Expected.Text)
294295
continue;
295296

296-
LineColumnRange ActualRange = ActualFixIt.getLineColumnRange(
297-
SM, BufferID,
298-
// Don't compute line numbers unless we have to.
299-
/*ComputeStartLocLine=*/Expected.Range.StartLine !=
300-
LineColumnRange::NoValue,
301-
/*ComputeEndLocLine=*/Expected.Range.EndLine !=
302-
LineColumnRange::NoValue);
297+
auto &ActualRange = ActualFixIt.getLineColumnRange(SM, BufferID);
303298

304299
if (Expected.Range.StartCol != ActualRange.StartCol ||
305-
Expected.Range.EndCol != ActualRange.EndCol) {
306-
continue;
307-
}
308-
if (Expected.Range.StartLine != LineColumnRange::NoValue &&
309-
Expected.Range.StartLine != ActualRange.StartLine) {
310-
continue;
311-
}
312-
if (Expected.Range.EndLine != LineColumnRange::NoValue &&
300+
Expected.Range.EndCol != ActualRange.EndCol ||
301+
Expected.Range.StartLine != ActualRange.StartLine ||
313302
Expected.Range.EndLine != ActualRange.EndLine) {
314303
continue;
315304
}
@@ -329,10 +318,7 @@ DiagnosticVerifier::renderFixits(ArrayRef<CapturedFixItInfo> ActualFixIts,
329318
interleave(
330319
ActualFixIts,
331320
[&](const CapturedFixItInfo &ActualFixIt) {
332-
LineColumnRange ActualRange =
333-
ActualFixIt.getLineColumnRange(SM, BufferID,
334-
/*ComputeStartLocLine=*/true,
335-
/*ComputeEndLocLine=*/true);
321+
auto &ActualRange = ActualFixIt.getLineColumnRange(SM, BufferID);
336322
OS << "{{";
337323

338324
if (ActualRange.StartLine != DiagnosticLineNo)
@@ -372,81 +358,85 @@ static Optional<LineColumnRange> parseExpectedFixItRange(
372358
llvm::function_ref<void(const char *, const Twine &)> diagnoseError) {
373359
assert(!Str.empty());
374360

375-
const auto parseLineAndColumn =
376-
[&]() -> Optional<std::pair<unsigned, unsigned>> {
377-
enum class LineOffsetKind : uint8_t { None, Plus, Minus };
361+
struct ParsedLineAndColumn {
362+
Optional<unsigned> Line;
363+
unsigned Column;
364+
};
378365

379-
LineOffsetKind lineOffsetKind = LineOffsetKind::None;
366+
const auto parseLineAndColumn = [&]() -> Optional<ParsedLineAndColumn> {
367+
enum class OffsetKind : uint8_t { None, Plus, Minus };
368+
369+
OffsetKind LineOffsetKind = OffsetKind::None;
380370
if (!Str.empty()) {
381371
switch (Str.front()) {
382372
case '+':
383-
lineOffsetKind = LineOffsetKind::Plus;
373+
LineOffsetKind = OffsetKind::Plus;
384374
Str = Str.drop_front();
385375
break;
386376
case '-':
387-
lineOffsetKind = LineOffsetKind::Minus;
377+
LineOffsetKind = OffsetKind::Minus;
388378
Str = Str.drop_front();
389379
break;
390380
default:
391381
break;
392382
}
393383
}
394384

395-
unsigned firstNumber = LineColumnRange::NoValue;
396-
if (Str.consumeInteger(10, firstNumber)) {
397-
if (lineOffsetKind > LineOffsetKind::None) {
385+
unsigned FirstVal = 0;
386+
if (Str.consumeInteger(10, FirstVal)) {
387+
if (LineOffsetKind == OffsetKind::None) {
398388
diagnoseError(Str.data(),
399-
"expected line offset after leading '+' or '-' in fix-it "
400-
"verification");
389+
"expected line or column number in fix-it verification");
401390
} else {
402391
diagnoseError(Str.data(),
403-
"expected line or column number in fix-it verification");
392+
"expected line offset after leading '+' or '-' in fix-it "
393+
"verification");
404394
}
405395
return None;
406396
}
407397

408-
unsigned secondNumber = LineColumnRange::NoValue;
409-
if (!Str.empty() && Str.front() == ':') {
410-
Str = Str.drop_front();
411-
412-
if (Str.consumeInteger(10, secondNumber)) {
413-
diagnoseError(
414-
Str.data(),
415-
"expected column number after ':' in fix-it verification");
416-
return None;
398+
// If the first value is not followed by a colon, it is either a column or a
399+
// line offset that is missing a column.
400+
if (Str.empty() || Str.front() != ':') {
401+
if (LineOffsetKind == OffsetKind::None) {
402+
return ParsedLineAndColumn{None, FirstVal};
417403
}
418-
} else if (lineOffsetKind > LineOffsetKind::None) {
404+
419405
diagnoseError(Str.data(),
420406
"expected colon-separated column number after line offset "
421407
"in fix-it verification");
422408
return None;
423409
}
424410

425-
if (secondNumber == LineColumnRange::NoValue) {
426-
// If only one value is specified, it's a column number;
427-
return std::make_pair(LineColumnRange::NoValue, firstNumber);
411+
unsigned Column = 0;
412+
Str = Str.drop_front();
413+
if (Str.consumeInteger(10, Column)) {
414+
diagnoseError(Str.data(),
415+
"expected column number after ':' in fix-it verification");
416+
return None;
428417
}
429418

430-
unsigned lineNo = DiagnosticLineNo;
431-
switch (lineOffsetKind) {
432-
case LineOffsetKind::None:
433-
lineNo = firstNumber;
419+
// Apply the offset relative to the line of the expected diagnostic.
420+
switch (LineOffsetKind) {
421+
case OffsetKind::None:
434422
break;
435-
case LineOffsetKind::Plus:
436-
lineNo += firstNumber;
423+
case OffsetKind::Plus:
424+
FirstVal += DiagnosticLineNo;
437425
break;
438-
case LineOffsetKind::Minus:
439-
lineNo -= firstNumber;
426+
case OffsetKind::Minus:
427+
FirstVal = DiagnosticLineNo - FirstVal;
440428
break;
441429
}
442430

443-
return std::make_pair(lineNo, secondNumber);
431+
return ParsedLineAndColumn{FirstVal, Column};
444432
};
445433

446434
LineColumnRange Range;
447435

448-
if (const auto lineAndCol = parseLineAndColumn()) {
449-
std::tie(Range.StartLine, Range.StartCol) = lineAndCol.value();
436+
if (const auto LineAndCol = parseLineAndColumn()) {
437+
// The start line defaults to the line of the expected diagnostic.
438+
Range.StartLine = LineAndCol->Line.value_or(DiagnosticLineNo);
439+
Range.StartCol = LineAndCol->Column;
450440
} else {
451441
return None;
452442
}
@@ -459,8 +449,10 @@ static Optional<LineColumnRange> parseExpectedFixItRange(
459449
return None;
460450
}
461451

462-
if (const auto lineAndCol = parseLineAndColumn()) {
463-
std::tie(Range.EndLine, Range.EndCol) = lineAndCol.value();
452+
if (const auto LineAndCol = parseLineAndColumn()) {
453+
// The end line defaults to the start line.
454+
Range.EndLine = LineAndCol->Line.value_or(Range.StartLine);
455+
Range.EndCol = LineAndCol->Column;
464456
} else {
465457
return None;
466458
}

‎test/AutoDiff/SILOptimizer/differentiation_control_flow_diagnostics.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,9 @@ enum Tree : Differentiable & AdditiveArithmetic {
168168
// (`Collection.makeIterator` and `IteratorProtocol.next`).
169169
// expected-error @+1 {{function is not differentiable}}
170170
@differentiable(reverse)
171-
// expected-note @+1 {{when differentiating this function definition}}
171+
// expected-note @+2 {{when differentiating this function definition}}
172+
// expected-note @+1 {{cannot differentiate through a non-differentiable result; do you want to use 'withoutDerivative(at:)'?}} {{+2:12-12=withoutDerivative(at: }} {{+2:17-17=)}}
172173
func loop_array(_ array: [Float]) -> Float {
173-
// expected-note @-1 {{cannot differentiate through a non-differentiable result; do you want to use 'withoutDerivative(at:)'?}} {{12-12=withoutDerivative(at: }} {{17-17=)}}
174174
var result: Float = 1
175175
for x in array {
176176
result = result * x

‎test/AutoDiff/Sema/derivative_attr_type_checking.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,16 +1105,16 @@ fileprivate func _private_original_fileprivate_derivative(_ x: Float) -> (value:
11051105
public func public_original_private_derivative(_ x: Float) -> Float { x }
11061106
// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_public_original_private_derivative' is fileprivate, but original function 'public_original_private_derivative' is public}}
11071107
@derivative(of: public_original_private_derivative)
1108-
// expected-note @+1 {{mark the derivative function as '@usableFromInline' to match the original function}} {{1-1=@usableFromInline }}
11091108
fileprivate func _public_original_private_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
1109+
// expected-note @-1 {{mark the derivative function as '@usableFromInline' to match the original function}} {{-1:1-1=@usableFromInline }}
11101110
fatalError()
11111111
}
11121112

11131113
public func public_original_internal_derivative(_ x: Float) -> Float { x }
11141114
// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_public_original_internal_derivative' is internal, but original function 'public_original_internal_derivative' is public}}
11151115
@derivative(of: public_original_internal_derivative)
1116-
// expected-note @+1 {{mark the derivative function as '@usableFromInline' to match the original function}} {{1-1=@usableFromInline }}
11171116
func _public_original_internal_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
1117+
// expected-note @-1 {{mark the derivative function as '@usableFromInline' to match the original function}} {{-1:1-1=@usableFromInline }}
11181118
fatalError()
11191119
}
11201120

‎test/Concurrency/async_initializer.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,6 @@ protocol AsyncDefaultConstructable {
139139
init() async
140140
}
141141

142-
protocol DefaultConstructable {
143-
init() // expected-note {{protocol requires initializer 'init()' with type '()'; do you want to add a stub?}} {{43-43=\n init() {\n <#code#>\n \}\n}}
144-
}
145-
146142
struct Location {
147143
var x : Int
148144
var y : Int
@@ -152,6 +148,9 @@ struct Location {
152148
}
153149
}
154150

151+
protocol DefaultConstructable {
152+
init() // expected-note {{protocol requires initializer 'init()' with type '()'; do you want to add a stub?}} {{+2:43-43=\n init() {\n <#code#>\n \}\n}}
153+
}
155154
extension Location: DefaultConstructable {} // expected-error {{type 'Location' does not conform to protocol 'DefaultConstructable'}}
156155

157156
extension Location: AsyncDefaultConstructable {}

‎test/Concurrency/require-explicit-sendable.swift

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,41 +2,41 @@
22

33
public protocol P { }
44

5+
// expected-note@+2{{consider making struct 'S1' conform to the 'Sendable' protocol}}{{18-18=: Sendable}}
6+
// expected-note@+1{{make struct 'S1' explicitly non-Sendable to suppress this warning}}{{+2:2-2=\n\n@available(*, unavailable)\nextension S1: Sendable { \}\n}}
57
public struct S1 { // expected-warning{{public struct 'S1' does not specify whether it is 'Sendable' or not}}
6-
// expected-note@-1{{consider making struct 'S1' conform to the 'Sendable' protocol}}{{18-18=: Sendable}}
7-
// expected-note@-2{{make struct 'S1' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension S1: Sendable { \}\n}}
88
var str: String
99
}
1010

1111
class C { }
1212

13+
// expected-note@+2{{add '@unchecked Sendable' conformance to struct 'S2' if this type manually implements concurrency safety}}{{18-18=: @unchecked Sendable}}
14+
// expected-note@+1{{make struct 'S2' explicitly non-Sendable to suppress this warning}}{{+2:2-2=\n\n@available(*, unavailable)\nextension S2: Sendable { \}\n}}
1315
public struct S2 { // expected-warning{{public struct 'S2' does not specify whether it is 'Sendable' or not}}
14-
// expected-note@-1{{add '@unchecked Sendable' conformance to struct 'S2' if this type manually implements concurrency safety}}{{18-18=: @unchecked Sendable}}
15-
// expected-note@-2{{make struct 'S2' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension S2: Sendable { \}\n}}
1616
var c: C
1717
}
1818

19+
// expected-note@+2{{consider making class 'C1' conform to the 'Sendable' protocol}}{{25-25=, Sendable}}
20+
// expected-note@+1{{make class 'C1' explicitly non-Sendable to suppress this warning}}{{+2:2-2=\n\n@available(*, unavailable)\nextension C1: Sendable { \}\n}}
1921
final public class C1: P { // expected-warning{{public class 'C1' does not specify whether it is 'Sendable' or not}}
20-
// expected-note@-1{{consider making class 'C1' conform to the 'Sendable' protocol}}{{25-25=, Sendable}}
21-
// expected-note@-2{{make class 'C1' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension C1: Sendable { \}\n}}
2222
let str: String = ""
2323
}
2424

25+
// expected-note@+2{{add '@unchecked Sendable' conformance to class 'C2' if this type manually implements concurrency safety}}{{17-17=: @unchecked Sendable}}
26+
// expected-note@+1{{make class 'C2' explicitly non-Sendable to suppress this warning}}{{+2:2-2=\n\n@available(*, unavailable)\nextension C2: Sendable { \}\n}}
2527
public class C2 { // expected-warning{{public class 'C2' does not specify whether it is 'Sendable' or not}}
26-
// expected-note@-1{{add '@unchecked Sendable' conformance to class 'C2' if this type manually implements concurrency safety}}{{17-17=: @unchecked Sendable}}
27-
// expected-note@-2{{make class 'C2' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension C2: Sendable { \}\n}}
2828
var str: String = ""
2929
}
3030

31+
// expected-note@+2{{consider making generic struct 'S3' conform to the 'Sendable' protocol}}{{+2:2-2=\n\nextension S3: Sendable where T: Sendable { \}\n}}
32+
// expected-note@+1{{make generic struct 'S3' explicitly non-Sendable to suppress this warning}}{{+2:2-2=\n\n@available(*, unavailable)\nextension S3: Sendable { \}\n}}
3133
public struct S3<T> { // expected-warning{{public generic struct 'S3' does not specify whether it is 'Sendable' or not}}
32-
// expected-note@-1{{consider making generic struct 'S3' conform to the 'Sendable' protocol}}{{2-2=\n\nextension S3: Sendable where T: Sendable { \}\n}}
33-
// expected-note@-2{{make generic struct 'S3' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension S3: Sendable { \}\n}}
3434
var t: T
3535
}
3636

37+
// expected-note@+2{{add '@unchecked Sendable' conformance to generic struct 'S4' if this type manually implements concurrency safety}}{{+3:2-2=\n\nextension S4: @unchecked Sendable where T: Sendable { \}\n}}
38+
// expected-note@+1{{make generic struct 'S4' explicitly non-Sendable to suppress this warning}}{{+3:2-2=\n\n@available(*, unavailable)\nextension S4: Sendable { \}\n}}
3739
public struct S4<T> { // expected-warning{{public generic struct 'S4' does not specify whether it is 'Sendable' or not}}
38-
// expected-note@-1{{add '@unchecked Sendable' conformance to generic struct 'S4' if this type manually implements concurrency safety}}{{2-2=\n\nextension S4: @unchecked Sendable where T: Sendable { \}\n}}
39-
// expected-note@-2{{make generic struct 'S4' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension S4: Sendable { \}\n}}
4040
var t: T
4141
var c: C
4242
}
@@ -71,20 +71,20 @@ func testMe(s5: S5, s7: S7) {
7171
acceptSendable(s7) // expected-warning{{conformance of 'S7' to 'Sendable' is unavailable}}
7272
}
7373

74+
// expected-note@+2{{consider making generic struct 'S8' conform to the 'Sendable' protocol}}{{+2:2-2=\n\nextension S8: Sendable where T: Sendable, U: Sendable, V: Sendable { \}\n}}
75+
// expected-note@+1{{make generic struct 'S8' explicitly non-Sendable to suppress this warning}}
7476
public struct S8<T: Hashable, U, V> { // expected-warning{{public generic struct 'S8' does not specify whether it is 'Sendable' or not}}
75-
// expected-note@-1{{consider making generic struct 'S8' conform to the 'Sendable' protocol}}{{2-2=\n\nextension S8: Sendable where T: Sendable, U: Sendable, V: Sendable { \}\n}}
76-
// expected-note@-2{{make generic struct 'S8' explicitly non-Sendable to suppress this warning}}
7777
var member: [T: (U, V?)]
7878
}
7979

8080
public protocol P2 {
8181
associatedtype A
8282
}
8383

84+
// expected-warning@+3{{public generic struct 'S9' does not specify whether it is 'Sendable' or not}}
85+
// expected-note@+2{{consider making generic struct 'S9' conform to the 'Sendable' protocol}}{{+2:2-2=\n\nextension S9: Sendable where T: Sendable, T.A: Sendable { \}\n}}
86+
// expected-note@+1{{make generic struct 'S9' explicitly non-Sendable to suppress this warning}}
8487
public struct S9<T: P2 & Hashable> {
85-
// expected-warning@-1{{public generic struct 'S9' does not specify whether it is 'Sendable' or not}}
86-
// expected-note@-2{{consider making generic struct 'S9' conform to the 'Sendable' protocol}}{{2-2=\n\nextension S9: Sendable where T: Sendable, T.A: Sendable { \}\n}}
87-
// expected-note@-3{{make generic struct 'S9' explicitly non-Sendable to suppress this warning}}
8888
var dict: [T : T.A] = [:]
8989
}
9090

0 commit comments

Comments
 (0)