Skip to content

Commit 7fd4615

Browse files
committed
SILGen: Fix crash when a stored property with an initial value has a reabstractable type
A constructor can constrain generic parameters more than the type itself, either because there is a 'where' clause on the constructor, or because the constructor is defined inside an extension with a 'where' clause. In this case, when the constructor calls a stored property initializer to implicitly initialize a stored property with an initial value, we must apply substitutions to get the correct result type for the call. If the original type of the stored property lowers differently based on the abstraction pattern, for example if it is a function type, then emitApply() would by default re-abstract the result to the most specific abstraction pattern possible. However, this was wrong because we store the result value into the stored property, and a stored property's type should always be lowered with the most generic abstraction pattern. In practice, this meant if you have a stored property of type (T) -> (), and an initializer with 'where T == String' for example, we would call the initializer, to produce a value with lowered type (@in_guaranteed String) -> (), then thunk it to a (@guaranteed String) -> (), and try to store the thunk into the stored property -- which has type (@in_guaranteed String) -> (). This would either miscompile or trigger an assertion. To get around this, we want to bypass the orig-to-subst conversion performed in emitApply(). My chosen solution is to have emitApply() emit the result into a ConvertingInitialization set up to perform a subst-to-orig conversion. Now that ConvertingInitialization is smart enough to peephole away matched pairs of orig-to-subst and subst-to-orig conversions, this always reduces to a no-op, and the emitApply() call produces and stores a value with the correct abstraction pattern. Fixes <rdar://problem/67419937>.
1 parent 85b94a4 commit 7fd4615

File tree

2 files changed

+71
-2
lines changed

2 files changed

+71
-2
lines changed

lib/SILGen/SILGenConstructor.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "ArgumentSource.h"
14+
#include "Conversion.h"
1415
#include "Initialization.h"
1516
#include "LValue.h"
1617
#include "RValue.h"
@@ -1058,8 +1059,37 @@ void SILGenFunction::emitMemberInitializers(DeclContext *dc,
10581059
// Figure out what we're initializing.
10591060
auto memberInit = emitMemberInit(*this, selfDecl, varPattern);
10601061

1061-
emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
1062-
origType, substType, memberInit.get());
1062+
// This whole conversion thing is about eliminating the
1063+
// paired orig-to-subst subst-to-orig conversions that
1064+
// will happen if the storage is at a different abstraction
1065+
// level than the constructor. When emitApply() is used
1066+
// to call the stored property initializer, it naturally
1067+
// wants to convert the result back to the most substituted
1068+
// abstraction level. To undo this, we use a converting
1069+
// initialization and rely on the peephole that optimizes
1070+
// out the redundant conversion.
1071+
auto loweredResultTy = getLoweredType(origType, substType);
1072+
auto loweredSubstTy = getLoweredType(substType);
1073+
1074+
if (loweredResultTy != loweredSubstTy) {
1075+
Conversion conversion = Conversion::getSubstToOrig(
1076+
origType, substType,
1077+
loweredResultTy);
1078+
1079+
ConvertingInitialization convertingInit(conversion,
1080+
SGFContext(memberInit.get()));
1081+
1082+
emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
1083+
origType, substType, &convertingInit);
1084+
1085+
auto finalValue = convertingInit.finishEmission(
1086+
*this, varPattern, ManagedValue::forInContext());
1087+
if (!finalValue.isInContext())
1088+
finalValue.forwardInto(*this, varPattern, memberInit.get());
1089+
} else {
1090+
emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
1091+
origType, substType, memberInit.get());
1092+
}
10631093
}
10641094
}
10651095
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
2+
3+
// rdar://problem/67419937
4+
5+
class Class<T> {
6+
var fn: ((T) -> ())? = nil
7+
8+
init(_: T) where T == Int {}
9+
}
10+
11+
// CHECK-LABEL: sil hidden [ossa] @$s34stored_property_init_reabstraction5ClassCyACySiGSicSiRszlufc : $@convention(method) (Int, @owned Class<Int>) -> @owned Class<Int> {
12+
// CHECK: [[SELF:%.*]] = mark_uninitialized [rootself] %1 : $Class<Int>
13+
// CHECK: [[BORROW:%.*]] = begin_borrow [[SELF]] : $Class<Int>
14+
// CHECK: [[ADDR:%.*]] = ref_element_addr [[BORROW]] : $Class<Int>, #Class.fn
15+
// CHECK: [[INIT:%.*]] = function_ref @$s34stored_property_init_reabstraction5ClassC2fnyxcSgvpfi : $@convention(thin) <τ_0_0> () -> @owned Optional<@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> () for <τ_0_0>>
16+
// CHECK: [[VALUE:%.*]] = apply [[INIT]]<Int>() : $@convention(thin) <τ_0_0> () -> @owned Optional<@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> () for <τ_0_0>>
17+
// CHECK: store [[VALUE]] to [init] [[ADDR]] : $*Optional<@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> () for <Int>>
18+
// CHECK: end_borrow [[BORROW]] : $Class<Int>
19+
20+
struct Struct<T> {
21+
var fn: ((T) -> ())? = nil
22+
23+
init(_: T) where T == Int {}
24+
}
25+
26+
// CHECK-LABEL: sil hidden [ossa] @$s34stored_property_init_reabstraction6StructVyACySiGSicSiRszlufC : $@convention(method) (Int, @thin Struct<Int>.Type) -> @owned Struct<Int> {
27+
// CHECK: [[SELF_BOX:%.*]] = mark_uninitialized [rootself] {{%.*}} : ${ var Struct<Int> }
28+
// CHECK: [[SELF:%.*]] = project_box [[SELF_BOX]] : ${ var Struct<Int> }, 0
29+
// CHECK: [[ADDR:%.*]] = struct_element_addr [[SELF]] : $*Struct<Int>, #Struct.fn
30+
// CHECK: [[INIT:%.*]] = function_ref @$s34stored_property_init_reabstraction6StructV2fnyxcSgvpfi : $@convention(thin) <τ_0_0> () -> @owned Optional<@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> () for <τ_0_0>>
31+
// CHECK: [[VALUE:%.*]] = apply [[INIT]]<Int>() : $@convention(thin) <τ_0_0> () -> @owned Optional<@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> () for <τ_0_0>>
32+
// CHECK: store [[VALUE]] to [init] [[ADDR]] : $*Optional<@callee_guaranteed @substituted <τ_0_0> (@in_guaranteed τ_0_0) -> () for <Int>>
33+
34+
struct ComplexExample<T, U> {
35+
var (fn, value): (((T) -> ())?, U?) = (nil, nil)
36+
var anotherValue: (((T) -> ())?, U?) = (nil, nil)
37+
38+
init(_: T) where T == String {}
39+
}

0 commit comments

Comments
 (0)