Skip to content

Commit dab6891

Browse files
committed
SILGen: Borrow the base of accessor LValueComponents.
The same base value is necessary to invoke other accessors as part of the same access, but we would end up consuming it as part of materializing the base value for calls into nonmutating setters. Fixes SR-8990 | rdar://problem/45274900.
1 parent e3a75de commit dab6891

15 files changed

+127
-47
lines changed

lib/SILGen/SILGenLValue.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,9 +1145,14 @@ namespace {
11451145
ManagedValue base, SILDeclRef accessor) &&
11461146
{
11471147
AccessorArgs result;
1148-
if (base)
1149-
result.base = SGF.prepareAccessorBaseArg(loc, base, BaseFormalType,
1150-
accessor);
1148+
if (base) {
1149+
// Borrow the base, because we may need it again to invoke other
1150+
// accessors.
1151+
result.base = SGF.prepareAccessorBaseArg(loc,
1152+
base.formalAccessBorrow(SGF, loc),
1153+
BaseFormalType,
1154+
accessor);
1155+
}
11511156

11521157
if (!Indices.isNull())
11531158
result.Indices = std::move(Indices);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %target-run-simple-swift | %FileCheck %s
2+
// REQUIRES: executable_test
3+
4+
// SR-8990
5+
6+
// CHECK: A
7+
// CHECK: B
8+
// CHECK: C
9+
10+
protocol SomeProtocol { }
11+
class SomeClass: SomeProtocol { deinit { print("C") } }
12+
struct SomeStruct { var x, y: Int }
13+
14+
extension SomeProtocol {
15+
var someProperty: SomeStruct {
16+
nonmutating set {
17+
print("B")
18+
}
19+
get {
20+
print("A")
21+
return SomeStruct(x: 1, y: 2)
22+
}
23+
}
24+
}
25+
26+
SomeClass().someProperty.x = 32

test/SILGen/boxed_existentials.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,10 @@ func test_property_of_lvalue(_ x: Error) -> String {
9191
// CHECK: [[VALUE:%.*]] = open_existential_box [[VALUE_BOX]] : $Error to $*[[VALUE_TYPE:@opened\(.*\) Error]]
9292
// CHECK: [[COPY:%.*]] = alloc_stack $[[VALUE_TYPE]]
9393
// CHECK: copy_addr [[VALUE]] to [initialization] [[COPY]]
94+
// CHECK: [[BORROW:%.*]] = alloc_stack $[[VALUE_TYPE]]
95+
// CHECK: copy_addr [[COPY]] to [initialization] [[BORROW]]
9496
// CHECK: [[METHOD:%.*]] = witness_method $[[VALUE_TYPE]], #Error._domain!getter.1
95-
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]<[[VALUE_TYPE]]>([[COPY]])
97+
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]<[[VALUE_TYPE]]>([[BORROW]])
9698
// CHECK: destroy_addr [[COPY]]
9799
// CHECK: dealloc_stack [[COPY]]
98100
// CHECK: destroy_value [[VALUE_BOX]]

