Skip to content

Commit 5e2b20d

Browse files
jckarterjckarter
committed
SILGen: Fix crashes when conditionally looking up generic subscripts and properties via AnyObject.
The fix for methods to lower the dynamic method type from the substituted AST type of the expression also needed to be applied to the optional chaining, subscript, and property paths. This also exposed a problem in the Clang importer, where imported subscript accessors would get the unbound generic context type as their Self parameter type instead of the type with the correct generic parameters. Fix this by renaming the all-too-convenient ParamDecl::createSelf factory to `createUnboundSelf`, and introduce a new `createSelf` that uses the bound generic type. Fixes rdar://problem/26447758.
1 parent 5c83792 commit 5e2b20d

File tree

16 files changed

+185
-55
lines changed

16 files changed

+185
-55
lines changed

‎include/swift/AST/Decl.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4392,9 +4392,22 @@ class ParamDecl : public VarDecl {
43924392
/// Note that this decl is created, but it is returned with an incorrect
43934393
/// DeclContext that needs to be set correctly. This is automatically handled
43944394
/// when a function is created with this as part of its argument list.
4395+
/// For a generic context, this also gives the parameter an unbound generic
4396+
/// type with the expectation that type-checking will fill in the context
4397+
/// generic parameters.
4398+
static ParamDecl *createUnboundSelf(SourceLoc loc, DeclContext *DC,
4399+
bool isStatic = false,
4400+
bool isInOut = false);
4401+
4402+
/// Create an implicit 'self' decl for a method in the specified decl context.
4403+
/// If 'static' is true, then this is self for a static method in the type.
43954404
///
4405+
/// Note that this decl is created, but it is returned with an incorrect
4406+
/// DeclContext that needs to be set correctly. This is automatically handled
4407+
/// when a function is created with this as part of its argument list.
43964408
static ParamDecl *createSelf(SourceLoc loc, DeclContext *DC,
4397-
bool isStatic = false, bool isInOut = false);
4409+
bool isStatic = false,
4410+
bool isInOut = false);
43984411

43994412
// Implement isa/cast/dyncast/etc.
44004413
static bool classof(const Decl *D) {

‎include/swift/AST/ParameterList.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,18 @@ class alignas(ParamDecl *) ParameterList final :
7373
/// DeclContext that needs to be set correctly. This is automatically handled
7474
/// when a function is created with this as part of its argument list.
7575
///
76+
static ParameterList *createUnboundSelf(SourceLoc loc, DeclContext *DC,
77+
bool isStaticMethod = false,
78+
bool isInOut = false);
79+
80+
/// Create an implicit 'self' decl for a method in the specified decl context.
81+
/// If 'static' is true, then this is self for a static method in the type.
82+
///
83+
/// Note that this decl is created, but it is returned with an incorrect
84+
/// DeclContext that needs to be set correctly. This is automatically handled
85+
/// when a function is created with this as part of its argument list.
7686
static ParameterList *createSelf(SourceLoc loc, DeclContext *DC,
77-
bool isStaticMethod = false,
87+
bool isStatic = false,
7888
bool isInOut = false);
7989

8090
SourceLoc getLParenLoc() const { return LParenLoc; }

‎include/swift/AST/Types.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2363,6 +2363,10 @@ BEGIN_CAN_TYPE_WRAPPER(AnyFunctionType, Type)
23632363
typedef AnyFunctionType::ExtInfo ExtInfo;
23642364
PROXY_CAN_TYPE_SIMPLE_GETTER(getInput)
23652365
PROXY_CAN_TYPE_SIMPLE_GETTER(getResult)
2366+
2367+
CanAnyFunctionType withExtInfo(ExtInfo info) const {
2368+
return CanAnyFunctionType(getPointer()->withExtInfo(info));
2369+
}
23662370
END_CAN_TYPE_WRAPPER(AnyFunctionType, Type)
23672371

23682372
/// FunctionType - A monomorphic function type, specified with an arrow.
@@ -2396,6 +2400,10 @@ BEGIN_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType)
23962400
const ExtInfo &info) {
23972401
return CanFunctionType(FunctionType::get(input, result, info));
23982402
}
2403+
2404+
CanFunctionType withExtInfo(ExtInfo info) const {
2405+
return CanFunctionType(cast<FunctionType>(getPointer()->withExtInfo(info)));
2406+
}
23992407
END_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType)
24002408

