Skip to content

Commit d113ca6

Browse files
AnthonyLatsisAnthonyLatsis
committed
CSApply: Fix several issues with non-failable to failable/throwing initializer delegation/chaining diagnostics
* Delegated-to `Optional` ctors were always handled as if they were failable, resulting in false-positive delegation errors. * Delegations via `try?` were not diagnosed if the called ctor had IUO failability on top of being `throws`. SILGen would then handle the `try?` as if it was a `try!`. * Delegations via `try?` were diagnosed with the wrong message if the `try?` was nested inside a `try!`, e.g. `try! try self.init(nonFailable:)` * If there are issues with both `try?` and failability of the called initializer, diagnose both.
1 parent 967e3c7 commit d113ca6

File tree

2 files changed

+177
-42
lines changed

2 files changed

+177
-42
lines changed

‎lib/Sema/CSApply.cpp

Lines changed: 66 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3636,57 +3636,89 @@ namespace {
36363636
}
36373637

36383638
Expr *visitRebindSelfInConstructorExpr(RebindSelfInConstructorExpr *expr) {
3639-
// A non-failable initializer cannot delegate to a failable
3640-
// initializer.
3641-
Expr *unwrappedSubExpr = expr->getSubExpr()->getSemanticsProvidingExpr();
3642-
Type valueTy = cs.getType(unwrappedSubExpr)->getOptionalObjectType();
3643-
auto inCtor = cast<ConstructorDecl>(dc->getInnermostMethodContext());
3644-
if (valueTy && !inCtor->isFailable()) {
3639+
Expr *subExpr = expr->getSubExpr();
3640+
auto *inCtor = cast<ConstructorDecl>(dc->getInnermostMethodContext());
3641+
3642+
// A non-failable initializer cannot delegate nor chain to
3643+
// * a throwing initializer via 'try?'
3644+
// * a failable initializer, unless the failability kind is IUO or the
3645+
// result is explicitly forced
3646+
if (!inCtor->isFailable() && cs.getType(subExpr)->isOptional()) {
36453647
bool isChaining;
36463648
auto *otherCtorRef = expr->getCalledConstructor(isChaining);
3647-
ConstructorDecl *ctor = otherCtorRef->getDecl();
3648-
assert(ctor);
3649+
auto *otherCtor = otherCtorRef->getDecl();
3650+
assert(otherCtor);
36493651

3650-
// If the initializer we're calling is not declared as
3651-
// checked, it's an error.
3652-
bool isError = !ctor->isImplicitlyUnwrappedOptional();
3652+
Expr *newSubExpr = subExpr;
3653+
auto &de = cs.getASTContext().Diags;
36533654

3654-
// If we're suppressing diagnostics, just fail.
3655-
if (isError && SuppressDiagnostics)
3656-
return nullptr;
3655+
// Look through 'try', 'try!', and identity expressions.
3656+
subExpr = subExpr->getValueProvidingExpr();
36573657

3658-
auto &ctx = cs.getASTContext();
3658+
// Diagnose if we find a 'try?'.
3659+
// FIXME: We could put up with occurences of 'try?' if they do not apply
3660+
// directly to the called ctor, e.g. 'try? try self.init()', or if the
3661+
// called ctor isn't throwing.
3662+
if (auto *OTE = dyn_cast<OptionalTryExpr>(subExpr)) {
3663+
if (SuppressDiagnostics)
3664+
return nullptr;
3665+
3666+
// Give the user the option of using 'try!' or making the enclosing
3667+
// initializer failable.
3668+
de.diagnose(OTE->getTryLoc(),
3669+
diag::delegate_chain_nonoptional_to_optional_try,
3670+
isChaining);
3671+
de.diagnose(OTE->getTryLoc(), diag::init_delegate_force_try)
3672+
.fixItReplace({OTE->getTryLoc(), OTE->getQuestionLoc()}, "try!");
3673+
de.diagnose(inCtor->getLoc(), diag::init_propagate_failure)
3674+
.fixItInsertAfter(inCtor->getLoc(), "?");
3675+
3676+
subExpr = OTE->getSubExpr();
3677+
}
3678+
3679+
while (true) {
3680+
subExpr = subExpr->getSemanticsProvidingExpr();
3681+
3682+
// Look through optional injections.
3683+
if (auto *IIOE = dyn_cast<InjectIntoOptionalExpr>(subExpr)) {
3684+
subExpr = IIOE->getSubExpr();
3685+
continue;
3686+
}
3687+
3688+
// Look through all try expressions.
3689+
if (auto *ATE = dyn_cast<AnyTryExpr>(subExpr)) {
3690+
subExpr = ATE->getSubExpr();
3691+
continue;
3692+
}
3693+
3694+
break;
3695+
}
3696+
3697+
// If we're still hitting an optional and the called ctor is failable,
3698+
// diagnose only if the failability kind is not IUO. Note that the
3699+
// expression type alone is not always indicative of the ctor's
3700+
// failability, because it could be declared on 'Optional' itself.
3701+
if (cs.getType(subExpr)->isOptional() && otherCtor->isFailable()) {
3702+
if (!otherCtor->isImplicitlyUnwrappedOptional()) {
3703+
if (SuppressDiagnostics)
3704+
return nullptr;
36593705

3660-
auto &de = cs.getASTContext().Diags;
3661-
if (isError) {
3662-
if (auto *optTry = dyn_cast<OptionalTryExpr>(unwrappedSubExpr)) {
3663-
de.diagnose(optTry->getTryLoc(),
3664-
diag::delegate_chain_nonoptional_to_optional_try,
3665-
isChaining);
3666-
de.diagnose(optTry->getTryLoc(), diag::init_delegate_force_try)
3667-
.fixItReplace({optTry->getTryLoc(), optTry->getQuestionLoc()},
3668-
"try!");
3669-
de.diagnose(inCtor->getLoc(), diag::init_propagate_failure)
3670-
.fixItInsertAfter(inCtor->getLoc(), "?");
3671-
} else {
36723706
// Give the user the option of adding '!' or making the enclosing
36733707
// initializer failable.
36743708
de.diagnose(otherCtorRef->getLoc(),
36753709
diag::delegate_chain_nonoptional_to_optional,
3676-
isChaining, ctor->getName());
3710+
isChaining, otherCtor->getName());
36773711
de.diagnose(otherCtorRef->getLoc(), diag::init_force_unwrap)
36783712
.fixItInsertAfter(expr->getEndLoc(), "!");
36793713
de.diagnose(inCtor->getLoc(), diag::init_propagate_failure)
36803714
.fixItInsertAfter(inCtor->getLoc(), "?");
36813715
}
3716+
3717+
// Recover by injecting the force operation (the first option).
3718+
newSubExpr = forceUnwrapIUO(newSubExpr);
36823719
}
36833720

3684-
// Recover by injecting the force operation (the first option).
3685-
Expr *newSub = new (ctx) ForceValueExpr(expr->getSubExpr(),
3686-
expr->getEndLoc());
3687-
cs.setType(newSub, valueTy);
3688-
newSub->setImplicit();
3689-
expr->setSubExpr(newSub);
3721+
expr->setSubExpr(newSubExpr);
36903722
}
36913723

36923724
return expr;

‎test/decl/init/failable.swift

Lines changed: 111 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
// RUN: %target-typecheck-verify-swift
1+
// Run in compatibility modes that disable and enable optional flattening
2+
// in 'try?' to verify that initializer delegation diagnostics that are related
3+
// to 'try?' are stable.
4+
// RUN: %target-typecheck-verify-swift -swift-version 4
5+
// RUN: %target-typecheck-verify-swift -swift-version 5
26

37
// REQUIRES: objc_interop
48
import Foundation
@@ -60,7 +64,7 @@ class Sub : Super {
6064
}
6165

6266
convenience init(forceNonfail: Int) {
63-
self.init(nonfail: forceNonfail)! // expected-error{{cannot force unwrap value of non-optional type 'Sub'}} {{37-38=}}
67+
self.init(nonfail: forceNonfail)! // expected-error{{cannot force unwrap value of non-optional type}} {{37-38=}}
6468
}
6569

6670
init(nonfail2: Int) { // okay, traps on nil
@@ -143,16 +147,115 @@ extension Super {
143147
}
144148

145149
struct SomeStruct {
146-
init(nonFail: Int) { // expected-note{{propagate the failure with 'init?'}}{{8-8=?}}
147-
self.init(fail: nonFail) // expected-error{{a non-failable initializer cannot delegate to failable initializer 'init(fail:)' written with 'init?'}}
148-
// expected-note@-1{{force potentially-failing result with '!'}}{{29-29=!}}
150+
init?(failable: Void) {}
151+
init!(failableIUO: Void) {}
152+
init(throws: Void) throws {}
153+
init?(failableAndThrows: Void) throws {}
154+
init!(failableIUOAndThrows: Void) throws {}
155+
156+
init(delegationOk1: Void) {
157+
self.init(failable: ())!
158+
}
159+
160+
init(delegationOk2: Void) {
161+
try! self.init(throws: ())
162+
}
163+
164+
init(delegationOk3: Void) {
165+
try! self.init(failableAndThrows: ())!
166+
}
167+
168+
init(delegationOk4: Void) {
169+
try! self.init(failableIUOAndThrows: ())
170+
}
171+
172+
init(nonFailable: Void) { // expected-note{{propagate the failure with 'init?'}}{{7-7=?}}
173+
self.init(failable: ()) // expected-error{{a non-failable initializer cannot delegate to failable initializer 'init(failable:)' written with 'init?'}}
174+
// expected-note@-1{{force potentially-failing result with '!'}}{{28-28=!}}
175+
}
176+
177+
init(delegationBad1: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
178+
try? self.init(nonFailable: ())
179+
// expected-warning@-1 {{no calls to throwing functions occur within 'try' expression}}
180+
// expected-error@-2 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
181+
// expected-note@-3 {{force potentially-failing result with 'try!'}}{{5-9=try!}}
182+
}
183+
184+
init(delegationBad2: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
185+
try? self.init(failableIUO: ())
186+
// expected-warning@-1 {{no calls to throwing functions occur within 'try' expression}}
187+
// expected-error@-2 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
188+
// expected-note@-3 {{force potentially-failing result with 'try!'}}{{5-9=try!}}
189+
}
190+
191+
init(delegationBad3: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
192+
try? self.init(throws: ())
193+
// expected-error@-1 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
194+
// expected-note@-2 {{force potentially-failing result with 'try!'}}{{5-9=try!}}
195+
}
196+
197+
init(delegationBad4: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
198+
try! try? self.init(throws: ())
199+
// expected-warning@-1 {{no calls to throwing functions occur within 'try' expression}}
200+
// expected-error@-2 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
201+
// expected-note@-3 {{force potentially-failing result with 'try!'}}{{10-14=try!}}
202+
}
203+
204+
init(delegationBad5: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
205+
try try? self.init(throws: ())
206+
// expected-warning@-1 {{no calls to throwing functions occur within 'try' expression}}
207+
// expected-error@-2 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
208+
// expected-note@-3 {{force potentially-failing result with 'try!'}}{{9-13=try!}}
209+
}
210+
211+
init(delegationBad6: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
212+
try? self.init(failableAndThrows: ())!
213+
// expected-error@-1 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
214+
// expected-note@-2 {{force potentially-failing result with 'try!'}}{{5-9=try!}}
215+
}
216+
217+
init(delegationBad7: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
218+
try! self.init(failableAndThrows: ())
219+
// expected-error@-1 {{a non-failable initializer cannot delegate to failable initializer 'init(failableAndThrows:)' written with 'init?'}}
220+
// expected-note@-2 {{force potentially-failing result with '!'}}{{42-42=!}}
221+
}
222+
223+
init(delegationBad8: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
224+
try? self.init(failableIUOAndThrows: ())
225+
// expected-error@-1 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
226+
// expected-note@-2 {{force potentially-failing result with 'try!'}}{{5-9=try!}}
227+
}
228+
229+
init(delegationBad9: Void) { // expected-note 2 {{propagate the failure with 'init?'}}{{7-7=?}}
230+
try? self.init(failableAndThrows: ())
231+
// expected-error@-1 {{a non-failable initializer cannot use 'try?' to delegate to another initializer}}
232+
// expected-error@-2 {{a non-failable initializer cannot delegate to failable initializer 'init(failableAndThrows:)' written with 'init?'}}
233+
// expected-note@-3 {{force potentially-failing result with '!'}}{{42-42=!}}
234+
// expected-note@-4 {{force potentially-failing result with 'try!'}}{{5-9=try!}}
235+
}
236+
}
237+
238+
extension Optional {
239+
init(delegationOk1: Void) {
240+
self.init(nonFailable: ())
241+
}
242+
243+
init?(delegationOk2: Void) {
244+
self.init(nonFailable: ())
245+
}
246+
247+
init?(delegationOk3: Void) {
248+
self.init(failable: ())
149249
}
150250

151-
init(nonFail2: Int) {
152-
self.init(fail: nonFail2)!
251+
init(delegationBad1: Void) { // expected-note {{propagate the failure with 'init?'}}{{7-7=?}}
252+
self.init(failable: ())
253+
// expected-error@-1 {{a non-failable initializer cannot delegate to failable initializer 'init(failable:)' written with 'init?'}}
254+
// expected-note@-2 {{force potentially-failing result with '!'}}{{28-28=!}}
153255
}
154256

155-
init?(fail: Int) {}
257+
init(nonFailable: Void) {}
258+
init?(failable: Void) {}
156259
}
157260

158261
// ----------------------------------------------------------------------------

0 commit comments

Comments
 (0)