test/SILGen/class_bound_protocols.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,20 +176,21 @@ func takesInheritsMutatingMethod(x: inout InheritsMutatingMethod,
176176
// CHECK-NEXT: [[X_PAYLOAD:%.*]] = open_existential_ref [[X_VALUE]] : $InheritsMutatingMethod to $@opened("{{.*}}") InheritsMutatingMethod
177177
// CHECK-NEXT: [[TEMPORARY:%.*]] = alloc_stack $@opened("{{.*}}") InheritsMutatingMethod
178178
// CHECK-NEXT: store [[X_PAYLOAD]] to [init] [[TEMPORARY]] : $*@opened("{{.*}}") InheritsMutatingMethod
179-
// CHECK-NEXT: [[X_PAYLOAD_RELOADED:%.*]] = load [take] [[TEMPORARY]]
179+
// CHECK-NEXT: [[X_PAYLOAD_RELOADED:%.*]] = load_borrow [[TEMPORARY]]
180180
//
181181
// ** *NOTE* This extra copy is here since RValue invariants enforce that all
182182
// ** loadable objects are actually loaded. So we form the RValue and
183183
// ** load... only to then need to store the value back in a stack location to
184184
// ** pass to an in_guaranteed method. PredictableMemOpts is able to handle this
185185
// ** type of temporary codegen successfully.
186186
// CHECK-NEXT: [[TEMPORARY_2:%.*]] = alloc_stack $@opened("{{.*}}") InheritsMutatingMethod
187-
// CHECK-NEXT: store [[X_PAYLOAD_RELOADED:%.*]] to [init] [[TEMPORARY_2]]
187+
// CHECK-NEXT: store_borrow [[X_PAYLOAD_RELOADED:%.*]] to [[TEMPORARY_2]]
188188
//
189189
// CHECK-NEXT: [[METHOD:%.*]] = witness_method $@opened("{{.*}}") InheritsMutatingMethod, #HasMutatingMethod.mutatingCounter!getter.1 : <Self where Self : HasMutatingMethod> (Self) -> () -> Value, [[X_PAYLOAD]] : $@opened("{{.*}}") InheritsMutatingMethod : $@convention(witness_method: HasMutatingMethod) <τ_0_0 where τ_0_0 : HasMutatingMethod> (@in_guaranteed τ_0_0) -> Value
190190
// CHECK-NEXT: [[RESULT_VALUE:%.*]] = apply [[METHOD]]<@opened("{{.*}}") InheritsMutatingMethod>([[TEMPORARY_2]]) : $@convention(witness_method: HasMutatingMethod) <τ_0_0 where τ_0_0 : HasMutatingMethod> (@in_guaranteed τ_0_0) -> Value
191-
// CHECK-NEXT: destroy_addr [[TEMPORARY_2]] : $*@opened("{{.*}}") InheritsMutatingMethod
192191
// CHECK-NEXT: dealloc_stack [[TEMPORARY_2]]
192+
// CHECK-NEXT: end_borrow
193+
// CHECK-NEXT: destroy_addr
193194
// CHECK-NEXT: end_access [[X_ADDR]] : $*InheritsMutatingMethod
194195
// CHECK-NEXT: assign [[RESULT_VALUE]] to [[RESULT]] : $*Value
195196
// CHECK-NEXT: dealloc_stack [[TEMPORARY]] : $*@opened("{{.*}}") InheritsMutatingMethod

test/SILGen/dynamic.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -453,13 +453,13 @@ public class Sub : Base {
453453
// CHECK-LABEL: sil private [transparent] @$s7dynamic3SubC1xSbvgSbyKXKfu_ : $@convention(thin) (@guaranteed Sub) -> (Bool, @error Error) {
454454
// CHECK: bb0([[VALUE:%.*]] : @guaranteed $Sub):
455455
// CHECK: [[VALUE_COPY:%.*]] = copy_value [[VALUE]]
456-
// CHECK: [[CASTED_VALUE_COPY:%.*]] = upcast [[VALUE_COPY]]
457-
// CHECK: [[BORROWED_CASTED_VALUE_COPY:%.*]] = begin_borrow [[CASTED_VALUE_COPY]]
458-
// CHECK: [[DOWNCAST_FOR_SUPERMETHOD:%.*]] = unchecked_ref_cast [[BORROWED_CASTED_VALUE_COPY]]
456+
// CHECK: [[CAST_VALUE_COPY:%.*]] = upcast [[VALUE_COPY]]
457+
// CHECK: [[BORROWED_CAST_VALUE_COPY:%.*]] = begin_borrow [[CAST_VALUE_COPY]]
458+
// CHECK: [[DOWNCAST_FOR_SUPERMETHOD:%.*]] = unchecked_ref_cast [[BORROWED_CAST_VALUE_COPY]]
459459
// CHECK: [[SUPER:%.*]] = objc_super_method [[DOWNCAST_FOR_SUPERMETHOD]] : $Sub, #Base.x!getter.1.foreign : (Base) -> () -> Bool, $@convention(objc_method) (Base) -> ObjCBool
460-
// CHECK: end_borrow [[BORROWED_CASTED_VALUE_COPY]]
461-
// CHECK: = apply [[SUPER]]([[CASTED_VALUE_COPY]])
462-
// CHECK: destroy_value [[CASTED_VALUE_COPY]]
460+
// CHECK: = apply [[SUPER]]([[BORROWED_CAST_VALUE_COPY]])
461+
// CHECK: end_borrow [[BORROWED_CAST_VALUE_COPY]]
462+
// CHECK: destroy_value [[CAST_VALUE_COPY]]
463463
// CHECK: } // end sil function '$s7dynamic3SubC1xSbvgSbyKXKfu_'
464464
override var x: Bool { return false || super.x }
465465
}

test/SILGen/generic_property_base_lifetime.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ func getIntPropGeneric<T: ProtocolB>(_ a: T) -> Int {
8383
// CHECK: bb0([[ARG:%.*]] : @guaranteed $ProtocolO):
8484
// CHECK: [[PROJECTION:%.*]] = open_existential_ref [[ARG]]
8585
// CHECK: [[PROJECTION_COPY:%.*]] = copy_value [[PROJECTION]]
86-
// CHECK: [[METHOD:%.*]] = objc_method [[PROJECTION_COPY]] : $@opened({{.*}}) ProtocolO, #ProtocolO.intProp!getter.1.foreign : {{.*}}
87-
// CHECK: apply [[METHOD]]<@opened{{.*}}>([[PROJECTION_COPY]])
86+
// CHECK: [[PROJECTION_BORROW:%.*]] = begin_borrow [[PROJECTION_COPY]]
87+
// CHECK: [[METHOD:%.*]] = objc_method [[PROJECTION_BORROW]] : $@opened({{.*}}) ProtocolO, #ProtocolO.intProp!getter.1.foreign : {{.*}}
88+
// CHECK: apply [[METHOD]]<@opened{{.*}}>([[PROJECTION_BORROW]])
8889
// CHECK: destroy_value [[PROJECTION_COPY]]
8990
// CHECK: } // end sil function '$s30generic_property_base_lifetime21getIntPropExistentialySiAA9ProtocolO_pF'
9091
func getIntPropExistential(_ a: ProtocolO) -> Int {

test/SILGen/lifetime.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
// RUN: %target-swift-emit-silgen -module-name lifetime -Xllvm -sil-full-demangle -parse-as-library -primary-file %s | %FileCheck %s
32

43
struct Buh<T> {
@@ -367,8 +366,9 @@ func logical_lvalue_lifetime(_ r: RefWithProp, _ i: Int, _ v: Val) {
367366
r.aleph_prop.b = v
368367
// CHECK: [[READ:%.*]] = begin_access [read] [unknown] [[PR]]
369368
// CHECK: [[R2:%[0-9]+]] = load [copy] [[READ]]
370-
// CHECK: [[MODIFY:%[0-9]+]] = class_method [[R2]] : $RefWithProp, #RefWithProp.aleph_prop!modify.1 :
371-
// CHECK: ([[ADDR:%.*]], [[TOKEN:%.*]]) = begin_apply [[MODIFY]]([[R2]])
369+
// CHECK: [[R2BORROW:%[0-9]+]] = begin_borrow [[R2]]
370+
// CHECK: [[MODIFY:%[0-9]+]] = class_method [[R2BORROW]] : $RefWithProp, #RefWithProp.aleph_prop!modify.1 :
371+
// CHECK: ([[ADDR:%.*]], [[TOKEN:%.*]]) = begin_apply [[MODIFY]]([[R2BORROW]])
372372
// CHECK: end_apply [[TOKEN]]
373373
}
374374

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %target-swift-emit-silgen -enable-sil-ownership -verify %s
2+
3+
// SR-8990
4+
5+
protocol SomeProtocol { }
6+
class SomeClass: SomeProtocol { }
7+
struct SomeStruct { var x, y: Int }
8+
9+
extension SomeProtocol {
10+
var someProperty: SomeStruct {
11+
nonmutating set { }
12+
get { return SomeStruct(x: 1, y: 2) }
13+
}
14+
}
15+
16+
func f(i: Int) {
17+
SomeClass().someProperty.x = i
18+
}
19+
20+

test/SILGen/objc_extensions.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ extension Sub {
3434
// CHECK: [[BORROWED_SELF_COPY_CAST:%.*]] = begin_borrow [[SELF_COPY_CAST]]
3535
// CHECK: [[CAST_BACK:%.*]] = unchecked_ref_cast [[BORROWED_SELF_COPY_CAST]] : $Base to $Sub
3636
// CHECK: [[SUPER_METHOD:%.*]] = objc_super_method [[CAST_BACK]] : $Sub, #Base.prop!getter.1.foreign
37-
// CHECK: end_borrow [[BORROWED_SELF_COPY_CAST]]
38-
// CHECK: [[RESULT:%.*]] = apply [[SUPER_METHOD]]([[SELF_COPY_CAST]])
37+
// CHECK: [[RESULT:%.*]] = apply [[SUPER_METHOD]]([[BORROWED_SELF_COPY_CAST]])
3938
// CHECK: bb3(
39+
// CHECK: end_borrow [[BORROWED_SELF_COPY_CAST]]
4040
// CHECK: destroy_value [[SELF_COPY_CAST]]
4141
// CHECK: } // end sil function '$s15objc_extensions3SubC4propSSSgvg'
4242

@@ -68,8 +68,7 @@ extension Sub {
6868
// CHECK: [[BORROWED_UPCAST_SELF_COPY:%.*]] = begin_borrow [[UPCAST_SELF_COPY]]
6969
// CHECK: [[CAST_BACK:%.*]] = unchecked_ref_cast [[BORROWED_UPCAST_SELF_COPY]] : $Base to $Sub
7070
// CHECK: [[GET_SUPER_METHOD:%.*]] = objc_super_method [[CAST_BACK]] : $Sub, #Base.prop!getter.1.foreign : (Base) -> () -> String?, $@convention(objc_method) (Base) -> @autoreleased Optional<NSString>
71-
// CHECK: end_borrow [[BORROWED_UPCAST_SELF_COPY]]
72-
// CHECK: [[OLD_NSSTRING:%.*]] = apply [[GET_SUPER_METHOD]]([[UPCAST_SELF_COPY]])
71+
// CHECK: [[OLD_NSSTRING:%.*]] = apply [[GET_SUPER_METHOD]]([[BORROWED_UPCAST_SELF_COPY]])
7372

7473
// CHECK: bb3([[OLD_NSSTRING_BRIDGED:%.*]] : @owned $Optional<String>):
7574
// This next line is completely not needed. But we are emitting it now.

test/SILGen/objc_ownership_conventions.swift

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,9 @@ func test10(_ g: Gizmo) -> AnyClass {
127127
// CHECK: bb0([[G:%[0-9]+]] : @guaranteed $Gizmo):
128128
// CHECK: [[G_COPY:%.*]] = copy_value [[G]]
129129
// CHECK-NEXT: [[NS_G_COPY:%[0-9]+]] = upcast [[G_COPY]] : $Gizmo to $NSObject
130-
// CHECK-NEXT: [[GETTER:%[0-9]+]] = objc_method [[NS_G_COPY]] : $NSObject, #NSObject.classProp!getter.1.foreign : (NSObject) -> () -> AnyObject.Type?, $@convention(objc_method) (NSObject) -> Optional<@objc_metatype AnyObject.Type>
131-
// CHECK-NEXT: [[OPT_OBJC:%.*]] = apply [[GETTER]]([[NS_G_COPY]]) : $@convention(objc_method) (NSObject) -> Optional<@objc_metatype AnyObject.Type>
130+
// CHECK-NEXT: [[NS_G_BORROW:%.*]] = begin_borrow [[NS_G_COPY]]
131+
// CHECK-NEXT: [[GETTER:%[0-9]+]] = objc_method [[NS_G_BORROW]] : $NSObject, #NSObject.classProp!getter.1.foreign : (NSObject) -> () -> AnyObject.Type?, $@convention(objc_method) (NSObject) -> Optional<@objc_metatype AnyObject.Type>
132+
// CHECK-NEXT: [[OPT_OBJC:%.*]] = apply [[GETTER]]([[NS_G_BORROW]]) : $@convention(objc_method) (NSObject) -> Optional<@objc_metatype AnyObject.Type>
132133
// CHECK-NEXT: switch_enum [[OPT_OBJC]] : $Optional<{{.*}}>, case #Optional.some!enumelt.1: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[NONE_BB:bb[0-9]+]]
133134
//
134135
// CHECK: [[SOME_BB]]([[OBJC:%.*]] : @trivial $@objc_metatype AnyObject.Type):
@@ -147,8 +148,9 @@ func test11(_ g: Gizmo) -> AnyClass {
147148
// CHECK: bb0([[G:%[0-9]+]] : @guaranteed $Gizmo):
148149
// CHECK: [[G_COPY:%.*]] = copy_value [[G]]
149150
// CHECK: [[NS_G_COPY:%[0-9]+]] = upcast [[G_COPY:%[0-9]+]] : $Gizmo to $NSObject
150-
// CHECK-NEXT: [[GETTER:%[0-9]+]] = objc_method [[NS_G_COPY]] : $NSObject, #NSObject.qualifiedClassProp!getter.1.foreign : (NSObject) -> () -> NSAnsing.Type?, $@convention(objc_method) (NSObject) -> Optional<@objc_metatype NSAnsing.Type>
151-
// CHECK-NEXT: [[OPT_OBJC:%.*]] = apply [[GETTER]]([[NS_G_COPY]]) : $@convention(objc_method) (NSObject) -> Optional<@objc_metatype NSAnsing.Type>
151+
// CHECK-NEXT: [[NS_G_BORROW:%.*]] = begin_borrow [[NS_G_COPY]]
152+
// CHECK-NEXT: [[GETTER:%[0-9]+]] = objc_method [[NS_G_BORROW]] : $NSObject, #NSObject.qualifiedClassProp!getter.1.foreign : (NSObject) -> () -> NSAnsing.Type?, $@convention(objc_method) (NSObject) -> Optional<@objc_metatype NSAnsing.Type>
153+
// CHECK-NEXT: [[OPT_OBJC:%.*]] = apply [[GETTER]]([[NS_G_BORROW]]) : $@convention(objc_method) (NSObject) -> Optional<@objc_metatype NSAnsing.Type>
152154
// CHECK-NEXT: switch_enum [[OPT_OBJC]] : $Optional<{{.*}}>, case #Optional.some!enumelt.1: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[NONE_BB:bb[0-9]+]]
153155
//
154156
// CHECK: [[SOME_BB]]([[OBJC:%.*]] : @trivial $@objc_metatype NSAnsing.Type):

0 commit comments

Comments
 (0)