24012409
/// PolymorphicFunctionType - A polymorphic function type.
@@ -2517,6 +2525,11 @@ BEGIN_CAN_TYPE_WRAPPER(GenericFunctionType, AnyFunctionType)
25172525
ArrayRef<CanTypeWrapper<GenericTypeParamType>> getGenericParams() const {
25182526
return getGenericSignature().getGenericParams();
25192527
}
2528+
2529+
CanGenericFunctionType withExtInfo(ExtInfo info) const {
2530+
return CanGenericFunctionType(
2531+
cast<GenericFunctionType>(getPointer()->withExtInfo(info)));
2532+
}
25202533
END_CAN_TYPE_WRAPPER(GenericFunctionType, AnyFunctionType)
25212534

25222535
/// Conventions for passing arguments as parameters.

‎lib/AST/Decl.cpp

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3594,11 +3594,43 @@ static Type getSelfTypeOfContext(DeclContext *dc) {
35943594
/// Note that this decl is created, but it is returned with an incorrect
35953595
/// DeclContext that needs to be set correctly. This is automatically handled
35963596
/// when a function is created with this as part of its argument list.
3597+
/// For a generic context, this also gives the parameter an unbound generic
3598+
/// type with the expectation that type-checking will fill in the context
3599+
/// generic parameters.
3600+
ParamDecl *ParamDecl::createUnboundSelf(SourceLoc loc, DeclContext *DC,
3601+
bool isStaticMethod, bool isInOut) {
3602+
ASTContext &C = DC->getASTContext();
3603+
auto selfType = getSelfTypeOfContext(DC);
3604+
3605+
// If we have a selfType (i.e. we're not in the parser before we know such
3606+
// things, configure it.
3607+
if (selfType) {
3608+
if (isStaticMethod)
3609+
selfType = MetatypeType::get(selfType);
3610+
3611+
if (isInOut)
3612+
selfType = InOutType::get(selfType);
3613+
}
3614+
3615+
auto *selfDecl = new (C) ParamDecl(/*IsLet*/!isInOut, SourceLoc(),SourceLoc(),
3616+
Identifier(), loc, C.Id_self, selfType,DC);
3617+
selfDecl->setImplicit();
3618+
return selfDecl;
3619+
}
3620+
3621+
/// Create an implicit 'self' decl for a method in the specified decl context.
3622+
/// If 'static' is true, then this is self for a static method in the type.
35973623
///
3624+
/// Note that this decl is created, but it is returned with an incorrect
3625+
/// DeclContext that needs to be set correctly. This is automatically handled
3626+
/// when a function is created with this as part of its argument list.
3627+
/// For a generic context, this also gives the parameter an unbound generic
3628+
/// type with the expectation that type-checking will fill in the context
3629+
/// generic parameters.
35983630
ParamDecl *ParamDecl::createSelf(SourceLoc loc, DeclContext *DC,
35993631
bool isStaticMethod, bool isInOut) {
36003632
ASTContext &C = DC->getASTContext();
3601-
auto selfType = getSelfTypeOfContext(DC);
3633+
auto selfType = DC->getSelfTypeInContext();
36023634

36033635
// If we have a selfType (i.e. we're not in the parser before we know such
36043636
// things, configure it.
@@ -3613,6 +3645,7 @@ ParamDecl *ParamDecl::createSelf(SourceLoc loc, DeclContext *DC,
36133645
auto *selfDecl = new (C) ParamDecl(/*IsLet*/!isInOut, SourceLoc(),SourceLoc(),
36143646
Identifier(), loc, C.Id_self, selfType,DC);
36153647
selfDecl->setImplicit();
3648+
selfDecl->setInterfaceType(DC->getSelfInterfaceType());
36163649
return selfDecl;
36173650
}
36183651

‎lib/AST/Parameter.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,27 @@ ParameterList::create(const ASTContext &C, SourceLoc LParenLoc,
4747
/// Note that this decl is created, but it is returned with an incorrect
4848
/// DeclContext that needs to be set correctly. This is automatically handled
4949
/// when a function is created with this as part of its argument list.
50+
/// For a generic context, this also gives the parameter an unbound generic
51+
/// type with the expectation that type-checking will fill in the context
52+
/// generic parameters.
53+
ParameterList *ParameterList::createUnboundSelf(SourceLoc loc,
54+
DeclContext *DC,
55+
bool isStaticMethod,
56+
bool isInOut) {
57+
auto *PD = ParamDecl::createUnboundSelf(loc, DC, isStaticMethod, isInOut);
58+
return create(DC->getASTContext(), PD);
59+
}
60+
61+
/// Create an implicit 'self' decl for a method in the specified decl context.
62+
/// If 'static' is true, then this is self for a static method in the type.
5063
///
51-
ParameterList *ParameterList::createSelf(SourceLoc loc, DeclContext *DC,
52-
bool isStaticMethod, bool isInOut) {
64+
/// Note that this decl is created, but it is returned with an incorrect
65+
/// DeclContext that needs to be set correctly. This is automatically handled
66+
/// when a function is created with this as part of its argument list.
67+
ParameterList *ParameterList::createSelf(SourceLoc loc,
68+
DeclContext *DC,
69+
bool isStaticMethod,
70+
bool isInOut) {
5371
auto *PD = ParamDecl::createSelf(loc, DC, isStaticMethod, isInOut);
5472
return create(DC->getASTContext(), PD);
5573
}
@@ -61,8 +79,6 @@ void ParameterList::setDeclContextOfParamDecls(DeclContext *DC) {
6179
P->setDeclContext(DC);
6280
}
6381

64-
65-
6682
/// Make a duplicate copy of this parameter list. This allocates copies of
6783
/// the ParamDecls, so they can be reparented into a new DeclContext.
6884
ParameterList *ParameterList::clone(const ASTContext &C,

‎lib/AST/Type.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,6 +1690,9 @@ bool TypeBase::isBindableTo(Type b, LazyResolver *resolver) {
16901690
}
16911691

16921692
bool visitType(TypeBase *orig, CanType subst) {
1693+
if (CanType(orig) == subst)
1694+
return true;
1695+
16931696
llvm_unreachable("not a valid canonical type substitution");
16941697
}
16951698

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ makeEnumRawValueConstructor(ClangImporter::Implementation &Impl,
347347
auto enumTy = enumDecl->getDeclaredTypeInContext();
348348
auto metaTy = MetatypeType::get(enumTy);
349349

350-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), enumDecl,
350+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), enumDecl,
351351
/*static*/false, /*inout*/true);
352352

353353
auto param = new (C) ParamDecl(/*let*/ true, SourceLoc(),
@@ -413,7 +413,7 @@ static FuncDecl *makeEnumRawValueGetter(ClangImporter::Implementation &Impl,
413413
VarDecl *rawValueDecl) {
414414
ASTContext &C = Impl.SwiftContext;
415415

416-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), enumDecl);
416+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), enumDecl);
417417

418418
ParameterList *params[] = {
419419
ParameterList::createWithoutLoc(selfDecl),
@@ -470,7 +470,7 @@ static FuncDecl *makeNewtypeBridgedRawValueGetter(
470470
VarDecl *storedVar) {
471471
ASTContext &C = Impl.SwiftContext;
472472

473-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), structDecl);
473+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), structDecl);
474474

