Skip to content

Commit 3249130

Browse files
committed
CS: Handle unbound references to @objc optional methods
1 parent d877ff9 commit 3249130

File tree

6 files changed

+234
-79
lines changed

6 files changed

+234
-79
lines changed

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3374,8 +3374,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
33743374
"method does not have a witness table entry");
33753375
}
33763376

3377-
// Get the expected type of a dynamic method reference.
3378-
SILType getDynamicMethodType(SILType selfType, SILDeclRef method) {
3377+
/// Verify the given type of a dynamic or @objc optional method reference.
3378+
bool verifyDynamicMethodType(CanSILFunctionType verifiedTy, SILType selfType,
3379+
SILDeclRef method) {
33793380
auto &C = F.getASTContext();
33803381

33813382
// The type of the dynamic method must match the usual type of the method,
@@ -3394,28 +3395,50 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
33943395
}
33953396
assert(!methodTy->isPolymorphic());
33963397

3397-
// Replace Self parameter with type of 'self' at the call site.
3398-
auto params = methodTy->getParameters();
3399-
SmallVector<SILParameterInfo, 4>
3400-
dynParams(params.begin(), params.end() - 1);
3401-
dynParams.push_back(SILParameterInfo(selfType.getASTType(),
3402-
params.back().getConvention()));
3398+
// Assume the parameter conventions are correct.
3399+
SmallVector<SILParameterInfo, 4> params;
3400+
{
3401+
const auto actualParams = methodTy->getParameters();
3402+
const auto verifiedParams = verifiedTy->getParameters();
3403+
if (actualParams.size() != verifiedParams.size())
3404+
return false;
3405+
3406+
for (const auto idx : indices(actualParams)) {
3407+
params.push_back(actualParams[idx].getWithConvention(
3408+
verifiedParams[idx].getConvention()));
3409+
}
3410+
}
3411+
3412+
// Have the 'self' parameter assume the type of 'self' at the call site.
3413+
params.back() = params.back().getWithInterfaceType(selfType.getASTType());
34033414

3404-
auto results = methodTy->getResults();
3405-
SmallVector<SILResultInfo, 4> dynResults(results.begin(), results.end());
3415+
// Assume the result conventions are correct.
3416+
SmallVector<SILResultInfo, 4> results;
3417+
{
3418+
const auto actualResults = methodTy->getResults();
3419+
const auto verifiedResults = verifiedTy->getResults();
3420+
if (actualResults.size() != verifiedResults.size())
3421+
return false;
3422+
3423+
for (const auto idx : indices(actualResults)) {
3424+
results.push_back(actualResults[idx].getWithConvention(
3425+
verifiedResults[idx].getConvention()));
3426+
}
3427+
}
34063428

3407-
// If the method returns Self, substitute AnyObject for the result type.
3429+
// If the method returns dynamic Self, substitute AnyObject for the
3430+
// result type.
34083431
if (auto fnDecl = dyn_cast<FuncDecl>(method.getDecl())) {
34093432
if (fnDecl->hasDynamicSelfResult()) {
34103433
auto anyObjectTy = C.getAnyObjectType();
3411-
for (auto &dynResult : dynResults) {
3434+
for (auto &result : results) {
34123435
auto newResultTy =
3413-
dynResult
3436+
result
34143437
.getReturnValueType(F.getModule(), methodTy,
34153438
F.getTypeExpansionContext())
34163439
->replaceCovariantResultType(anyObjectTy, 0);
3417-
dynResult = SILResultInfo(newResultTy->getCanonicalType(),
3418-
dynResult.getConvention());
3440+
result = SILResultInfo(newResultTy->getCanonicalType(),
3441+
result.getConvention());
34193442
}
34203443
}
34213444
}
@@ -3424,13 +3447,14 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
34243447
methodTy->getExtInfo(),
34253448
methodTy->getCoroutineKind(),
34263449
methodTy->getCalleeConvention(),
3427-
dynParams,
3450+
params,
34283451
methodTy->getYields(),
3429-
dynResults,
3452+
results,
34303453
methodTy->getOptionalErrorResult(),
34313454
SubstitutionMap(), SubstitutionMap(),
34323455
F.getASTContext());
3433-
return SILType::getPrimitiveObjectType(fnTy);
3456+
3457+
return fnTy->isBindableTo(verifiedTy);
34343458
}
34353459

