Skip to content

Commit 865b155

Browse files
committed
SIL: Thunks that capture local environments must also capture the primary environment
I suspect there are latent bugs here with generic-class-constrained existentials, same-element requirements involving packs, etc. We can't assume that the local archetypes don't have "hidden" dependencies on the outer parameters that are not encoded in the thunk's interface type.
1 parent da31c59 commit 865b155

File tree

4 files changed

+16
-29
lines changed

4 files changed

+16
-29
lines changed

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3021,43 +3021,39 @@ struct LocalArchetypeRequirementCollector {
30213021
///
30223022
/// \param localArchetypes - the list of local archetypes to promote
30233023
/// into the signature, if any
3024-
/// \param inheritGenericSig - whether to inherit the generic signature from the
3025-
/// parent function.
30263024
/// \param genericEnv - the new generic environment
30273025
/// \param contextSubs - map non-local archetypes from the original function
30283026
/// to archetypes in the thunk
30293027
/// \param interfaceSubs - map interface types to old archetypes
30303028
static CanGenericSignature
30313029
buildThunkSignature(SILFunction *fn,
3032-
bool inheritGenericSig,
30333030
ArrayRef<CanLocalArchetypeType> localArchetypes,
30343031
GenericEnvironment *&genericEnv,
30353032
SubstitutionMap &contextSubs,
30363033
SubstitutionMap &interfaceSubs,
30373034
llvm::DenseMap<ArchetypeType*, Type> &contextLocalArchetypes) {
30383035
auto *mod = fn->getModule().getSwiftModule();
30393036
auto &ctx = mod->getASTContext();
3037+
auto forwardingSubs = fn->getForwardingSubstitutionMap();
30403038

30413039
// If there are no local archetypes, we just inherit the generic
30423040
// environment from the parent function.
30433041
if (localArchetypes.empty()) {
30443042
auto genericSig =
30453043
fn->getLoweredFunctionType()->getInvocationGenericSignature();
30463044
genericEnv = fn->getGenericEnvironment();
3047-
interfaceSubs = fn->getForwardingSubstitutionMap();
3045+
interfaceSubs = forwardingSubs;
30483046
contextSubs = interfaceSubs;
30493047
return genericSig;
30503048
}
30513049

30523050
// Add the existing generic signature.
30533051
unsigned depth = 0;
30543052
GenericSignature baseGenericSig;
3055-
if (inheritGenericSig) {
3056-
if (auto genericSig =
3057-
fn->getLoweredFunctionType()->getInvocationGenericSignature()) {
3058-
baseGenericSig = genericSig;
3059-
depth = genericSig.getGenericParams().back()->getDepth() + 1;
3060-
}
3053+
if (auto genericSig =
3054+
fn->getLoweredFunctionType()->getInvocationGenericSignature()) {
3055+
baseGenericSig = genericSig;
3056+
depth = genericSig.getGenericParams().back()->getDepth() + 1;
30613057
}
30623058

30633059
// Add new generic parameters to replace the local archetypes.
@@ -3088,13 +3084,11 @@ buildThunkSignature(SILFunction *fn,
30883084
->getInvocationGenericSignature()) {
30893085
contextSubs = SubstitutionMap::get(
30903086
calleeGenericSig,
3091-
[&](SubstitutableType *type) -> Type {
3092-
return genericEnv->mapTypeIntoContext(type);
3093-
},
3094-
MakeAbstractConformanceForGenericType());
3087+
genericEnv->getForwardingSubstitutionMap());
30953088
}
30963089

30973090
// Calculate substitutions to map interface types to the caller's archetypes.
3091+
30983092
interfaceSubs = SubstitutionMap::get(
30993093
genericSig,
31003094
[&](SubstitutableType *type) -> Type {
@@ -3103,7 +3097,7 @@ buildThunkSignature(SILFunction *fn,
31033097
return collector.ParamSubs[param->getIndex()];
31043098
}
31053099
}
3106-
return fn->mapTypeIntoContext(type);
3100+
return Type(type).subst(forwardingSubs);
31073101
},
31083102
MakeAbstractConformanceForGenericType());
31093103

@@ -3142,18 +3136,14 @@ CanSILFunctionType swift::buildSILFunctionThunkType(
31423136
if (withoutActuallyEscaping)
31433137
extInfoBuilder = extInfoBuilder.withNoEscape(false);
31443138

3145-
// Does the thunk type involve archetypes other than local archetypes?
3146-
bool hasArchetypes = false;
31473139
// Does the thunk type involve a local archetype type?
31483140
SmallVector<CanLocalArchetypeType, 8> localArchetypes;
31493141
auto archetypeVisitor = [&](CanType t) {
31503142
if (auto archetypeTy = dyn_cast<ArchetypeType>(t)) {
3151-
if (auto opened = dyn_cast<LocalArchetypeType>(archetypeTy)) {
3152-
auto root = opened.getRoot();
3143+
if (auto local = dyn_cast<LocalArchetypeType>(archetypeTy)) {
3144+
auto root = local.getRoot();
31533145
if (llvm::find(localArchetypes, root) == localArchetypes.end())
31543146
localArchetypes.push_back(root);
3155-
} else {
3156-
hasArchetypes = true;
31573147
}
31583148
}
31593149
};
@@ -3169,7 +3159,6 @@ CanSILFunctionType swift::buildSILFunctionThunkType(
31693159
sourceType.visit(archetypeVisitor);
31703160

31713161
genericSig = buildThunkSignature(fn,
3172-
hasArchetypes,
31733162
localArchetypes,
31743163
genericEnv,
31753164
contextSubs,

test/SILGen/partial_apply_protocol.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ func testClonable(c: Clonable) {
5454

5555
//===----------------------------------------------------------------------===//
5656
// Partial apply of methods returning Self-derived types from generic context
57-
//
58-
// Make sure the thunk only has the context generic parameters if needed!
5957
//===----------------------------------------------------------------------===//
6058

6159
// CHECK-LABEL: sil hidden [ossa] @$s22partial_apply_protocol28testClonableInGenericContext1c1tyAA0E0_p_xtlF : $@convention(thin) <T> (@in_guaranteed any Clonable, @in_guaranteed T) -> ()
@@ -75,8 +73,8 @@ func testClonableInGenericContext<T>(c: Clonable, t: T) {
7573
// CHECK: [[METHOD_FN:%.*]] = witness_method $@opened("{{.*}}", any Clonable) Self, #Clonable.getCloneFn :
7674
// CHECK: [[RESULT:%.*]] = apply [[METHOD_FN]]<@opened("{{.*}}", any Clonable) Self>({{.*}})
7775
// CHECK: [[RESULT_CONV:%.*]] = convert_function [[RESULT]]
78-
// CHECK: [[THUNK_FN:%.*]] = function_ref @$sxIegr_22partial_apply_protocol8Clonable_pIegr_AaBRzlTR : $@convention(thin) <τ_0_0 where τ_0_0 : Clonable> (@guaranteed @callee_guaranteed () -> @out τ_0_0) -> @out any Clonable
79-
// CHECK: [[THUNK:%.*]] = partial_apply [callee_guaranteed] [[THUNK_FN]]<@opened("{{.*}}", any Clonable) Self>([[RESULT_CONV]]) : $@convention(thin) <τ_0_0 where τ_0_0 : Clonable> (@guaranteed @callee_guaranteed () -> @out τ_0_0) -> @out any Clonable
76+
// CHECK: [[THUNK_FN:%.*]] = function_ref @$sqd__Iegr_22partial_apply_protocol8Clonable_pIegr_AaBRd__r__lTR : $@convention(thin) <τ_0_0><τ_1_0 where τ_1_0 : Clonable> (@guaranteed @callee_guaranteed () -> @out τ_1_0) -> @out any Clonable
77+
// CHECK: [[THUNK:%.*]] = partial_apply [callee_guaranteed] [[THUNK_FN]]<T, @opened("{{.*}}", any Clonable) Self>([[RESULT_CONV]]) : $@convention(thin) <τ_0_0><τ_1_0 where τ_1_0 : Clonable> (@guaranteed @callee_guaranteed () -> @out τ_1_0) -> @out any Clonable
8078
let _: () -> Clonable = c.getCloneFn()
8179

8280
// CHECK: [[THUNK_FN:%.*]] = function_ref @$s22partial_apply_protocol28testClonableInGenericContext1c1tyAA0E0_p_xtlFAaE_pycycAaE_pcfu5_ : $@convention(thin) (@in_guaranteed any Clonable) -> @owned @callee_guaranteed () -> @owned @callee_guaranteed () -> @out any Clonable

test/SILGen/variadic-generic-reabstract-tuple-result.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ func test11<each T>() -> Use<repeat each T> {
312312
// CHECK-NEXT: [[DEST_ELT_ADDR:%.*]] = pack_element_get [[EXPANSION_INDEX]] of %0 : $*Pack{repeat @callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> Bool for <each T>} as $*@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> Bool for <@pack_element([[UUID]]) each T>
313313
// CHECK-NEXT: [[ESCAPING_ELT:%.*]] = convert_function [[ELT]]
314314
// CHECK-NEXT: // function_ref
315-
// CHECK-NEXT: [[THUNK:%.*]] = function_ref @$sxSbIegnr_xSbIegnd_lTR : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0, @guaranteed @callee_guaranteed (@in_guaranteed τ_0_0) -> @out Bool) -> Bool
315+
// CHECK-NEXT: [[THUNK:%.*]] = function_ref @$sqd__SbIegnr_qd__SbIegnd_Rvzr__lTR : $@convention(thin) <each τ_0_0><τ_1_0> (@in_guaranteed τ_1_0, @guaranteed @callee_guaranteed (@in_guaranteed τ_1_0) -> @out Bool) -> Bool
316316
// CHECK-NEXT: [[CONVERTED_ELT:%.*]] = partial_apply
317317
// CHECK-NEXT: [[CONVERTED_ELT_2:%.*]] = convert_function [[CONVERTED_ELT]]
318318
// CHECK-NEXT: store [[CONVERTED_ELT_2]] to [init] [[DEST_ELT_ADDR]] :

test/SILGen/variadic-generic-reabstraction.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ func forwardAndReabstractFunctionPack<each T>(functions: repeat (each T) -> Bool
2727
// CHECK-NEXT: [[COPY_CONVERT:%.*]] = convert_function [[COPY]] : $@noescape @callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> Bool for <@pack_element([[UUID]]) each T> to $@noescape @callee_guaranteed (@in_guaranteed @pack_element([[UUID]]) each T) -> Bool
2828
// Wrap in the conversion thunk.
2929
// CHECK-NEXT: // function_ref
30-
// CHECK-NEXT: [[THUNK:%.*]] = function_ref @$sxSbIgnd_xSbIegnr_lTR : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0, @guaranteed @noescape @callee_guaranteed (@in_guaranteed τ_0_0) -> Bool) -> @out Bool
31-
// CHECK-NEXT: [[THUNKED:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]<@pack_element([[UUID]]) each T>([[COPY_CONVERT]])
30+
// CHECK-NEXT: [[THUNK:%.*]] = function_ref @$sqd__SbIgnd_qd__SbIegnr_Rvzr__lTR : $@convention(thin) <each τ_0_0><τ_1_0> (@in_guaranteed τ_1_0, @guaranteed @noescape @callee_guaranteed (@in_guaranteed τ_1_0) -> Bool) -> @out Bool
31+
// CHECK-NEXT: [[THUNKED:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]<Pack{repeat each T}, @pack_element([[UUID]]) each T>([[COPY_CONVERT]])
3232
// Convert to a substituted type.
3333
// CHECK-NEXT: [[THUNKED_CONVERT:%.*]] = convert_function [[THUNKED]] : $@callee_guaranteed (@in_guaranteed @pack_element([[UUID]]) each T) -> @out Bool to $@callee_guaranteed @substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <@pack_element([[UUID]]) each T, Bool>
3434
// Convert to noescape.

0 commit comments

Comments
 (0)