Skip to content

Commit 6c676c9

Browse files
committed
[AST][SILOptimizer]: unify missing return diagnostics in some cases
Previously, missing return diagnostics for unreachable subscripts differed from the treatment unreachable functions received, leading to inconsistent diagnostic behavior. This change removes the responsibility for handling the relevant diagnostics from the AST code, in favor of the diagnostics implemented via the SIL optimizer. Additionally, where the AST-generation code would previously have diagnosed a missing return for an implicit empty getter, it will now admit as valid, deferring the missing return diagnostics to the later SIL passes.
1 parent 4efb210 commit 6c676c9

File tree

9 files changed

+168
-58
lines changed

9 files changed

+168
-58
lines changed

include/swift/AST/DiagnosticsParse.def

-5
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

+17-29
Original file line numberDiff line numberDiff line change
@@ -8013,35 +8013,6 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags, ParameterList *Indices,
80138013
bool parsingLimitedSyntax = Flags.contains(PD_InProtocol) ||
80148014
SF.Kind == SourceFileKind::SIL;
80158015

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

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

test/Constraints/closures.swift

+6-3
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

+4-4
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 {
+135
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

+1-1
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'}}
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

+1-1
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

+2-12
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

+2-3
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)