Skip to content

Commit 8022b10

Browse files
committed
[silgen] When forwarding a tuple value, make sure that it is at +1.
In the code, we assume generally that a value that is "forwarded" into memory is forwarded at +1. This isn't enforced and when I tried to enforce it at +0, I quickly hit assertion failures. So rather than fix the myriad places, I am using ensurePlusOne to handle such cases without needing to change a bunch of the code. rdar://34222540
1 parent 0614c78 commit 8022b10

File tree

2 files changed

+99
-6
lines changed

2 files changed

+99
-6
lines changed

lib/SILGen/RValue.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ class ImplodeLoadableTupleValue
189189
case ImplodeKind::Unmanaged:
190190
return v.getUnmanagedValue();
191191
case ImplodeKind::Forward:
192-
return v.forward(SGF);
192+
return v.ensurePlusOne(SGF, l).forward(SGF);
193193
case ImplodeKind::Copy:
194194
return v.copyUnmanaged(SGF, l).forward(SGF);
195195
}
@@ -242,7 +242,10 @@ class ImplodeAddressOnlyTuple
242242
llvm_unreachable("address-only types always managed!");
243243

244244
case ImplodeKind::Forward:
245-
v.forwardInto(SGF, l, address);
245+
// If a value is forwarded into, we require the value to be at +1. If the
246+
// the value is already at +1, we just forward. Otherwise, we perform the
247+
// copy.
248+
v.ensurePlusOne(SGF, l).forwardInto(SGF, l, address);
246249
break;
247250

