Skip to content

Commit bfce117

Browse files
committed
ManualOwnership: diagnose copies of generics
copy_addr often is `Releasing`, which is `RefCounting | Deallocating`
1 parent b3b9d28 commit bfce117

File tree

2 files changed

+53
-5
lines changed

2 files changed

+53
-5
lines changed

lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,15 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
376376
LocWithParent loc(inst->getLoc().getSourceLoc(), parentLoc);
377377

378378
if (perfConstr == PerformanceConstraints::ManualOwnership) {
379-
if (impact == RuntimeEffect::RefCounting) {
379+
if (impact & RuntimeEffect::RefCounting) {
380380
bool shouldDiagnose = false;
381381
switch (inst->getKind()) {
382+
case SILInstructionKind::DestroyAddrInst:
383+
case SILInstructionKind::DestroyValueInst:
384+
break; // These modify reference counts, but aren't copies.
382385
case SILInstructionKind::ExplicitCopyAddrInst:
383386
case SILInstructionKind::ExplicitCopyValueInst:
384-
break;
387+
break; // Explicitly acknowledged copies are OK.
385388
case SILInstructionKind::LoadInst: {
386389
// FIXME: we don't have an `explicit_load` and transparent functions can
387390
// end up bringing in a `load [copy]`. A better approach is needed to

test/SIL/manual_ownership.swift

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ public class Triangle {
3434
borrowing func borrowing() {}
3535
}
3636

37+
// MARK: utilities
38+
39+
func eat(_ t: consuming Triangle) {}
40+
func use(_ t: borrowing Triangle) {}
41+
42+
func consume_generic<T>(_ t: consuming T) {}
43+
func borrow_generic<T>(_ t: borrowing T) {}
44+
3745
/// MARK: return statements
3846

3947
@_manualOwnership
@@ -107,12 +115,9 @@ func basic_methods_consuming(_ t1: Triangle) {
107115
@_manualOwnership
108116
func basic_methods_consuming_fixed(_ t1: Triangle) {
109117
let t2 = Triangle()
110-
var t3 = Triangle()
111-
t3 = Triangle()
112118

113119
(copy t1).consuming()
114120
(copy t2).consuming() // FIXME: why is this not propagated?
115-
(copy t3).consuming()
116121
}
117122

118123
func consumingFunc(_ t0: consuming Triangle) {}
@@ -190,6 +195,8 @@ public func basic_loop_nontrivial_values_fixed(_ t: Triangle, _ xs: [Triangle])
190195
/// MARK: Globals
191196
let ref_result = [5, 13, 29]
192197

198+
// FIXME: if we had a borrow operator, we could allow people to elide these simple copies that
199+
// are present to avoid exclusivity issues. We'd need to start generating read coroutines.
193200
@_manualOwnership
194201
func access_global_1() -> Int {
195202
return ref_result[2] // expected-error {{explicit 'copy' required here}}
@@ -199,3 +206,41 @@ func access_global_1_fixed() -> Int {
199206
return (copy ref_result)[2]
200207
}
201208

209+
/// MARK: closures
210+
211+
// FIXME: (1) Closure capture lists need to support the short-hand [copy t] and produce explicit copies.
212+
// We also need a better error message for when this is missed for closure captures.
213+
// (2) Escaping closures need to be recursively checked by the PerformanceDiagnostics.
214+
// We might just need to widen the propagation of [manual_ownership]?
215+
@_manualOwnership
216+
func testClosures() -> () -> Triangle {
217+
let t = Triangle()
218+
return { [t = copy t] in return t } // expected-error {{explicit 'copy' required here}}
219+
}
220+
221+
@_manualOwnership
222+
func testClosures_noescape() -> Triangle {
223+
let t = Triangle()
224+
let f = { [t = copy t] in
225+
eat(t) // FIXME: missing required copies
226+
eat(t)
227+
return t
228+
}
229+
return f()
230+
}
231+
232+
/// MARK: generics
233+
234+
@_manualOwnership
235+
func copy_generic<T>(_ t: T) {
236+
consume_generic(t) // expected-error {{explicit 'copy' required here}}
237+
borrow_generic(t)
238+
consume_generic(t) // expected-error {{explicit 'copy' required here}}
239+
}
240+
241+
@_manualOwnership
242+
func copy_generic_fixed<T>(_ t: T) {
243+
consume_generic(copy t)
244+
borrow_generic(t)
245+
consume_generic(copy t)
246+
}

0 commit comments

Comments
 (0)