Skip to content

Commit 80fa362

Browse files
ahoppenahoppen
committed
[Sema] Fix crash when retrieving typeContextInfo for a partially bound generic type
In the following test case, we are crashing while building the generic signature of `someGenericFunc`, potentially invoked on `model` in line 11. ```swift struct MyBinding<BindingOuter> { func someGenericFunc<BindingInner>(x: BindingInner) {} } struct MyTextField<TextFieldOuter> { init<TextFieldInner>(text: MyBinding<TextFieldInner>) {} } struct EncodedView { func foo(model: MyBinding<String>) { let _ = MyTextField<String>(text: model) } } ``` Because we know that `model` has type `MyBinding<TextFieldInner>`, we substitute the `BindingOuter` generic parameter by `TextFieldInner`. Thus, `someGenericFunc` has the signature `<TextFieldInner /* substitutes BindingOuter */, BindingInner>`. `TextFieldInner` and `BindingOuter` both have `depth = 1`, `index = 0`. Thus the verification in `GenericSignatureBuilder` is failing. After discussion with Slava, the root issue appears to be that we shouldn’t be calling `subst` on a `GenericFunctionType` at all. Instead we should be using `substGenericArgs` which doesn’t attempt to rebuild a generic signature, but instead builds a non-generic function type. -------------------------------------------------------------------------------- We slightly regress in code completion results by showing two `collidingGeneric` twice in the following case. ```swift protocol P1 { func collidingGeneric<T>(x: T) } protocol P2 { func collidingGeneric<T>(x: T) } class C : P1, P2 { #^COMPLETE^# } ``` Previously, we were representing the type of `collidingGeneric` by a generic function type with generic param `T` that doesn’t have any restrictions. Since we are now using `substGenericArgs` instead of `subst`, we receive a non-generic function type that represents `T` as an archetype. And since that archetype is different for the two function signatures, we show the result twice in code completion. One could also argue that showing the result twice is intended (or at least acceptable) behaviour since, the two protocol may name their generic params differently. E.g. in ```swift protocol P1 { func collidingGeneric<S>(x: S) } protocol P2 { func collidingGeneric<T>(x: T) } class C : P1, P2 { #^COMPLETE^# } ``` we might be expected to show the following two results ``` func collidingGeneric<S>(x: S) func collidingGeneric<T>(x: T) ``` Resolves rdar://76711477 [SR-14495]
1 parent 47ca24d commit 80fa362

File tree

4 files changed

+78
-26
lines changed

4 files changed

+78
-26
lines changed

‎lib/Sema/LookupVisibleDecls.cpp

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ class OverrideFilteringConsumer : public VisibleDeclConsumer {
913913
continue;
914914
}
915915

916-
// Does it make sense to substitute types?
916+
ModuleDecl *M = DC->getParentModule();
917917

918918
// If the base type is AnyObject, we might be doing a dynamic
919919
// lookup, so the base type won't match the type of the member's
@@ -930,16 +930,35 @@ class OverrideFilteringConsumer : public VisibleDeclConsumer {
930930
(BaseTy->getNominalOrBoundGenericNominal() ||
931931
BaseTy->is<ArchetypeType>()) &&
932932
VD->getDeclContext()->isTypeContext());
933-
ModuleDecl *M = DC->getParentModule();
933+
934+
/// Substitute generic parameters in the signature of a found decl. The
935+
/// returned type can be used to determine if we have already found a
936+
/// conflicting declaration.
937+
auto substGenericArgs = [&](CanType SignatureType, ValueDecl *VD,
938+
Type BaseTy) -> CanType {
939+
if (!SignatureType || !shouldSubst) {
940+
return SignatureType;
941+
}
942+
if (auto GenFuncSignature =
943+
SignatureType->getAs<GenericFunctionType>()) {
944+
GenericEnvironment *GenEnv;
945+
if (auto *GenCtx = VD->getAsGenericContext()) {
946+
GenEnv = GenCtx->getGenericEnvironment();
947+
} else {
948+
GenEnv = DC->getGenericEnvironmentOfContext();
949+
}
950+
auto subs = BaseTy->getMemberSubstitutionMap(M, VD, GenEnv);
951+
auto CT = GenFuncSignature->substGenericArgs(subs);
952+
if (!CT->hasError()) {
953+
return CT->getCanonicalType();
954+
}
955+
}
956+
return SignatureType;
957+
};
934958

