Skip to content

Commit c88c967

Browse files
committed
AST: Fix issue where SIL type lowering did not commute with subst()
SIL type lowering uses ReplaceOpaqueTypesWithUnderlyingTypes to lower away opaque result types when the current context can "see" the underlying type of the opaque result type. To determine if an opaque result type could be replaced with its underlying type, we check if every nominal type declaration appearing in the "fully substituted" underlying type is visible from the current context. To form the "fully substituted" type, we first apply the substitution map stored in the opaque type decl, which replaces the 'Self' generic parameter with the actual concrete result type. Then, we apply the substitution map stored in the archetype itself. However, calling subst() on an opaque archetype modifies the substitution map stored in the archetype. What this means in practice is that if I have an opaque type declaration that depends on its outer generic parameters, the fully-substituted type that I see here depends on whether subst() has been performed or not. For example, suppose I have the following: protocol P {} struct S : P {} extension P { func foo() -> some Any { return [self] } } The opaque result decl for P.foo() maps 'Self' to 'Array<Self>'. Now, imagine someone calls foo() with the substitution 'Self := S'. The fully-substituted underlying type is 'Array<S>'. If 'S' is private, we will decide that we cannot replace the opaque result type with its underlying type. However, if we had performed the opaque type replacement _before_ substituting 'Self := S', then we will end up replacing the opaque result type. To re-state this in yet another form, we want the following identity to hold here: getLoweredType(opaqueType).subst(subMap) == getLoweredType(opaqueType.subst(subMap)) We can ensure that this holds by only applying the archetype's substitutions _after_ we check visibility. Fixes rdar://problem/76556368.
1 parent c475105 commit c88c967

File tree

3 files changed

+54
-11
lines changed

3 files changed

+54
-11
lines changed

lib/AST/Type.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3181,15 +3181,12 @@ operator()(SubstitutableType *maybeOpaqueType) const {
31813181
// archetype in question. This will map the inner generic signature of the
31823182
// opaque type to its outer signature.
31833183
auto partialSubstTy = archetype->getInterfaceType().subst(*subs);
3184-
// Then apply the substitutions from the root opaque archetype, to specialize
3185-
// for its type arguments.
3186-
auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
31873184

31883185
// Check that we are allowed to substitute the underlying type into the
31893186
// context.
31903187
auto inContext = this->inContext;
31913188
auto isContextWholeModule = this->isContextWholeModule;
3192-
if (substTy.findIf(
3189+
if (partialSubstTy.findIf(
31933190
[inContext, substitutionKind, isContextWholeModule](Type t) -> bool {
31943191
if (!canSubstituteTypeInto(t, inContext, substitutionKind,
31953192
isContextWholeModule))
@@ -3198,6 +3195,12 @@ operator()(SubstitutableType *maybeOpaqueType) const {
31983195
}))
31993196
return maybeOpaqueType;
32003197

3198+
// Then apply the substitutions from the root opaque archetype, to specialize
3199+
// for its type arguments. We perform this substitution after checking for
3200+
// visibility, since we do not want the result of the visibility check to
3201+
// depend on the substitutions previously applied.
3202+
auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
3203+
32013204
// If the type changed, but still contains opaque types, recur.
32023205
if (!substTy->isEqual(maybeOpaqueType) && substTy->hasOpaqueArchetype()) {
32033206
return ::substOpaqueTypesWithUnderlyingTypes(
@@ -3264,18 +3267,12 @@ operator()(CanType maybeOpaqueType, Type replacementType,
32643267
// archetype in question. This will map the inner generic signature of the
32653268
// opaque type to its outer signature.
32663269
auto partialSubstTy = archetype->getInterfaceType().subst(*subs);
3267-
auto partialSubstRef =
3268-
abstractRef.subst(archetype->getInterfaceType(), *subs);
3269-
3270-
// Then apply the substitutions from the root opaque archetype, to specialize
3271-
// for its type arguments.
3272-
auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
32733270

32743271
// Check that we are allowed to substitute the underlying type into the
32753272
// context.
32763273
auto inContext = this->inContext;
32773274
auto isContextWholeModule = this->isContextWholeModule;
3278-
if (substTy.findIf(
3275+
if (partialSubstTy.findIf(
32793276
[inContext, substitutionKind, isContextWholeModule](Type t) -> bool {
32803277
if (!canSubstituteTypeInto(t, inContext, substitutionKind,
32813278
isContextWholeModule))
@@ -3284,6 +3281,14 @@ operator()(CanType maybeOpaqueType, Type replacementType,
32843281
}))
32853282
return abstractRef;
32863283

3284+
// Then apply the substitutions from the root opaque archetype, to specialize
3285+
// for its type arguments. We perform this substitution after checking for
3286+
// visibility, since we do not want the result of the visibility check to
3287+
// depend on the substitutions previously applied.
3288+
auto substTy = partialSubstTy.subst(opaqueRoot->getSubstitutions());
3289+
3290+
auto partialSubstRef =
3291+
abstractRef.subst(archetype->getInterfaceType(), *subs);
32873292
auto substRef =
32883293
partialSubstRef.subst(partialSubstTy, opaqueRoot->getSubstitutions());
32893294

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
public protocol P {
2+
associatedtype A
3+
var v: A { get }
4+
}
5+
6+
public extension P {
7+
func foo() -> some P {
8+
return self
9+
}
10+
}
11+
12+
public struct S: P {
13+
public init() {}
14+
public var v: some P { return self }
15+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// RUN: %target-swift-frontend -disable-availability-checking -emit-module %S/Inputs/opaque_result_type_private_substitution_other.swift -emit-module-path %t/opaque_result_type_private_substitution_other.swiftmodule
4+
// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen %s -I %t -DMODULE
5+
6+
// RUN: %target-swift-frontend -disable-availability-checking -emit-module %S/Inputs/opaque_result_type_private_substitution_other.swift -emit-module-path %t/opaque_result_type_private_substitution_other.swiftmodule -enable-library-evolution
7+
// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen %s -I %t -DMODULE
8+
9+
// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen %s %S/Inputs/opaque_result_type_private_substitution_other.swift
10+
// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen -primary-file %s %S/Inputs/opaque_result_type_private_substitution_other.swift
11+
// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen -primary-file %s -primary-file %S/Inputs/opaque_result_type_private_substitution_other.swift
12+
13+
#if MODULE
14+
import opaque_result_type_private_substitution_other
15+
#endif
16+
17+
struct S1: P {
18+
var v: some P { S2().foo() }
19+
}
20+
21+
private struct S2: P {
22+
var v: some P { S() }
23+
}

0 commit comments

Comments
 (0)