475475
ParameterList *params[] = {
476476
ParameterList::createWithoutLoc(selfDecl),
@@ -517,7 +517,7 @@ static FuncDecl *makeFieldGetterDecl(ClangImporter::Implementation &Impl,
517517
VarDecl *importedFieldDecl,
518518
ClangNode clangNode = ClangNode()) {
519519
auto &C = Impl.SwiftContext;
520-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), importedDecl);
520+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), importedDecl);
521521

522522
ParameterList *params[] = {
523523
ParameterList::createWithoutLoc(selfDecl),
@@ -545,7 +545,7 @@ static FuncDecl *makeFieldSetterDecl(ClangImporter::Implementation &Impl,
545545
VarDecl *importedFieldDecl,
546546
ClangNode clangNode = ClangNode()) {
547547
auto &C = Impl.SwiftContext;
548-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), importedDecl,
548+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), importedDecl,
549549
/*isStatic*/false, /*isInOut*/true);
550550
auto newValueDecl = new (C) ParamDecl(/*isLet */ true,SourceLoc(),SourceLoc(),
551551
Identifier(), SourceLoc(), C.Id_value,
@@ -946,7 +946,7 @@ static bool addErrorDomain(NominalTypeDecl *swiftDecl,
946946
DeclRefExpr(ConcreteDeclRef(swiftValueDecl), {}, isImplicit);
947947
ParameterList *params[] = {
948948
ParameterList::createWithoutLoc(
949-
ParamDecl::createSelf(SourceLoc(), swiftDecl, isStatic)),
949+
ParamDecl::createUnboundSelf(SourceLoc(), swiftDecl, isStatic)),
950950
ParameterList::createEmpty(C)};
951951
auto toStringTy = ParameterList::getFullType(stringTy, params);
952952

@@ -1651,7 +1651,7 @@ namespace {
16511651
auto &context = Impl.SwiftContext;
16521652

16531653
// Create the 'self' declaration.
1654-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), structDecl,
1654+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), structDecl,
16551655
/*static*/false, /*inout*/true);
16561656