34363460
/// Visitor class that checks whether a given decl ref has an entry in the
@@ -4881,9 +4905,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
48814905
"true bb for dynamic_method_br must take an argument");
48824906

48834907
auto bbArgTy = DMBI->getHasMethodBB()->args_begin()[0]->getType();
4884-
require(getDynamicMethodType(operandType, DMBI->getMember())
4885-
.getASTType()
4886-
->isBindableTo(bbArgTy.getASTType()),
4908+
require(verifyDynamicMethodType(cast<SILFunctionType>(bbArgTy.getASTType()),
4909+
operandType, DMBI->getMember()),
48874910
"bb argument for dynamic_method_br must be of the method's type");
48884911
}
48894912

lib/Sema/CSApply.cpp

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -902,11 +902,10 @@ namespace {
902902
bool shouldBuildCurryThunk(OverloadChoice choice,
903903
bool baseIsInstance) {
904904
ValueDecl *member = choice.getDecl();
905-
auto isDynamic = choice.getKind() == OverloadChoiceKind::DeclViaDynamic;
906905

907906
// FIXME: We should finish plumbing this through for dynamic
908907
// lookup as well.
909-
if (isDynamic || member->getAttrs().hasAttribute<OptionalAttr>())
908+
if (choice.getKind() == OverloadChoiceKind::DeclViaDynamic)
910909
return false;
911910

912911
// If we're inside a selector expression, don't build the thunk.
@@ -922,6 +921,11 @@ namespace {
922921
if (!baseIsInstance && member->isInstanceMember())
923922
return true;
924923

924+
// Bound optional method references are represented via
925+
// DynamicMemberRefExpr instead of a curry thunk.
926+
if (member->getAttrs().hasAttribute<OptionalAttr>())
927+
return false;
928+
925929
// Figure out how many argument lists we need.
926930
unsigned maxArgCount = member->getNumCurryLevels();
927931

@@ -1224,7 +1228,8 @@ namespace {
12241228
AutoClosureExpr *
12251229
buildDoubleCurryThunk(DeclRefExpr *memberRef, ValueDecl *member,
12261230
FunctionType *outerThunkTy,
1227-
ConstraintLocatorBuilder memberLocator) {
1231+
ConstraintLocatorBuilder memberLocator,
1232+
DeclNameLoc memberLoc, bool isDynamicLookup) {
12281233
auto &ctx = cs.getASTContext();
12291234

12301235
const auto selfThunkParam = outerThunkTy->getParams().front();
@@ -1256,15 +1261,15 @@ namespace {
12561261
cs.cacheType(selfParamRef);
12571262
}
12581263

1259-
const auto selfCalleeParam =
1260-
cs.getType(memberRef)->castTo<FunctionType>()->getParams().front();
1264+
auto *const selfCalleeTy = cs.getType(memberRef)->castTo<FunctionType>();
1265+
const auto selfCalleeParam = selfCalleeTy->getParams().front();
12611266
const auto selfCalleeParamTy = selfCalleeParam.getPlainType();
12621267

12631268
// Open the 'self' parameter reference if warranted.
12641269
Expr *selfOpenedRef = selfParamRef;
12651270
if (selfCalleeParamTy->hasOpenedExistential()) {
12661271
// If we're opening an existential:
1267-
// - The type of 'ref' inside the thunk is written in terms of the
1272+
// - The type of 'memberRef' inside the thunk is written in terms of the
12681273
// open existental archetype.
12691274
// - The type of the thunk is written in terms of the
12701275
// erased existential bounds.
@@ -1281,25 +1286,56 @@ namespace {
12811286
adjustExprOwnershipForParam(selfParamRef, selfThunkParam);
12821287
}
12831288

1284-
// The inner thunk, "{ args... in self.member(args...) }".
1285-
auto *const innerThunk = buildSingleCurryThunk(
1286-
selfOpenedRef, memberRef, cast<DeclContext>(member),
1287-
outerThunkTy->getResult()->castTo<FunctionType>(), memberLocator);
1288-
1289-
// Rewrite the body to close the existential if warranted.
1290-
if (selfCalleeParamTy->hasOpenedExistential()) {
1291-
auto *body = innerThunk->getSingleExpressionBody();
1292-
body = new (ctx) OpenExistentialExpr(
1293-
selfParamRef, cast<OpaqueValueExpr>(selfOpenedRef), body,
1294-
body->getType());
1295-
cs.cacheType(body);
1289+
Expr *outerThunkBody = nullptr;
1290+
1291+
// For an @objc optional member or a member found via dynamic lookup,
1292+
// build a dynamic member reference. Otherwise, build a nested
1293+
// "{ args... in self.member(args...) }" thunk that calls the member.
1294+
if (isDynamicLookup || member->getAttrs().hasAttribute<OptionalAttr>()) {
1295+
outerThunkBody = new (ctx) DynamicMemberRefExpr(
1296+
selfOpenedRef, SourceLoc(),
1297+
resolveConcreteDeclRef(member, memberLocator), memberLoc);
1298+
outerThunkBody->setImplicit(true);
1299+
outerThunkBody->setType(selfCalleeTy->getResult());
1300+
cs.cacheType(outerThunkBody);
1301+
1302+
outerThunkBody = coerceToType(outerThunkBody, outerThunkTy->getResult(),
1303+
memberLocator);
1304+
1305+
// Close the existential if warranted.
1306+
if (selfCalleeParamTy->hasOpenedExistential()) {
1307+
// If the callee's 'self' parameter has non-trivial ownership, adjust
1308+
// the argument type accordingly.
1309+
adjustExprOwnershipForParam(selfOpenedRef, selfCalleeParam);
1310+
1311+
outerThunkBody = new (ctx) OpenExistentialExpr(
1312+
selfParamRef, cast<OpaqueValueExpr>(selfOpenedRef),
1313+
outerThunkBody, outerThunkBody->getType());
1314+
cs.cacheType(outerThunkBody);
1315+
}
1316+
} else {
1317+
auto *innerThunk = buildSingleCurryThunk(
1318+
selfOpenedRef, memberRef, cast<DeclContext>(member),
1319+
outerThunkTy->getResult()->castTo<FunctionType>(), memberLocator);
1320+
1321+
// Rewrite the body to close the existential if warranted.
1322+
if (selfCalleeParamTy->hasOpenedExistential()) {
1323+
auto *body = innerThunk->getSingleExpressionBody();
1324+
body = new (ctx) OpenExistentialExpr(
1325+
selfParamRef, cast<OpaqueValueExpr>(selfOpenedRef), body,
1326+
body->getType());
1327+
cs.cacheType(body);
1328+
1329+
innerThunk->setBody(body);
1330+
}
12961331

1297-
innerThunk->setBody(body);
1332+
outerThunkBody = innerThunk;
12981333
}
12991334

13001335
// Finally, construct the outer thunk.
1301-
auto outerThunk = new (ctx) AutoClosureExpr(
1302-
innerThunk, outerThunkTy, AutoClosureExpr::InvalidDiscriminator, dc);
1336+
auto *outerThunk =
1337+
new (ctx) AutoClosureExpr(outerThunkBody, outerThunkTy,
1338+
AutoClosureExpr::InvalidDiscriminator, dc);
13031339
outerThunk->setThunkKind(AutoClosureExpr::Kind::DoubleCurryThunk);
13041340
outerThunk->setParameterList(
13051341
ParameterList::create(ctx, SourceLoc(), selfParamDecl, SourceLoc()));
@@ -1488,8 +1524,28 @@ namespace {
14881524
}
14891525
assert(base && "Unable to convert base?");
14901526

1491-
// Handle dynamic references.
14921527
if (isDynamic || member->getAttrs().hasAttribute<OptionalAttr>()) {
1528+
// If the @objc attribute was inferred based on deprecated Swift 3
1529+
// rules, complain at this use site.
1530+
if (auto attr = member->getAttrs().getAttribute<ObjCAttr>()) {
1531+
if (attr->isSwift3Inferred() &&
1532+
context.LangOpts.WarnSwift3ObjCInference ==
1533+
Swift3ObjCInferenceWarnings::Minimal) {
1534+
context.Diags.diagnose(
1535+
memberLoc, diag::expr_dynamic_lookup_swift3_objc_inference,
1536+
member->getDescriptiveKind(), member->getName(),
1537+
member->getDeclContext()->getSelfNominalTypeDecl()->getName());
1538+
context.Diags
1539+
.diagnose(member, diag::make_decl_objc,
1540+
member->getDescriptiveKind())
1541+
.fixItInsert(member->getAttributeInsertionLoc(false), "@objc ");
1542+
}
1543+
}
1544+
}
1545+
1546+
// Handle dynamic references.
1547+
if (isDynamic || (!isPartialApplication &&
1548+
member->getAttrs().hasAttribute<OptionalAttr>())) {
14931549
base = cs.coerceToRValue(base);
14941550
Expr *ref = new (context) DynamicMemberRefExpr(base, dotLoc, memberRef,
14951551
memberLoc);
@@ -1510,23 +1566,6 @@ namespace {
15101566

15111567
closeExistentials(ref, locator, /*force=*/openedExistential);
15121568

1513-
// If this attribute was inferred based on deprecated Swift 3 rules,
1514-
// complain.
1515-
if (auto attr = member->getAttrs().getAttribute<ObjCAttr>()) {
1516-
if (attr->isSwift3Inferred() &&
1517-
context.LangOpts.WarnSwift3ObjCInference ==
1518-
Swift3ObjCInferenceWarnings::Minimal) {
1519-
context.Diags.diagnose(
1520-
memberLoc, diag::expr_dynamic_lookup_swift3_objc_inference,
1521-
member->getDescriptiveKind(), member->getName(),
1522-
member->getDeclContext()->getSelfNominalTypeDecl()->getName());
1523-
context.Diags
1524-
.diagnose(member, diag::make_decl_objc,
1525-
member->getDescriptiveKind())
1526-
.fixItInsert(member->getAttributeInsertionLoc(false), "@objc ");
1527-
}
1528-
}
1529-
15301569
// We also need to handle the implicitly unwrap of the result
15311570
// of the called function if that's the type checking solution
15321571
// we ended up with.
@@ -1649,7 +1688,7 @@ namespace {
16491688
// Replace the DeclRefExpr with a closure expression which SILGen
16501689
// knows how to emit.
16511690
ref = buildDoubleCurryThunk(declRefExpr, member, curryThunkTy,
1652-
memberLocator);
1691+
memberLocator, memberLoc, isDynamic);
16531692
}
16541693

16551694
// If the member is a method with a dynamic 'Self' result type, wrap an

lib/Sema/ConstraintSystem.cpp

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,13 +2054,6 @@ ConstraintSystem::getTypeOfMemberReference(
20542054

20552055
AnyFunctionType *funcType;
20562056

2057-
// Check if we need to apply a layer of optionality to the resulting type.
2058-
auto isReferenceOptional = false;
2059-
if (!isRequirementOrWitness(locator)) {
2060-
if (isDynamicResult || value->getAttrs().hasAttribute<OptionalAttr>())
2061-
isReferenceOptional = true;
2062-
}
2063-
20642057
if (isa<AbstractFunctionDecl>(value) ||
20652058
isa<EnumElementDecl>(value)) {
20662059
if (auto ErrorTy = value->getInterfaceType()->getAs<ErrorType>()) {
@@ -2089,12 +2082,6 @@ ConstraintSystem::getTypeOfMemberReference(
20892082
if (doesStorageProduceLValue(subscript, baseTy, useDC, locator))
20902083
elementTy = LValueType::get(elementTy);
20912084

2092-
// Optional and dynamic subscripts are a special case, because the
2093-
// optionality is applied to the result type and not the type of the
2094-
// reference.
2095-
if (isReferenceOptional)
2096-
elementTy = OptionalType::get(elementTy->getRValueType());
2097-
20982085
auto indices = subscript->getInterfaceType()
20992086
->castTo<AnyFunctionType>()->getParams();
21002087
// FIXME: Verify ExtInfo state is correct, not working by accident.
@@ -2239,6 +2226,38 @@ ConstraintSystem::getTypeOfMemberReference(
22392226
}
22402227
}
22412228

2229+
// Check if we need to apply a layer of optionality to the uncurried type.
2230+
if (!isRequirementOrWitness(locator)) {
2231+
if (isDynamicResult || value->getAttrs().hasAttribute<OptionalAttr>()) {
2232+
const auto applyOptionality = [&](FunctionType *fnTy) -> Type {
2233+
Type resultTy;
2234+
// Optional and dynamic subscripts are a special case, because the
2235+
// optionality is applied to the result type and not the type of the
2236+
// reference.
2237+
if (isa<SubscriptDecl>(value)) {
2238+
auto *innerFn = fnTy->getResult()->castTo<FunctionType>();
2239+
resultTy = FunctionType::get(
2240+
innerFn->getParams(),
2241+
OptionalType::get(innerFn->getResult()->getRValueType()),
2242+
innerFn->getExtInfo());
2243+
} else {
2244+
resultTy = OptionalType::get(fnTy->getResult()->getRValueType());
2245+
}
2246+
2247+
return FunctionType::get(fnTy->getParams(), resultTy,
2248+
fnTy->getExtInfo());
2249+
};
2250+
2251+
// FIXME: Refactor 'replaceCovariantResultType' not to rely on the passed
2252+
// uncurry level.
2253+
//
2254+
// This is done after handling dynamic 'Self' to make
2255+
// 'replaceCovariantResultType' work, so we have to transform both types.
2256+
openedType = applyOptionality(openedType->castTo<FunctionType>());
2257+
type = applyOptionality(type->castTo<FunctionType>());
2258+
}
2259+
}
2260+
22422261
if (hasAppliedSelf) {
22432262
// For a static member referenced through a metatype or an instance
22442263
// member referenced through an instance, strip off the 'self'.
@@ -2293,10 +2312,6 @@ ConstraintSystem::getTypeOfMemberReference(
22932312
}
22942313
}
22952314

2296-
// If we need to wrap the type in an optional, do so now.
2297-
if (isReferenceOptional && !isa<SubscriptDecl>(value))
2298-
type = OptionalType::get(type->getRValueType());
2299-
23002315
if (isForCodeCompletion() && type->hasError()) {
23012316
// In code completion, replace error types by placeholder types so we can
23022317
// match the types we know instead of bailing out completely.
@@ -3002,6 +3017,14 @@ void ConstraintSystem::bindOverloadType(
30023017
return;
30033018
}
30043019

3020+
// The opened type of an unbound member reference has optionality applied
3021+
// to the uncurried type.
3022+
if (!doesMemberRefApplyCurriedSelf(choice.getBaseType(),
3023+
choice.getDecl())) {
3024+
bindTypeOrIUO(openedType);
3025+
return;
3026+
}
3027+
30053028
// Build an outer disjunction to attempt binding both T? and T, then bind
30063029
// as normal. This is needed to correctly handle e.g IUO properties which
30073030
// may need two levels of optionality unwrapped T??.

0 commit comments

Comments
 (0)