Skip to content

Commit 4ee6ead

Browse files
committed
[ConstraintSystem] Implicitly force results of @optional protocol requirements.
We were not handling IUO results of @optional protocol methods properly, sometimes forcing the @optional requirement rather than the result of the call. Fixes rdar://problem/37240984.
1 parent b5f19d6 commit 4ee6ead

File tree

4 files changed

+102
-10
lines changed

4 files changed

+102
-10
lines changed

lib/Sema/CSApply.cpp

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,8 @@ namespace {
10651065
// We also need to handle the implicitly unwrap of the result
10661066
// of the called function if that's the type checking solution
10671067
// we ended up with.
1068-
return forceUnwrapIfExpected(ref, member, memberLocator);
1068+
return forceUnwrapIfExpected(ref, member, memberLocator,
1069+
member->getAttrs().hasAttribute<OptionalAttr>());
10691070
}
10701071

10711072
// For types and properties, build member references.
@@ -2396,9 +2397,22 @@ namespace {
23962397
return expr;
23972398
}
23982399

2399-
Expr *forceUnwrapResult(Expr *expr) {
2400+
// Add a forced unwrap of an expression which either has type Optional<T>
2401+
// or is a function that returns an Optional<T>. The latter turns into a
2402+
// conversion expression that we will hoist above the ApplyExpr
2403+
// that needs to be forced during the process of rewriting the expression.
2404+
//
2405+
// forForcedOptional is used to indicate that we will further need
2406+
// to hoist this result above an explicit force of an optional that is
2407+
// in place for something like an @optional protocol member from
2408+
// Objective C that we might otherwise mistake for the thing we mean to
2409+
// force here.
2410+
Expr *forceUnwrapResult(Expr *expr, bool forForcedOptional =false) {
24002411
auto ty = simplifyType(cs.getType(expr));
24012412

2413+
if (forForcedOptional)
2414+
ty = ty->getOptionalObjectType();
2415+
24022416
if (auto *fnTy = ty->getAs<AnyFunctionType>()) {
24032417
auto underlyingType = cs.replaceFinalResultTypeWithUnderlying(fnTy);
24042418

@@ -2422,12 +2436,13 @@ namespace {
24222436
}
24232437

24242438
Expr *forceUnwrapIfExpected(Expr *expr, Decl *decl,
2425-
ConstraintLocatorBuilder locator) {
2439+
ConstraintLocatorBuilder locator,
2440+
bool forForcedOptional =false) {
24262441
if (!shouldForceUnwrapResult(decl, locator))
24272442
return expr;
24282443

24292444
// Force the expression if required for the solution.
2430-
return forceUnwrapResult(expr);
2445+
return forceUnwrapResult(expr, forForcedOptional);
24312446
}
24322447

24332448
Expr *visitDeclRefExpr(DeclRefExpr *expr) {
@@ -3774,9 +3789,29 @@ namespace {
37743789
}
37753790

37763791
Expr *visitForceValueExpr(ForceValueExpr *expr) {
3792+
// Check to see if we are forcing an
3793+
// ImplicitlyUnwrappedFunctionConversionExpr. This can happen
3794+
// in cases where we had a ForceValueExpr of an optional for a
3795+
// declaration for a function whose result type we need to
3796+
// implicitly force after applying. We need to hoist the function
3797+
// conversion above the ForceValueExpr, so that we may ultimately
3798+
// hoist it above the ApplyExpr where we will eventually rewrite the
3799+
// function conversion into a force of the result.
3800+
Expr *replacement = expr;
3801+
if (auto fnConv =
3802+
dyn_cast<ImplicitlyUnwrappedFunctionConversionExpr>(expr->getSubExpr())) {
3803+
auto fnConvSubExpr = fnConv->getSubExpr();
3804+
auto fnConvSubObjTy =
3805+
cs.getType(fnConvSubExpr)->getOptionalObjectType();
3806+
cs.setType(expr, fnConvSubObjTy);
3807+
expr->setSubExpr(fnConvSubExpr);
3808+
fnConv->setSubExpr(expr);
3809+
replacement = fnConv;
3810+
}
3811+
37773812
Type valueType = simplifyType(cs.getType(expr));
37783813
cs.setType(expr, valueType);
3779-
3814+
37803815
// Coerce the object type, if necessary.
37813816
auto subExpr = expr->getSubExpr();
37823817
if (auto objectTy = cs.getType(subExpr)->getOptionalObjectType()) {
@@ -3789,7 +3824,7 @@ namespace {
37893824
}
37903825
}
37913826

3792-
return expr;
3827+
return replacement;
37933828
}
37943829

37953830
Expr *visitOpenExistentialExpr(OpenExistentialExpr *expr) {

lib/Sema/ConstraintSystem.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,7 +1565,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
15651565
Type openedFullType;
15661566

15671567
bool isDynamicResult = choice.getKind() == OverloadChoiceKind::DeclViaDynamic;
1568-
bool createdDynamicResultDisjunction = false;
1568+
bool bindConstraintCreated = false;
15691569

15701570
switch (auto kind = choice.getKind()) {
15711571
case OverloadChoiceKind::Decl:
@@ -1622,6 +1622,19 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
16221622
//
16231623
// Subscript declarations are handled within
16241624
// getTypeOfMemberReference(); their result types are optional.
1625+
1626+
// Deal with values declared as implicitly unwrapped, or
1627+
// functions with return types that are implicitly unwrapped.
1628+
if (choice.isImplicitlyUnwrappedValueOrReturnValue()) {
1629+
// Build the disjunction to attempt binding both T? and T (or
1630+
// function returning T? and function returning T).
1631+
Type ty = createTypeVariable(locator, TVO_CanBindToInOut);
1632+
buildDisjunctionForImplicitlyUnwrappedOptional(ty, refType,
1633+
locator);
1634+
addConstraint(ConstraintKind::Bind, boundType,
1635+
OptionalType::get(ty->getRValueType()), locator);
1636+
bindConstraintCreated = true;
1637+
}
16251638
refType = OptionalType::get(refType->getRValueType());
16261639
}
16271640
// For a non-subscript declaration found via dynamic lookup, strip
@@ -1696,7 +1709,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
16961709
refType = OptionalType::get(refType->getRValueType());
16971710
}
16981711

1699-
createdDynamicResultDisjunction = true;
1712+
bindConstraintCreated = true;
17001713
}
17011714

17021715
// If the declaration is unavailable, note that in the score.
@@ -1785,8 +1798,8 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
17851798
openedFullType,
17861799
refType};
17871800

1788-
// We created appropriate disjunctions for dynamic result above.
1789-
if (!createdDynamicResultDisjunction) {
1801+
// In some cases we already created the appropriate bind constraints.
1802+
if (!bindConstraintCreated) {
17901803
if (choice.isImplicitlyUnwrappedValueOrReturnValue()) {
17911804
// Build the disjunction to attempt binding both T? and T (or
17921805
// function returning T? and function returning T).

test/Constraints/iuo_objc.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -verify %s
2+
// REQUIRES: objc_interop
3+
4+
import Foundation
5+
6+
func iuo_error(prop: IUOProperty) {
7+
let _: Coat? = prop.iuo.optional()
8+
// expected-error@-1 {{value of optional type '(() -> Coat?)?' not unwrapped; did you mean to use '!' or '?'?}}
9+
let _: Coat? = prop.iuo.optional()!
10+
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
11+
let _: Coat? = prop.iuo.optional!()
12+
let _: Coat? = prop.iuo.optional!()!
13+
let _: Coat? = prop.iuo!.optional()
14+
// expected-error@-1 {{value of optional type '(() -> Coat?)?' not unwrapped; did you mean to use '!' or '?'?}}
15+
let _: Coat? = prop.iuo!.optional()!
16+
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
17+
let _: Coat? = prop.iuo!.optional!()
18+
let _: Coat? = prop.iuo!.optional!()!
19+
let _: Coat = prop.iuo.optional()
20+
// expected-error@-1 {{value of optional type '(() -> Coat)?' not unwrapped; did you mean to use '!' or '?'?}}
21+
let _: Coat = prop.iuo.optional()!
22+
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
23+
let _: Coat = prop.iuo.optional!()
24+
let _: Coat = prop.iuo.optional!()!
25+
let _: Coat = prop.iuo!.optional()
26+
// expected-error@-1 {{value of optional type '(() -> Coat)?' not unwrapped; did you mean to use '!' or '?'?}}
27+
let _: Coat = prop.iuo!.optional()!
28+
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
29+
let _: Coat = prop.iuo!.optional!()
30+
let _: Coat = prop.iuo!.optional!()!
31+
}

test/Inputs/clang-importer-sdk/usr/include/Foundation.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,7 @@ typedef struct NonNilableReferences {
868868
@protocol NSProtocolWithOptionalRequirement
869869
@optional
870870
-(void)optionalRequirement;
871+
-(DummyClass *)optionalRequirementMethodWithIUOResult;
871872
@end
872873

873874
@interface NSClassWithMethodFromNSProtocolWithOptionalRequirement
@@ -1144,3 +1145,15 @@ void install_global_event_handler(_Nullable event_handler handler);
11441145

11451146
__nullable id returnNullableId(void);
11461147
void takeNullableId(__nullable id);
1148+
1149+
@interface I
1150+
@end
1151+
1152+
@protocol OptionalMethods
1153+
@optional
1154+
- (Coat *)optional;
1155+
@end
1156+
1157+
@interface IUOProperty
1158+
@property (readonly) id<OptionalMethods> iuo;
1159+
@end

0 commit comments

Comments
 (0)