16571657
// self & param.
@@ -1892,7 +1892,7 @@ namespace {
18921892
auto &context = Impl.SwiftContext;
18931893

18941894
// Create the 'self' declaration.
1895-
auto selfDecl = ParamDecl::createSelf(SourceLoc(), structDecl,
1895+
auto selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), structDecl,
18961896
/*static*/false, /*inout*/true);
18971897

18981898
// Construct the set of parameters from the list of members.
@@ -2839,7 +2839,7 @@ namespace {
28392839

28402840
bool selfIsInOut =
28412841
!dc->getDeclaredTypeOfContext()->hasReferenceSemantics();
2842-
auto selfParam = ParamDecl::createSelf(SourceLoc(), dc, /*static=*/false,
2842+
auto selfParam = ParamDecl::createUnboundSelf(SourceLoc(), dc, /*static=*/false,
28432843
/*inout=*/selfIsInOut);
28442844

28452845
OptionalTypeKind initOptionality;
@@ -2906,7 +2906,7 @@ namespace {
29062906
}
29072907

29082908
bodyParams.push_back(ParameterList::createWithoutLoc(
2909-
ParamDecl::createSelf(SourceLoc(), dc, !selfIdx.hasValue(),
2909+
ParamDecl::createUnboundSelf(SourceLoc(), dc, !selfIdx.hasValue(),
29102910
selfIsInOut)));
29112911
bodyParams.push_back(getNonSelfParamList(
29122912
decl, selfIdx, name.getArgumentNames(), allowNSUIntegerAsInt, !name));
@@ -3620,7 +3620,7 @@ namespace {
36203620
// Add the implicit 'self' parameter patterns.
36213621
SmallVector<ParameterList *, 4> bodyParams;
36223622
auto selfVar =
3623-
ParamDecl::createSelf(SourceLoc(), dc, /*isStatic*/!isInstance);
3623+
ParamDecl::createUnboundSelf(SourceLoc(), dc, /*isStatic*/!isInstance);
36243624
bodyParams.push_back(ParameterList::createWithoutLoc(selfVar));
36253625
Type selfInterfaceType;
36263626
if (dc->getAsProtocolOrProtocolExtensionContext()) {
@@ -4056,7 +4056,7 @@ namespace {
40564056

40574057
// Add the implicit 'self' parameter patterns.
40584058
SmallVector<ParameterList*, 4> bodyParams;
4059-
auto selfMetaVar = ParamDecl::createSelf(SourceLoc(), dc, /*static*/true);
4059+
auto selfMetaVar = ParamDecl::createUnboundSelf(SourceLoc(), dc, /*static*/true);
40604060
auto selfTy = dc->getDeclaredTypeInContext();
40614061
auto selfMetaTy = MetatypeType::get(selfTy);
40624062
bodyParams.push_back(ParameterList::createWithoutLoc(selfMetaVar));
@@ -4174,7 +4174,7 @@ namespace {
41744174
if (known != Impl.Constructors.end())
41754175
return known->second;
41764176

4177-
auto *selfVar = ParamDecl::createSelf(SourceLoc(), dc);
4177+
auto *selfVar = ParamDecl::createUnboundSelf(SourceLoc(), dc);
41784178

41794179
// Create the actual constructor.
41804180
auto result = Impl.createDeclWithClangNode<ConstructorDecl>(objcMethod,
@@ -6867,7 +6867,7 @@ ClangImporter::Implementation::createConstant(Identifier name, DeclContext *dc,
68676867

68686868
// 'self'
68696869
if (dc->isTypeContext()) {
6870-
auto *selfDecl = ParamDecl::createSelf(SourceLoc(), dc, isStatic);
6870+
auto *selfDecl = ParamDecl::createUnboundSelf(SourceLoc(), dc, isStatic);
68716871
getterArgs.push_back(ParameterList::createWithoutLoc(selfDecl));
68726872
}
68736873

‎lib/Parse/ParseDecl.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3120,7 +3120,7 @@ static FuncDecl *createAccessorFunc(SourceLoc DeclLoc, ParameterList *param,
31203120

31213121
// Add the implicit 'self' to Params, if needed.
31223122
if (Flags & Parser::PD_HasContainerType)
3123-
Params.push_back(ParameterList::createSelf(DeclLoc, P->CurDeclContext));
3123+
Params.push_back(ParameterList::createUnboundSelf(DeclLoc, P->CurDeclContext));
31243124

31253125
// Add the "(value)" and subscript indices parameter clause.
31263126
Params.push_back(ValueArg);
@@ -4540,7 +4540,7 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
45404540
// "(int)->int" on FooTy into "(self: FooTy.Type)->(int)->int".
45414541
// Note that we can't actually compute the type here until Sema.
45424542
if (HasContainerType)
4543-
BodyParams.push_back(ParameterList::createSelf(NameLoc, CurDeclContext));
4543+
BodyParams.push_back(ParameterList::createUnboundSelf(NameLoc, CurDeclContext));
45444544

45454545
DefaultArgumentInfo DefaultArgs(HasContainerType);
45464546
TypeRepr *FuncRetTy = nullptr;
@@ -5377,7 +5377,7 @@ Parser::parseDeclInit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
53775377
Attributes.add(new (Context) RethrowsAttr(throwsLoc));
53785378
}
53795379

5380-
auto *SelfDecl = ParamDecl::createSelf(ConstructorLoc, CurDeclContext);
5380+
auto *SelfDecl = ParamDecl::createUnboundSelf(ConstructorLoc, CurDeclContext);
53815381

53825382
Scope S2(this, ScopeKind::ConstructorBody);
53835383
auto *CD = new (Context) ConstructorDecl(FullName, ConstructorLoc,
@@ -5473,7 +5473,7 @@ parseDeclDeinit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
54735473
}
54745474
}
54755475

5476-
auto *SelfDecl = ParamDecl::createSelf(DestructorLoc, CurDeclContext);
5476+
auto *SelfDecl = ParamDecl::createUnboundSelf(DestructorLoc, CurDeclContext);
54775477

54785478
Scope S(this, ScopeKind::DestructorBody);
54795479
auto *DD = new (Context) DestructorDecl(Context.Id_deinit, DestructorLoc,

‎lib/SIL/SILVerifier.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2768,9 +2768,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
27682768
require(DMBI->getHasMethodBB()->bbarg_size() == 1,
27692769
"true bb for dynamic_method_br must take an argument");
27702770

2771-
requireSameType(DMBI->getHasMethodBB()->bbarg_begin()[0]->getType(),
2772-
getDynamicMethodType(operandType, DMBI->getMember()),
2773-
"bb argument for dynamic_method_br must be of the method's type");
2771+
auto bbArgTy = DMBI->getHasMethodBB()->bbarg_begin()[0]->getType();
2772+
require(getDynamicMethodType(operandType, DMBI->getMember())
2773+
.getSwiftRValueType()
2774+
->isBindableTo(bbArgTy.getSwiftRValueType(), nullptr),
2775+
"bb argument for dynamic_method_br must be of the method's type");
27742776
}
27752777

27762778
void checkProjectBlockStorageInst(ProjectBlockStorageInst *PBSI) {

0 commit comments

Comments
 (0)