Skip to content

Commit 268acbc

Browse files
committed
SIL: Use @in_guaranteed convention for 'self' parameter of materializeForSet callback of non-mutating setter
We cannot in general use @guaranteed here, otherwise classes will not be able to conform to protocols with mutable property requirements (or we could always open-code materializeForSet witness thunks for classes, but that has its own downsides so its not a clear win). Fixes <rdar://problem/36867783>.
1 parent 11f949e commit 268acbc

11 files changed

+35
-33
lines changed

lib/SIL/TypeLowering.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,25 +2071,28 @@ CanSILFunctionType TypeConverter::getMaterializeForSetCallbackType(
20712071
auto selfMetatypeType = MetatypeType::get(selfType,
20722072
MetatypeRepresentation::Thick);
20732073

2074+
auto canSelfType = selfType->getCanonicalType(genericSig);
2075+
auto canSelfMetatypeType = selfMetatypeType->getCanonicalType(genericSig);
2076+
auto selfConvention = (storage->isSetterMutating()
2077+
? ParameterConvention::Indirect_Inout
2078+
: ParameterConvention::Indirect_In_Guaranteed);
2079+
20742080
{
20752081
GenericContextScope scope(*this, genericSig);
20762082

20772083
// If 'self' is a metatype, make it @thin or @thick as needed, but not inside
20782084
// selfMetatypeType.
2079-
if (auto metatype = selfType->getAs<MetatypeType>()) {
2085+
if (auto metatype = canSelfType->getAs<MetatypeType>()) {
20802086
if (!metatype->hasRepresentation())
2081-
selfType = getLoweredType(metatype).getSwiftRValueType();
2087+
canSelfType = getLoweredType(metatype).getSwiftRValueType();
20822088
}
20832089
}
20842090

2085-
auto canSelfType = selfType->getCanonicalType(genericSig);
2086-
auto canSelfMetatypeType = selfMetatypeType->getCanonicalType(genericSig);
2087-
20882091
// Create the SILFunctionType for the callback.
20892092
SILParameterInfo params[] = {
20902093
{ ctx.TheRawPointerType, ParameterConvention::Direct_Unowned },
20912094
{ ctx.TheUnsafeValueBufferType, ParameterConvention::Indirect_Inout },
2092-
{ canSelfType, ParameterConvention::Indirect_Inout },
2095+
{ canSelfType, selfConvention },
20932096
{ canSelfMetatypeType, ParameterConvention::Direct_Unowned },
20942097
};
20952098
ArrayRef<SILResultInfo> results = {};

lib/SILGen/SILGenMaterializeForSet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ MaterializeForSetEmitter::createSetterCallback(SILFunction &F,
964964
}
965965

966966
// The callback gets the address of 'self' at +0.
967-
ManagedValue mSelf = ManagedValue::forLValue(self);
967+
ManagedValue mSelf = ManagedValue::forUnmanaged(self);
968968

969969
// That's enough to build the l-value.
970970
LValue lvalue = buildLValue(SGF, loc, mSelf, std::move(indices),

test/SILGen/accessors.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,8 @@ func test0(_ ref: A) {
6565
// CHECK-NEXT: switch_enum [[OPT_CALLBACK]] : $Optional<Builtin.RawPointer>, case #Optional.some!enumelt.1: [[WRITEBACK:bb[0-9]+]], case #Optional.none!enumelt: [[CONT:bb[0-9]+]]
6666

6767
// CHECK: [[WRITEBACK]]([[CALLBACK_ADDR:%.*]] : @trivial $Builtin.RawPointer):
68-
// CHECK-NEXT: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout A, @thick A.Type) -> ()
68+
// CHECK-NEXT: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed A, @thick A.Type) -> ()
6969
// CHECK-NEXT: [[TEMP2:%.*]] = alloc_stack $A
70-
// SEMANTIC SIL TODO: This is an issue caused by the callback for materializeForSet in the class case taking the value as @inout when it should really take it as @guaranteed.
7170
// CHECK-NEXT: store_borrow [[BORROWED_ARG_LHS]] to [[TEMP2]] : $*A
7271
// CHECK-NEXT: [[T0:%.*]] = metatype $@thick A.Type
7372
// CHECK-NEXT: [[T1:%.*]] = address_to_pointer [[ADDR]] : $*OrdinarySub to $Builtin.RawPointer
@@ -128,7 +127,7 @@ func test1(_ ref: B) {
128127
// CHECK-NEXT: switch_enum [[OPT_CALLBACK]] : $Optional<Builtin.RawPointer>, case #Optional.some!enumelt.1: [[WRITEBACK:bb[0-9]+]], case #Optional.none!enumelt: [[CONT:bb[0-9]+]]
129128
//
130129
// CHECK: [[WRITEBACK]]([[CALLBACK_ADDR:%.*]] : @trivial $Builtin.RawPointer):
131-
// CHECK-NEXT: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout B, @thick B.Type) -> ()
130+
// CHECK-NEXT: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed B, @thick B.Type) -> ()
132131
// CHECK-NEXT: [[TEMP2:%.*]] = alloc_stack $B
133132
// CHECK-NEXT: store_borrow [[BORROWED_ARG_RHS]] to [[TEMP2]] : $*B
134133
// CHECK-NEXT: [[T0:%.*]] = metatype $@thick B.Type
@@ -154,7 +153,7 @@ func test1(_ ref: B) {
154153
// CHECK-NEXT: switch_enum [[OPT_CALLBACK]] : $Optional<Builtin.RawPointer>, case #Optional.some!enumelt.1: [[WRITEBACK:bb[0-9]+]], case #Optional.none!enumelt: [[CONT:bb[0-9]+]]
155154
//
156155
// CHECK: [[WRITEBACK]]([[CALLBACK_ADDR:%.*]] : @trivial $Builtin.RawPointer):
157-
// CHECK-NEXT: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout B, @thick B.Type) -> ()
156+
// CHECK-NEXT: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed B, @thick B.Type) -> ()
158157
// CHECK-NEXT: [[TEMP2:%.*]] = alloc_stack $B
159158
// CHECK-NEXT: store_borrow [[BORROWED_ARG_LHS]] to [[TEMP2]] : $*B
160159
// CHECK-NEXT: [[T0:%.*]] = metatype $@thick B.Type

test/SILGen/addressors.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ class G {
354354
// CHECK: strong_release [[OWNER]] : $Builtin.NativeObject
355355

356356
// materializeForSet callback for G.value
357-
// CHECK-LABEL: sil private [transparent] @$S10addressors1GC5values5Int32VvmytfU_ : $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout G, @thick G.Type) -> () {
357+
// CHECK-LABEL: sil private [transparent] @$S10addressors1GC5values5Int32VvmytfU_ : $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed G, @thick G.Type) -> () {
358358
// CHECK: bb0([[BUFFER:%0]] : $Builtin.RawPointer, [[STORAGE:%1]] : $*Builtin.UnsafeValueBuffer, [[SELF:%2]] : $*G, [[SELFTYPE:%3]] : $@thick G.Type):
359359
// CHECK: [[T0:%.*]] = project_value_buffer $Builtin.NativeObject in [[STORAGE]] : $*Builtin.UnsafeValueBuffer
360360
// CHECK: [[OWNER:%.*]] = load [[T0]]
@@ -475,7 +475,7 @@ class I {
475475
// CHECK: strong_unpin [[OWNER]] : $Optional<Builtin.NativeObject>
476476

477477
// materializeForSet callback for I.value
478-
// CHECK-LABEL: sil private [transparent] @$S10addressors1IC5values5Int32VvmytfU_ : $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout I, @thick I.Type) -> () {
478+
// CHECK-LABEL: sil private [transparent] @$S10addressors1IC5values5Int32VvmytfU_ : $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed I, @thick I.Type) -> () {
479479
// CHECK: bb0([[BUFFER:%0]] : $Builtin.RawPointer, [[STORAGE:%1]] : $*Builtin.UnsafeValueBuffer, [[SELF:%2]] : $*I, [[SELFTYPE:%3]] : $@thick I.Type):
480480
// CHECK: [[T0:%.*]] = project_value_buffer $Optional<Builtin.NativeObject> in [[STORAGE]] : $*Builtin.UnsafeValueBuffer
481481
// CHECK: [[OWNER:%.*]] = load [[T0]]

test/SILGen/constrained_extensions.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public class GenericClass<X, Y> {}
137137
extension GenericClass where Y == () {
138138
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlE5valuexvg : $@convention(method) <X, Y where Y == ()> (@guaranteed GenericClass<X, ()>) -> @out X
139139
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlE5valuexvs : $@convention(method) <X, Y where Y == ()> (@in X, @guaranteed GenericClass<X, ()>) -> ()
140-
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlE5valuexvmytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
140+
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlE5valuexvmytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
141141
// CHECK-LABEL: sil [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlE5valuexvm : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @guaranteed GenericClass<X, ()>) -> (Builtin.RawPointer, Optional<Builtin.RawPointer>)
142142
public var value: X {
143143
get { while true {} }
@@ -146,7 +146,7 @@ extension GenericClass where Y == () {
146146

147147
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlE5emptyytvg : $@convention(method) <X, Y where Y == ()> (@guaranteed GenericClass<X, ()>) -> ()
148148
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlE5emptyytvs : $@convention(method) <X, Y where Y == ()> (@guaranteed GenericClass<X, ()>) -> ()
149-
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlE5emptyytvmytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
149+
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlE5emptyytvmytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
150150
// CHECK-LABEL: sil [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlE5emptyytvm : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @guaranteed GenericClass<X, ()>) -> (Builtin.RawPointer, Optional<Builtin.RawPointer>)
151151
public var empty: Y {
152152
get { return () }
@@ -155,7 +155,7 @@ extension GenericClass where Y == () {
155155

156156
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlEyxyt_tcig : $@convention(method) <X, Y where Y == ()> (@guaranteed GenericClass<X, ()>) -> @out X
157157
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlEyxyt_tcis : $@convention(method) <X, Y where Y == ()> (@in X, @guaranteed GenericClass<X, ()>) -> ()
158-
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlEyxyt_tcimytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
158+
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlEyxyt_tcimytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
159159
// CHECK-LABEL: sil [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlEyxyt_tcim : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @guaranteed GenericClass<X, ()>) -> (Builtin.RawPointer, Optional<Builtin.RawPointer>)
160160
public subscript(_: Y) -> X {
161161
get { while true {} }
@@ -164,7 +164,7 @@ extension GenericClass where Y == () {
164164

165165
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlEyyxcig : $@convention(method) <X, Y where Y == ()> (@in X, @guaranteed GenericClass<X, ()>) -> ()
166166
// CHECK-LABEL: sil @$S22constrained_extensions12GenericClassCAAytRs_rlEyyxcis : $@convention(method) <X, Y where Y == ()> (@in X, @guaranteed GenericClass<X, ()>) -> ()
167-
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlEyyxcimytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
167+
// CHECK-LABEL: sil shared [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlEyyxcimytfU_ : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed GenericClass<X, ()>, @thick GenericClass<X, ()>.Type) -> ()
168168
// CHECK-LABEL: sil [transparent] [serialized] @$S22constrained_extensions12GenericClassCAAytRs_rlEyyxcim : $@convention(method) <X, Y where Y == ()> (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in X, @guaranteed GenericClass<X, ()>) -> (Builtin.RawPointer, Optional<Builtin.RawPointer>)
169169
public subscript(_: X) -> Y {
170170
get { while true {} }

test/SILGen/lifetime.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ func logical_lvalue_lifetime(_ r: RefWithProp, _ i: Int, _ v: Val) {
389389
// CHECK: [[ADDR:%.*]] = pointer_to_address [[PTR]]
390390
// CHECK: [[MARKED_ADDR:%.*]] = mark_dependence [[ADDR]] : $*Aleph on [[R2]]
391391
// CHECK: {{.*}}([[CALLBACK_ADDR:%.*]] :
392-
// CHECK: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout RefWithProp, @thick RefWithProp.Type) -> ()
392+
// CHECK: [[CALLBACK:%.*]] = pointer_to_thin_function [[CALLBACK_ADDR]] : $Builtin.RawPointer to $@convention(method) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @in_guaranteed RefWithProp, @thick RefWithProp.Type) -> ()
393393
// CHECK: [[TEMP:%.*]] = alloc_stack $RefWithProp
394394
// CHECK: store [[R2]] to [init] [[TEMP]]
395395
// CHECK: apply [[CALLBACK]]({{.*}}, [[STORAGE]], [[TEMP]], {{%.*}})

0 commit comments

Comments
 (0)