Skip to content

Commit e373a55

Browse files
committed
Sema: Fix type member lookup if generic signature validation fails
When validateGenericFuncSignature() returns true, finalizeGenericParamList() is never called. Name lookup, via PartialGenericTypeToArchetypeResolver would return a dependent member type of the generic type parameter type, not an archetype as expected in this case. This would later on lead to a crash in ReplaceDependentTypes if the function body contained a reference to such a member type. Fix this by marking the GenericTypeParamDecls as invalid in this case, and returning an ErrorType from PartialGenericTypeToArchetypeResolver if given an invalid GenericTypeParamDecl. While we're at it, there was an unused isInvalid local variable in TypeCheckDecl::visitFuncDecl(). It was written to but never read. Replace the writes with calls to setInvalid(). Fixes <rdar://problem/19620340>. Swift SVN r30632
1 parent 6256158 commit e373a55

11 files changed

+66
-25
lines changed

lib/Sema/TypeCheckDecl.cpp

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,32 @@ void TypeChecker::revertGenericParamList(GenericParamList *genericParams) {
10031003
}
10041004
}
10051005

1006+
static void markInvalidGenericSignature(AbstractFunctionDecl *AFD,
1007+
TypeChecker &TC) {
1008+
ArchetypeBuilder builder = TC.createArchetypeBuilder(AFD->getParentModule());
1009+
auto genericParams = AFD->getGenericParams();
1010+
1011+
// If there is a parent context, add the generic parameters and requirements
1012+
// from that context.
1013+
addContextParamsAndRequirements(builder, AFD->getDeclContext());
1014+
1015+
// If there aren't any generic parameters at this level, we're done.
1016+
if (!genericParams)
1017+
return;
1018+
1019+
// Visit each of the generic parameters.
1020+
for (auto param : *genericParams)
1021+
builder.addGenericParameter(param);
1022+
1023+
// Wire up the archetypes.
1024+
for (auto GP : *genericParams)
1025+
GP->setArchetype(builder.getArchetype(GP));
1026+
1027+
genericParams->setAllArchetypes(
1028+
TC.Context.AllocateCopy(builder.getAllArchetypes()));
1029+
}
1030+
1031+
10061032
/// Finalize the given generic parameter list, assigning archetypes to
10071033
/// the generic parameters.
10081034
static void finalizeGenericParamList(ArchetypeBuilder &builder,
@@ -4104,11 +4130,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
41044130
else if (FD->getAttrs().hasAttribute<NonMutatingAttr>())
41054131
FD->setMutating(false);
41064132

4107-
bool isInvalid = false;
4108-
41094133
// Check whether the return type is dynamic 'Self'.
41104134
if (checkDynamicSelfReturn(FD))
4111-
isInvalid = true;
4135+
FD->setInvalid();
41124136

41134137
// Observing accessors (and their generated regular accessors) may have
41144138
// the type of the var inferred.
@@ -4138,9 +4162,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
41384162
if (auto gp = FD->getGenericParams()) {
41394163
gp->setOuterParameters(outerGenericParams);
41404164

4141-
if (TC.validateGenericFuncSignature(FD))
4142-
isInvalid = true;
4143-
else {
4165+
if (TC.validateGenericFuncSignature(FD)) {
4166+
markInvalidGenericSignature(FD, TC);
4167+
} else {
41444168
// Create a fresh archetype builder.
41454169
ArchetypeBuilder builder =
41464170
TC.createArchetypeBuilder(FD->getModuleContext());
@@ -4156,15 +4180,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
41564180
builder.inferRequirements(FD->getBodyResultTypeLoc(), gp);
41574181
}
41584182

4159-
// Revert all of the types within the signature of the function.
4183+
// Revert the types within the signature so it can be type-checked with
4184+
// archetypes below.
41604185
TC.revertGenericFuncSignature(FD);
41614186

4187+
// Assign archetypes.
41624188
finalizeGenericParamList(builder, FD->getGenericParams(), FD, TC);
41634189
}
41644190
} else if (outerGenericParams) {
4165-
if (TC.validateGenericFuncSignature(FD))
4166-
isInvalid = true;
4167-
else if (!FD->hasType()) {
4191+
if (TC.validateGenericFuncSignature(FD)) {
4192+
markInvalidGenericSignature(FD, TC);
4193+
} else if (!FD->hasType()) {
41684194
// Revert all of the types within the signature of the function.
41694195
TC.revertGenericFuncSignature(FD);
41704196
} else {
@@ -5511,8 +5537,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
55115537
gp->setOuterParameters(outerGenericParams);
55125538

55135539
if (TC.validateGenericFuncSignature(CD)) {
5514-
CD->overwriteType(ErrorType::get(TC.Context));
5515-
CD->setInvalid();
5540+
markInvalidGenericSignature(CD, TC);
55165541
} else {
55175542
ArchetypeBuilder builder =
55185543
TC.createArchetypeBuilder(CD->getModuleContext());
@@ -5527,7 +5552,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
55275552
// Infer requirements from the parameters of the constructor.
55285553
builder.inferRequirements(CD->getBodyParamPatterns()[1], gp);
55295554

5530-
// Revert the constructor signature so it can be type-checked with
5555+
// Revert the types within the signature so it can be type-checked with
55315556
// archetypes below.
55325557
TC.revertGenericFuncSignature(CD);
55335558

@@ -5536,7 +5561,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
55365561
}
55375562
} else if (outerGenericParams) {
55385563
if (TC.validateGenericFuncSignature(CD)) {
5539-
CD->overwriteType(ErrorType::get(TC.Context));
55405564
CD->setInvalid();
55415565
} else {
55425566
// Revert all of the types within the signature of the constructor.
@@ -5975,16 +5999,18 @@ void TypeChecker::validateDecl(ValueDecl *D, bool resolveTypeParams) {
59755999
if (proto->hasType())
59766000
return;
59776001
proto->computeType();
6002+
6003+
auto gp = proto->getGenericParams();
59786004

59796005
// Validate the generic type parameters.
59806006
validateGenericTypeSignature(proto);
59816007

5982-
revertGenericParamList(proto->getGenericParams());
6008+
revertGenericParamList(gp);
59836009

59846010
ArchetypeBuilder builder =
59856011
createArchetypeBuilder(proto->getModuleContext());
5986-
checkGenericParamList(builder, proto->getGenericParams(), *this);
5987-
finalizeGenericParamList(builder, proto->getGenericParams(), proto, *this);
6012+
checkGenericParamList(builder, gp, *this);
6013+
finalizeGenericParamList(builder, gp, proto, *this);
59886014

59896015
checkInheritanceClause(D);
59906016
validateAttributes(*this, D);

lib/Sema/TypeCheckGeneric.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ Type PartialGenericTypeToArchetypeResolver::resolveGenericTypeParamType(
9090
if (!gpDecl)
9191
return Type(gp);
9292

93+
9394
auto archetype = gpDecl->getArchetype();
9495
if (!archetype)
9596
return Type(gp);

test/Generics/associated_type_typo.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,17 @@ func typoAssoc4<T : P2 where T.Assocp2.assoc : P3>(_: T) { }
3939
// CHECK-NEXT: T[.P2].AssocP2[.P1].Assoc : P3 [explicit
4040
// CHECK-NEXT: T[.P2].AssocP2.assoc == T[.P2].AssocP2[.P1].Assoc [protocol
4141
// CHECK-NEXT: Generic signature
42+
43+
44+
// <rdar://problem/19620340>
45+
46+
func typoFunc1<T : P1>(x: TypoType) { // expected-error{{use of undeclared type 'TypoType'}}
47+
let _: T.Assoc -> () = { let _ = $0 }
48+
}
49+
50+
func typoFunc2<T : P1>(x: TypoType, y: T) { // expected-error{{use of undeclared type 'TypoType'}}
51+
let _: T.Assoc -> () = { let _ = $0 }
52+
}
53+
54+
func typoFunc3<T : P1>(x: TypoType, y: T.Assoc -> ()) { // expected-error{{use of undeclared type 'TypoType'}}
55+
}

validation-test/compiler_crashers/0357-swift-type-transform.swift renamed to validation-test/compiler_crashers_fixed/0357-swift-type-transform.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)

validation-test/compiler_crashers/0416-swift-typechecker-conformstoprotocol.swift renamed to validation-test/compiler_crashers_fixed/0416-swift-typechecker-conformstoprotocol.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)

validation-test/compiler_crashers/0535-swift-type-transform.swift renamed to validation-test/compiler_crashers_fixed/0535-swift-type-transform.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)

validation-test/compiler_crashers/1366-swift-constraints-constraintsystem-simplifyconstraint.swift renamed to validation-test/compiler_crashers_fixed/1366-swift-constraints-constraintsystem-simplifyconstraint.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)

validation-test/compiler_crashers/2108-swift-type-transform.swift renamed to validation-test/compiler_crashers_fixed/2108-swift-type-transform.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -parse
1+
// RUN: not %target-swift-frontend %s -parse
22

33
// Distributed under the terms of the MIT license
44
// Test case submitted to project by https://github.com/practicalswift (practicalswift)

0 commit comments

Comments
 (0)