935959
auto FoundSignature = VD->getOverloadSignature();
936-
auto FoundSignatureType = VD->getOverloadSignatureType();
937-
if (FoundSignatureType && shouldSubst) {
938-
auto subs = BaseTy->getMemberSubstitutionMap(M, VD);
939-
auto CT = FoundSignatureType.subst(subs);
940-
if (!CT->hasError())
941-
FoundSignatureType = CT->getCanonicalType();
942-
}
960+
auto FoundSignatureType =
961+
substGenericArgs(VD->getOverloadSignatureType(), VD, BaseTy);
943962

944963
bool FoundConflicting = false;
945964
for (auto I = PossiblyConflicting.begin(), E = PossiblyConflicting.end();
@@ -952,14 +971,9 @@ class OverrideFilteringConsumer : public VisibleDeclConsumer {
952971
continue;
953972

954973
auto OtherSignature = OtherVD->getOverloadSignature();
955-
auto OtherSignatureType = OtherVD->getOverloadSignatureType();
956-
if (OtherSignatureType && shouldSubst) {
957-
auto ActualBaseTy = getBaseTypeForMember(M, OtherVD, BaseTy);
958-
auto subs = ActualBaseTy->getMemberSubstitutionMap(M, OtherVD);
959-
auto CT = OtherSignatureType.subst(subs);
960-
if (!CT->hasError())
961-
OtherSignatureType = CT->getCanonicalType();
962-
}
974+
auto ActualBaseTy = getBaseTypeForMember(M, OtherVD, BaseTy);
975+
auto OtherSignatureType = substGenericArgs(
976+
OtherVD->getOverloadSignatureType(), OtherVD, ActualBaseTy);
963977

964978
if (conflicting(M->getASTContext(), FoundSignature, FoundSignatureType,
965979
OtherSignature, OtherSignatureType,

‎test/IDE/complete_override_access_control_protocol.swift

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,21 @@ public class TestPublicDE : ProtocolDPrivate, ProtocolEPublic {
177177
#^TEST_PUBLIC_DE^#
178178
}
179179

180-
// TEST_PRIVATE_DE: Begin completions, 2 items
180+
// FIXME: We should only be suggesting `collidingGeneric` once in all the test cases below.
181+
182+
// TEST_PRIVATE_DE: Begin completions, 3 items
181183
// TEST_PRIVATE_DE-DAG: Decl[InstanceMethod]/Super: func colliding() {|}{{; name=.+$}}
182184
// TEST_PRIVATE_DE-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
185+
// TEST_PRIVATE_DE-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
183186

184-
// TEST_INTERNAL_DE: Begin completions, 2 items
187+
// TEST_INTERNAL_DE: Begin completions, 3 items
185188
// TEST_INTERNAL_DE-DAG: Decl[InstanceMethod]/Super: func colliding() {|}{{; name=.+$}}
186189
// TEST_INTERNAL_DE-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
190+
// TEST_INTERNAL_DE-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
187191

188-
// TEST_PUBLIC_DE: Begin completions, 2 items
192+
// TEST_PUBLIC_DE: Begin completions, 3 items
189193
// TEST_PUBLIC_DE-DAG: Decl[InstanceMethod]/Super: public func colliding() {|}{{; name=.+$}}
194+
// TEST_PUBLIC_DE-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
190195
// TEST_PUBLIC_DE-DAG: Decl[InstanceMethod]/Super: public func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
191196

192197
private class TestPrivateED : ProtocolEPublic, ProtocolDPrivate {
@@ -203,16 +208,19 @@ public class TestPublicED : ProtocolEPublic, ProtocolDPrivate {
203208
#^TEST_PUBLIC_ED^#
204209
}
205210

206-
// TEST_PRIVATE_ED: Begin completions, 2 items
211+
// TEST_PRIVATE_ED: Begin completions, 3 items
207212
// TEST_PRIVATE_ED-DAG: Decl[InstanceMethod]/Super: func colliding() {|}{{; name=.+$}}
208213
// TEST_PRIVATE_ED-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
214+
// TEST_PRIVATE_ED-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
209215

210-
// TEST_INTERNAL_ED: Begin completions, 2 items
216+
// TEST_INTERNAL_ED: Begin completions, 3 items
217+
// TEST_INTERNAL_ED-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
211218
// TEST_INTERNAL_ED-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
212219
// TEST_INTERNAL_ED-DAG: Decl[InstanceMethod]/Super: func colliding() {|}{{; name=.+$}}
213220

214-
// TEST_PUBLIC_ED: Begin completions, 2 items
221+
// TEST_PUBLIC_ED: Begin completions, 3 items
215222
// TEST_PUBLIC_ED-DAG: Decl[InstanceMethod]/Super: public func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
223+
// TEST_PUBLIC_ED-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
216224
// TEST_PUBLIC_ED-DAG: Decl[InstanceMethod]/Super: public func colliding() {|}{{; name=.+$}}
217225

218226
private class TestPrivateEF : ProtocolEPublic, ProtocolFPublic {
@@ -229,14 +237,17 @@ public class TestPublicEF : ProtocolEPublic, ProtocolFPublic {
229237
#^TEST_PUBLIC_EF^#
230238
}
231239

232-
// TEST_PRIVATE_EF: Begin completions, 2 items
240+
// TEST_PRIVATE_EF: Begin completions, 3 items
233241
// TEST_PRIVATE_EF-DAG: Decl[InstanceMethod]/Super: func colliding() {|}{{; name=.+$}}
234242
// TEST_PRIVATE_EF-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
243+
// TEST_PRIVATE_EF-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
235244

236-
// TEST_INTERNAL_EF: Begin completions, 2 items
245+
// TEST_INTERNAL_EF: Begin completions, 3 items
237246
// TEST_INTERNAL_EF-DAG: Decl[InstanceMethod]/Super: func colliding() {|}{{; name=.+$}}
238247
// TEST_INTERNAL_EF-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
248+
// TEST_INTERNAL_EF-DAG: Decl[InstanceMethod]/Super: func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
239249

240-
// TEST_PUBLIC_EF: Begin completions, 2 items
250+
// TEST_PUBLIC_EF: Begin completions, 3 items
241251
// TEST_PUBLIC_EF-DAG: Decl[InstanceMethod]/Super: public func colliding() {|}{{; name=.+$}}
242252
// TEST_PUBLIC_EF-DAG: Decl[InstanceMethod]/Super: public func collidingGeneric<T>(x: T) {|}{{; name=.+$}}
253+
// TEST_PUBLIC_EF-DAG: Decl[InstanceMethod]/Super: public func collidingGeneric<T>(x: T) {|}{{; name=.+$}}

‎test/SourceKit/TypeContextInfo/typecontext_generics.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@ struct S<T> {
1919
static var instance: Self = S<T>()
2020
}
2121

22+
struct MyBinding<BindingOuter> {
23+
func someGenericFunc<BindingInner>(x: BindingInner) {}
24+
}
25+
26+
struct MyTextField<TextFieldOuter> {
27+
init<TextFieldInner>(text: MyBinding<TextFieldInner>) {}
28+
}
29+
30+
struct EncodedView {
31+
func foo(model: MyBinding<String>) {
32+
let _ = MyTextField<String>(text: model)
33+
}
34+
}
35+
2236
// RUN: %sourcekitd-test -req=typecontextinfo -pos=7:12 %s -- %s > %t.response.1
2337
// RUN: %diff -u %s.response.1 %t.response.1
2438
// RUN: %sourcekitd-test -req=typecontextinfo -pos=8:12 %s -- %s > %t.response.2
@@ -32,3 +46,6 @@ struct S<T> {
3246
// RUN: %diff -u %s.response.5 %t.response.5
3347
// RUN: %sourcekitd-test -req=typecontextinfo -pos=16:18 %s -- %s > %t.response.6
3448
// RUN: %diff -u %s.response.6 %t.response.6
49+
50+
// RUN: %sourcekitd-test -req=typecontextinfo -pos=32:43 %s -- %s > %t.response.7
51+
// RUN: %diff -u %s.response.7 %t.response.7

‎test/SourceKit/TypeContextInfo/typecontext_generics.swift.response.7

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
key.results: [
3+
{
4+
key.typename: "MyBinding<TextFieldInner>",
5+
key.typeusr: "$s20typecontext_generics9MyBindingVyqd__GD",
6+
key.implicitmembers: [
7+
]
8+
}
9+
]
10+
}

0 commit comments

Comments
 (0)