Skip to content

Commit 4df3747

Browse files
jamieQjamieQ
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 4df3747

File tree

8 files changed

+163
-35
lines changed

8 files changed

+163
-35
lines changed

‎include/swift/AST/DiagnosticsParse.def

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,6 @@ ERROR(static_var_decl_global_scope,none,
255255
(StaticSpellingKind))
256256
ERROR(unexpected_curly_braces_in_decl, none,
257257
"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*))
261258
ERROR(expected_getset_in_protocol,none,
262259
"expected get or set in a protocol property", ())
263260
ERROR(unexpected_getset_implementation_in_protocol,none,

‎lib/Parse/ParseDecl.cpp

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8013,26 +8013,51 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags, ParameterList *Indices,
80138013
bool parsingLimitedSyntax = Flags.contains(PD_InProtocol) ||
80148014
SF.Kind == SourceFileKind::SIL;
80158015

8016+
auto parseImplicitGetter = [&]() {
8017+
assert(Tok.is(tok::l_brace));
8018+
accessors.LBLoc = Tok.getLoc();
8019+
auto *getter = AccessorDecl::createParsed(
8020+
Context, AccessorKind::Get, storage, /*declLoc*/ Tok.getLoc(),
8021+
/*accessorKeywordLoc*/ SourceLoc(), /*paramList*/ nullptr,
8022+
/*asyncLoc*/ SourceLoc(), /*throwsLoc*/ SourceLoc(),
8023+
/*thrownTy*/ nullptr, CurDeclContext);
8024+
accessors.add(getter);
8025+
parseAbstractFunctionBody(getter);
8026+
accessors.RBLoc = getter->getEndLoc();
8027+
};
8028+
80168029
// If the body is completely empty, preserve it. This is at best a getter with
80178030
// an implicit fallthrough off the end.
80188031
if (peekToken().is(tok::r_brace)) {
8019-
accessors.LBLoc = consumeToken(tok::l_brace);
8020-
accessors.RBLoc = consumeToken(tok::r_brace);
8032+
auto consumeEmptyBraces = [&]() {
8033+
accessors.LBLoc = consumeToken(tok::l_brace);
8034+
accessors.RBLoc = consumeToken(tok::r_brace);
8035+
};
80218036

80228037
// In the limited syntax, fall out and let the caller handle it.
8023-
if (parsingLimitedSyntax)
8038+
if (parsingLimitedSyntax) {
8039+
consumeEmptyBraces();
80248040
return makeParserSuccess();
8041+
}
80258042

80268043
if (ResultType != nullptr) {
80278044
// An error type at this point means we couldn't parse
80288045
// the result type for subscript correctly which will be
80298046
// already diagnosed as missing result type in declaration.
8030-
if (ResultType->getKind() == TypeReprKind::Error)
8047+
if (ResultType->getKind() == TypeReprKind::Error) {
8048+
consumeEmptyBraces();
80318049
return makeParserError();
8050+
}
80328051

8033-
diagnose(accessors.RBLoc, diag::missing_accessor_return_decl,
8034-
/*subscript*/ Indices != nullptr, ResultType);
8052+
// Otherwise treat this as a 'success' from a parsing perspective. We
8053+
// need more information to accurately diagnose whether a return is
8054+
// actually necessary here (e.g. if the empty getter is unreachable,
8055+
// we admit it).
8056+
parseImplicitGetter();
8057+
return makeParserSuccess();
80358058
} else {
8059+
consumeEmptyBraces();
8060+
80368061
// This is supposed to be a computed property, but we don't
80378062
// have a result type representation which indicates this is probably not
80388063
// a well-formed computed property. So we can assume that empty braces
@@ -8042,19 +8067,6 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags, ParameterList *Indices,
80428067
return makeParserError();
80438068
}
80448069

8045-
auto parseImplicitGetter = [&]() {
8046-
assert(Tok.is(tok::l_brace));
8047-
accessors.LBLoc = Tok.getLoc();
8048-
auto *getter = AccessorDecl::createParsed(
8049-
Context, AccessorKind::Get, storage, /*declLoc*/ Tok.getLoc(),
8050-
/*accessorKeywordLoc*/ SourceLoc(), /*paramList*/ nullptr,
8051-
/*asyncLoc*/ SourceLoc(), /*throwsLoc*/ SourceLoc(),
8052-
/*thrownTy*/ nullptr, CurDeclContext);
8053-
accessors.add(getter);
8054-
parseAbstractFunctionBody(getter);
8055-
accessors.RBLoc = getter->getEndLoc();
8056-
};
8057-
80588070
// Prepare backtracking for implicit getter.
80598071
std::optional<CancellableBacktrackingScope> backtrack;
80608072
backtrack.emplace(*this);

‎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: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
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() -> Int { 42 }
96+
func implicitly_unreachable_implicit_return() -> Int { 27 }
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() -> Int { 42 }
119+
func implicitly_unreachable_implicit_return() -> Int { 27 }
120+
}

‎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'}}
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: 4 additions & 4 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
@@ -1347,9 +1347,9 @@ class LazyPropInClass {
13471347
// https://github.com/apple/swift/issues/57936
13481348

13491349
enum E1_57936 {
1350-
var foo: Int {} // expected-error{{missing return in accessor expected to return 'Int'}}
1350+
var foo: Int {} // missing return expectations moved to `SILOptimizer/missing_returns`
13511351
}
13521352

13531353
enum E2_57936<T> {
1354-
var foo: T {} // expected-error{{missing return in accessor expected to return 'T'}}
1354+
var foo: T {} // missing return expectations moved to `SILOptimizer/missing_returns`
13551355
}

‎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+
// missing return expectations moved to `SILOptimizer/missing_returns`
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)