248251
case ImplodeKind::Copy:
@@ -510,10 +513,9 @@ void RValue::addElement(SILGenFunction &SGF, ManagedValue element,
510513

511514
SILValue RValue::forwardAsSingleValue(SILGenFunction &SGF, SILLocation l) && {
512515
assert(isComplete() && "rvalue is not complete");
513-
assert(isPlusOne(SGF) && "Can not forward borrowed RValues");
516+
// *NOTE* Inside implodeTupleValues, we copy our values if they are not at +1.
514517
SILValue result
515518
= implodeTupleValues<ImplodeKind::Forward>(values, SGF, type, l);
516-
517519
makeUsed();
518520
return result;
519521
}
@@ -522,8 +524,9 @@ SILValue RValue::forwardAsSingleStorageValue(SILGenFunction &SGF,
522524
SILType storageType,
523525
SILLocation l) && {
524526
assert(isComplete() && "rvalue is not complete");
525-
assert(isPlusOne(SGF) && "Can not forward borrowed RValues");
526-
SILValue result = std::move(*this).forwardAsSingleValue(SGF, l);
527+
// Conversions must always be done at +1.
528+
SILValue result =
529+
std::move(*this).ensurePlusOne(SGF, l).forwardAsSingleValue(SGF, l);
527530
return SGF.emitConversionFromSemanticValue(l, result, storageType);
528531
}
529532

test/SILGen/guaranteed_normal_args.swift

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,33 @@ precedencegroup AssignmentPrecedence {
1111
assignment: true
1212
}
1313

14+
public protocol ExpressibleByNilLiteral {
15+
init(nilLiteral: ())
16+
}
17+
18+
protocol IteratorProtocol {
19+
associatedtype Element
20+
mutating func next() -> Element?
21+
}
22+
23+
protocol Sequence {
24+
associatedtype Element
25+
associatedtype Iterator : IteratorProtocol where Iterator.Element == Element
26+
27+
func makeIterator() -> Iterator
28+
}
29+
1430
enum Optional<T> {
1531
case none
1632
case some(T)
1733
}
1834

35+
extension Optional : ExpressibleByNilLiteral {
36+
public init(nilLiteral: ()) {
37+
self = .none
38+
}
39+
}
40+
1941
func _diagnoseUnexpectedNilOptional(_filenameStart: Builtin.RawPointer,
2042
_filenameLength: Builtin.Word,
2143
_filenameIsASCII: Builtin.Int1,
@@ -42,6 +64,41 @@ protocol Protocol {
4264
static func useInput(_ input: Builtin.Int32, into processInput: (AssocType) -> ())
4365
}
4466

67+
struct FakeArray<Element> {
68+
// Just to make this type non-trivial
69+
var k: Klass
70+
71+
// We are only interested in this being called. We are not interested in its
72+
// implementation.
73+
mutating func append(_ t: Element) {}
74+
}
75+
76+
struct FakeDictionary<Key, Value> {
77+
}
78+
79+
struct FakeDictionaryIterator<Key, Value> {
80+
var dictionary: FakeDictionary<Key, Value>?
81+
82+
init(_ newDictionary: FakeDictionary<Key, Value>) {
83+
dictionary = newDictionary
84+
}
85+
}
86+
87+
extension FakeDictionaryIterator : IteratorProtocol {
88+
public typealias Element = (Key, Value)
89+
public mutating func next() -> Element? {
90+
return .none
91+
}
92+
}
93+
94+
extension FakeDictionary : Sequence {
95+
public typealias Element = (Key, Value)
96+
public typealias Iterator = FakeDictionaryIterator<Key, Value>
97+
public func makeIterator() -> FakeDictionaryIterator<Key, Value> {
98+
return FakeDictionaryIterator(self)
99+
}
100+
}
101+
45102
///////////
46103
// Tests //
47104
///////////
@@ -108,3 +165,36 @@ struct ReabstractionThunkTest : Protocol {
108165
}
109166
}
110167

168+
// Make sure that we provide a cleanup to x properly before we pass it to
169+
// result.
170+
extension FakeDictionary {
171+
// CHECK-LABEL: sil hidden @$Ss14FakeDictionaryV20makeSureToCopyTuplesyyF : $@convention(method) <Key, Value> (FakeDictionary<Key, Value>) -> () {
172+
// CHECK: [[X:%.*]] = alloc_stack $(Key, Value), let, name "x"
173+
// CHECK: [[INDUCTION_VAR:%.*]] = unchecked_take_enum_data_addr {{%.*}} : $*Optional<(Key, Value)>, #Optional.some!enumelt.1
174+
// CHECK: [[INDUCTION_VAR_0:%.*]] = tuple_element_addr [[INDUCTION_VAR]] : $*(Key, Value), 0
175+
// CHECK: [[INDUCTION_VAR_1:%.*]] = tuple_element_addr [[INDUCTION_VAR]] : $*(Key, Value), 1
176+
// CHECK: [[X_0:%.*]] = tuple_element_addr [[X]] : $*(Key, Value), 0
177+
// CHECK: [[X_1:%.*]] = tuple_element_addr [[X]] : $*(Key, Value), 1
178+
// CHECK: copy_addr [take] [[INDUCTION_VAR_0]] to [initialization] [[X_0]]
179+
// CHECK: copy_addr [take] [[INDUCTION_VAR_1]] to [initialization] [[X_1]]
180+
// CHECK: [[X_0:%.*]] = tuple_element_addr [[X]] : $*(Key, Value), 0
181+
// CHECK: [[X_1:%.*]] = tuple_element_addr [[X]] : $*(Key, Value), 1
182+
// CHECK: [[TMP_X:%.*]] = alloc_stack $(Key, Value)
183+
// CHECK: [[TMP_X_0:%.*]] = tuple_element_addr [[TMP_X]] : $*(Key, Value), 0
184+
// CHECK: [[TMP_0:%.*]] = alloc_stack $Key
185+
// CHECK: copy_addr [[X_0]] to [initialization] [[TMP_0]]
186+
// CHECK: copy_addr [take] [[TMP_0]] to [initialization] [[TMP_X_0]]
187+
// CHECK: [[TMP_X_1:%.*]] = tuple_element_addr [[TMP_X]] : $*(Key, Value), 1
188+
// CHECK: [[TMP_1:%.*]] = alloc_stack $Value
189+
// CHECK: copy_addr [[X_1]] to [initialization] [[TMP_1]]
190+
// CHECK: copy_addr [take] [[TMP_1]] to [initialization] [[TMP_X_1]]
191+
// CHECK: [[FUNC:%.*]] = function_ref @$Ss9FakeArrayV6appendyyxF : $@convention(method) <τ_0_0> (@in_guaranteed τ_0_0, @inout FakeArray<τ_0_0>) -> ()
192+
// CHECK: apply [[FUNC]]<(Key, Value)>([[TMP_X]],
193+
// CHECK: } // end sil function '$Ss14FakeDictionaryV20makeSureToCopyTuplesyyF'
194+
func makeSureToCopyTuples() {
195+
var result = FakeArray<Element>(k: Klass())
196+
for x in self {
197+
result.append(x)
198+
}
199+
}
200+
}

0 commit comments

Comments
 (0)