Skip to content

Commit e65647c

Browse files
hamishknighthamishknight
authored
Merge pull request #75574 from jamieQ/missing-return-diagnostics
[AST][SILOptimizer]: unify missing return diagnostics in some cases
2 parents 3058718 + 6c676c9 commit e65647c

File tree

9 files changed

+168
-58
lines changed

9 files changed

+168
-58
lines changed

‎include/swift/AST/DiagnosticsParse.def

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,6 @@ ERROR(enum_case_dot_prefix,none,
253253
ERROR(static_var_decl_global_scope,none,
254254
"%select{%error|static properties|class properties}0 may only be declared on a type",
255255
(StaticSpellingKind))
256-
ERROR(unexpected_curly_braces_in_decl, none,
257-
"unexpected '{' in declaration", ())
258-
ERROR(missing_accessor_return_decl,none,
259-
"missing return in %select{accessor|subscript}0 expected to return %1",
260-
(bool, TypeRepr*))
261256
ERROR(expected_getset_in_protocol,none,
262257
"expected get or set in a protocol property", ())
263258
ERROR(unexpected_getset_implementation_in_protocol,none,

‎lib/Parse/ParseDecl.cpp

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8021,35 +8021,6 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags, ParameterList *Indices,
80218021
bool parsingLimitedSyntax = Flags.contains(PD_InProtocol) ||
80228022
SF.Kind == SourceFileKind::SIL;
80238023

8024-
// If the body is completely empty, preserve it. This is at best a getter with
8025-
// an implicit fallthrough off the end.
8026-
if (peekToken().is(tok::r_brace)) {
8027-
accessors.LBLoc = consumeToken(tok::l_brace);
8028-
accessors.RBLoc = consumeToken(tok::r_brace);
8029-
8030-
// In the limited syntax, fall out and let the caller handle it.
8031-
if (parsingLimitedSyntax)
8032-
return makeParserSuccess();
8033-
8034-
if (ResultType != nullptr) {
8035-
// An error type at this point means we couldn't parse
8036-
// the result type for subscript correctly which will be
8037-
// already diagnosed as missing result type in declaration.
8038-
if (ResultType->getKind() == TypeReprKind::Error)
8039-
return makeParserError();
8040-
8041-
diagnose(accessors.RBLoc, diag::missing_accessor_return_decl,
8042-
/*subscript*/ Indices != nullptr, ResultType);
8043-
} else {
8044-
// This is supposed to be a computed property, but we don't
8045-
// have a result type representation which indicates this is probably not
8046-
// a well-formed computed property. So we can assume that empty braces
8047-
// are unexpected at this position for this declaration.
8048-
diagnose(accessors.LBLoc, diag::unexpected_curly_braces_in_decl);
8049-
}
8050-
return makeParserError();
8051-
}
8052-
80538024
auto parseImplicitGetter = [&]() {
80548025
assert(Tok.is(tok::l_brace));
80558026
accessors.LBLoc = Tok.getLoc();
@@ -8063,6 +8034,23 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags, ParameterList *Indices,
80638034
accessors.RBLoc = getter->getEndLoc();
80648035
};
80658036

8037+
// If the body is completely empty, preserve it. This is at best a getter with
8038+
// an implicit fallthrough off the end.
8039+
if (peekToken().is(tok::r_brace)) {
8040+
if (parsingLimitedSyntax) {
8041+
// In the limited syntax, fall out and let the caller handle it.
8042+
accessors.LBLoc = consumeToken(tok::l_brace);
8043+
accessors.RBLoc = consumeToken(tok::r_brace);
8044+
} else {
8045+
// Otherwise, treat the empty braces as a valid implicit getter. We need
8046+
// more information to determine whether missing return diagnostics are
8047+
// necessary.
8048+
parseImplicitGetter();
8049+
}
8050+
8051+
return makeParserSuccess();
8052+
}
8053+
80668054
// Prepare backtracking for implicit getter.
80678055
std::optional<CancellableBacktrackingScope> backtrack;
80688056
backtrack.emplace(*this);

‎test/Constraints/closures.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,12 @@ do {
9696
}
9797
inSubcall = false
9898

99-
// This is a problem, but isn't clear what was intended.
100-
var somethingElse = true { // expected-error {{unexpected '{' in declaration}}
101-
}
99+
// These are a problems, but it's not clear what was intended.
100+
var somethingElse = true {
101+
// expected-error@-1 {{computed property must have an explicit type}}
102+
// expected-error@-2 {{variable with getter/setter cannot have an initial value}}
103+
}
104+
var somethingElseWithTypeAnno: Bool = true {} // expected-error {{variable with getter/setter cannot have an initial value}}
102105
inSubcall = false
103106

104107
var v2 : Bool = false

‎test/Parse/omit_return.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -545,10 +545,10 @@ func ff_implicitMemberAccessEnumCase() -> Unit {
545545

546546

547547
var fv_nop: () {
548-
} // expected-error {{missing return in accessor expected to return '()'}}
548+
} // missing return expectations moved to `SILOptimizer/missing_returns`
549549

550550
var fv_missing: String {
551-
} // expected-error {{missing return in accessor expected to return 'String'}}
551+
} // missing return expectations moved to `SILOptimizer/missing_returns`
552552

553553
var fv_implicit: String {
554554
"hello"
@@ -1054,12 +1054,12 @@ var fvs_optionalTryImplicit: String? {
10541054

10551055
enum S_nop {
10561056
subscript() -> () {
1057-
} // expected-error {{missing return in subscript expected to return '()'}}
1057+
} // missing return expectations moved to `SILOptimizer/missing_returns`
10581058
}
10591059

10601060
enum S_missing {
10611061
subscript() -> String {
1062-
} // expected-error {{missing return in subscript expected to return 'String'}}
1062+
} // missing return expectations moved to `SILOptimizer/missing_returns`
10631063
}
10641064

10651065
enum S_implicit {

‎test/SILOptimizer/missing_returns.swift

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// RUN: %target-swift-frontend %s -emit-sil -verify
2+
3+
// MARK: Relocated Test Cases
4+
// Missing return diagnostics used to also be implemented during parsing/AST
5+
// construction in addition to the SIL passes. Some existing test cases have
6+
// been moved here after removing the earlier phases' diagnostics in favor of
7+
// those implemented via the SIL passes.
8+
9+
// MARK: `decl/subscript/subscripting`
10+
11+
struct MissingGetterSubscript1 {
12+
subscript (i : Int) -> Int {
13+
} // expected-error {{missing return in getter expected to return 'Int'}}
14+
}
15+
16+
// MARK: `decl/var/properties`
17+
18+
struct X {}
19+
20+
var x13: X {} // expected-error {{missing return in getter expected to return 'X'}}
21+
22+
struct X14 {}
23+
extension X14 {
24+
var x14: X {
25+
} // expected-error {{missing return in getter expected to return 'X'}}
26+
}
27+
28+
// https://github.com/apple/swift/issues/57936
29+
30+
enum E1_57936 {
31+
var foo: Int {} // expected-error{{missing return in getter expected to return 'Int'}}
32+
}
33+
34+
enum E2_57936<T> {
35+
var foo: T {} // expected-error{{missing return in getter expected to return 'T'}}
36+
}
37+
38+
// MARK: `decl/var/result_builders`
39+
40+
@resultBuilder
41+
struct Maker {
42+
static func buildBlock() -> Int { 42 }
43+
}
44+
45+
@Maker
46+
var globalWithEmptyImplicitGetter: Int {}
47+
48+
// MARK: `Parse/omit_return`
49+
50+
var fv_nop: () {
51+
}
52+
53+
var fv_missing: String {
54+
} // expected-error {{missing return in getter expected to return 'String'}}
55+
56+
enum S_nop {
57+
subscript() -> () {
58+
}
59+
}
60+
61+
enum S_missing {
62+
subscript() -> String {
63+
} // expected-error {{missing return in getter expected to return 'String'}}
64+
}
65+
66+
// MARK: `Sema/generic-subscript`
67+
68+
struct S_generic_subscript_missing_return {
69+
subscript<Value>(x: Int) -> Value {
70+
} // expected-error {{missing return in getter expected to return 'Value'}}
71+
}
72+
73+
// MARK: New Test Cases
74+
75+
enum MyEmptyType {}
76+
extension MyEmptyType {
77+
var i: Int {} // expected-error{{missing return in getter expected to return 'Int'}}
78+
var n: MyEmptyType {} // expected-error{{getter with uninhabited return type 'MyEmptyType' is missing call to another never-returning function on all paths}}
79+
80+
static subscript<A>(root: MyEmptyType) -> A {}
81+
82+
subscript(_ e: MyEmptyType) -> Int {}
83+
subscript<T>(_ e: MyEmptyType) -> T {}
84+
subscript(_ i: Int) -> Int {} // expected-error{{missing return in getter expected to return 'Int'}}
85+
subscript<T>(_ p: Int) -> T {} // expected-error{{missing return in getter expected to return 'T'}}
86+
subscript(_ i: Int) -> Self {} // expected-error{{getter with uninhabited return type 'MyEmptyType' is missing call to another never-returning function on all paths}}
87+
subscript(_ s: Self) -> Self {}
88+
89+
static func unreachable_static_implicit_return(_ e: MyEmptyType) -> Int {}
90+
func unreachable(_ e: MyEmptyType) -> Int { // expected-note{{'e' is of type 'MyEmptyType' which cannot be constructed because it is an enum with no cases}}
91+
42 // expected-warning{{will never be executed}}
92+
}
93+
94+
// FIXME: should these produce warnings since they implicity take an uninhabited 'self' param?
95+
func implicitly_unreachable() { _ = 42 }
96+
func implicitly_unreachable_implicit_return() -> Int { 42 }
97+
}
98+
99+
extension Never {
100+
var i: Int {} // expected-error{{missing return in getter expected to return 'Int'}}
101+
var n: Never {} // expected-error{{getter with uninhabited return type 'Never' is missing call to another never-returning function on all paths}}
102+
103+
static subscript<A>(root: Never) -> A {}
104+
105+
subscript(_ n: Never) -> Int {}
106+
subscript<T>(_ e: Never) -> T {}
107+
subscript(_ i: Int) -> Int {} // expected-error{{missing return in getter expected to return 'Int'}}
108+
subscript<T>(_ p: Int) -> T {} // expected-error{{missing return in getter expected to return 'T'}}
109+
subscript(_ i: Int) -> Self {} // expected-error{{getter with uninhabited return type 'Never' is missing call to another never-returning function on all paths}}
110+
subscript(_ s: Self) -> Self {}
111+
112+
static func unreachable_static_implicit_return(_ n: Never) -> Int {}
113+
func unreachable(_ n: Never) -> Int { // expected-note{{'n' is of type 'Never' which cannot be constructed because it is an enum with no cases}}
114+
42 // expected-warning{{will never be executed}}
115+
}
116+
117+
// FIXME: should these produce unreachable code warnings since they implicity take an uninhabited 'self' param?
118+
func implicitly_unreachable() { _ = 42 }
119+
func implicitly_unreachable_implicit_return() -> Int { 42 }
120+
}
121+
122+
enum InhabitedType {
123+
case inhabitant
124+
125+
// Uninhabited params
126+
subscript(_ n: Never) -> Int {}
127+
subscript<T>(_ e: Never) -> T {}
128+
subscript(_ v: MyEmptyType, e: Int) -> Never {}
129+
130+
// Inhabited params
131+
subscript(_ i: Int) -> Int {} // expected-error{{missing return in getter expected to return 'Int'}}
132+
subscript(_ j: Int) -> Void {}
133+
subscript(_ k: Int) -> Never {} // expected-error{{getter with uninhabited return type 'Never' is missing call to another never-returning function on all paths}}
134+
// FIXME: ^ this diagnostic should probably use the word 'subscript' rather than 'getter'
135+
}

‎test/Sema/generic-subscript.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ protocol P {
88

99
struct S : P { // expected-error {{type 'S' does not conform to protocol 'P'}} expected-note {{add stubs for conformance}}
1010
subscript<Value>(x: Int) -> Value { // expected-note {{candidate has non-matching type '<Value> (Int) -> Value'}}
11-
} // expected-error {{missing return in subscript expected to return 'Value'}}
11+
} // missing return expectations moved to `SILOptimizer/missing_returns`
1212
}
1313

1414
struct S2: P {

‎test/decl/subscript/subscripting.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ struct RetOverloadedSubscript {
226226

227227
struct MissingGetterSubscript1 {
228228
subscript (i : Int) -> Int {
229-
} // expected-error {{missing return in subscript expected to return 'Int'}}
229+
} // missing return expectations moved to `SILOptimizer/missing_returns`
230230
}
231231
struct MissingGetterSubscript2 {
232232
subscript (i : Int, j : Int) -> Int {

‎test/decl/var/properties.swift

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,12 +378,12 @@ var x12: X {
378378
}
379379
}
380380

381-
var x13: X {} // expected-error {{missing return in accessor expected to return 'X'}}
381+
var x13: X {} // missing return expectations moved to `SILOptimizer/missing_returns`
382382

383383
struct X14 {}
384384
extension X14 {
385385
var x14: X {
386-
} // expected-error {{missing return in accessor expected to return 'X'}}
386+
} // missing return expectations moved to `SILOptimizer/missing_returns`
387387
}
388388

389389
// Type checking problems
@@ -1343,13 +1343,3 @@ class LazyPropInClass {
13431343
lazy var foo: Int = { return 0 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}}
13441344
// expected-note@-1 {{Remove '=' to make 'foo' a computed property}}{{21-23=}}{{3-8=}}
13451345
}
1346-
1347-
// https://github.com/apple/swift/issues/57936
1348-
1349-
enum E1_57936 {
1350-
var foo: Int {} // expected-error{{missing return in accessor expected to return 'Int'}}
1351-
}
1352-
1353-
enum E2_57936<T> {
1354-
var foo: T {} // expected-error{{missing return in accessor expected to return 'T'}}
1355-
}

‎test/decl/var/result_builders.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@ typealias typename = Inventor
1818
@Maker // expected-error {{result builder attribute 'Maker' can only be applied to a variable if it defines a getter}}
1919
var global: Int
2020

21-
// FIXME: should this be allowed?
2221
@Maker
2322
var globalWithEmptyImplicitGetter: Int {}
24-
// expected-error@-1 {{missing return in accessor expected to return 'Int'}}
25-
// expected-error@-3 {{result builder attribute 'Maker' can only be applied to a variable if it defines a getter}}
23+
// expected-error@-1{{result builder 'Maker' does not implement any 'buildBlock' or a combination of 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)' with sufficient availability for this call site}}
24+
// Note: no missing return error is expected in this case. Similar test added to `SILOptimizer/missing_returns` to verify SIL diagnostic behavior.
2625

2726
@Maker
2827
var globalWithEmptyExplicitGetter: Int { get {} } // expected-error{{result builder 'Maker' does not implement any 'buildBlock' or a combination of 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)' with sufficient availability for this call site}}

0 commit comments

Comments
 (0)