From da4f27e31bd94d6f04ec1dd4fc21a0a5356faff6 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 11:50:39 -0700 Subject: [PATCH 1/7] ManualOwnership: adjust CopyExpr emission We were getting an address-based emission for loadable types, if the result of a LoadExpr was explicit-copied. This emission had extra hidden copies that could not be silenced. --- lib/SILGen/SILGenExpr.cpp | 13 ++++++++++++ test/SIL/manual_ownership.swift | 34 ++++++++++++++++++++++++------ test/SILGen/manual_ownership.swift | 28 ++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 test/SILGen/manual_ownership.swift diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 7b04c5f022cf8..49b9be3fd596d 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -7279,6 +7279,19 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) { if (auto *li = dyn_cast(subExpr)) { FormalEvaluationScope writeback(SGF); + + // If we're relying on ManualOwnership for explicit-copies enforcement, + // avoid doing address-based emission for loadable types. + if (subType.isLoadableOrOpaque(SGF.F) && + SGF.F.getPerfConstraints() == PerformanceConstraints::ManualOwnership) { + // Interpret this 'load' as a borrow + copy instead. + LValue lv = + SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedObjectRead); + auto value = SGF.emitBorrowedLValue(li, std::move(lv)); + ManagedValue copy = SGF.B.createExplicitCopyValue(E, value); + return RValue(SGF, {copy}, subType.getASTType()); + } + LValue lv = SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedAddressRead); auto address = SGF.emitAddressOfLValue(subExpr, std::move(lv)); diff --git a/test/SIL/manual_ownership.swift b/test/SIL/manual_ownership.swift index 8fa9d90508412..da4943d83a6f8 100644 --- a/test/SIL/manual_ownership.swift +++ b/test/SIL/manual_ownership.swift @@ -172,9 +172,8 @@ public func basic_loop_trivial_values_fixed(_ t: Triangle, _ xs: [Triangle]) { // FIXME: the only reason for so many copies below is because // `Triangle.nontrivial` only exposes get/set rather than read/modify by default // -// We should figure out when the coroutine accessors are generated, and ensure -// that when it is available, it is used without copying the result, rather than -// calling the get/set +// There's complexity in auto-generating a read accessor for classes, but if it's provided +// we could then allow someone to elide the copy with a `borrow x` expression. @_manualOwnership public func basic_loop_nontrivial_values(_ t: Triangle, _ xs: [Triangle]) { @@ -185,14 +184,35 @@ public func basic_loop_nontrivial_values(_ t: Triangle, _ xs: [Triangle]) { t.nontrivial.a = p // expected-error {{accessing 't.nontrivial' produces a copy of it}} } -// FIXME: there should be no copies required in the below, other than what's already written. @_manualOwnership public func basic_loop_nontrivial_values_fixed(_ t: Triangle, _ xs: [Triangle]) { - var p: Pair = (copy t.nontrivial).a // expected-error {{accessing 't.nontrivial' produces a copy of it}} + var p: Pair = (copy t.nontrivial).a for x in copy xs { - p = p.midpoint((copy x.nontrivial).a) // expected-error {{accessing 'x.nontrivial' produces a copy of it}} + p = p.midpoint((copy x.nontrivial).a) } - (copy t.nontrivial).a = p // expected-error {{accessing 't.nontrivial' produces a copy of it}} + (copy t.nontrivial).a = p +} + +@_manualOwnership +public func basic_loop_nontrivial_values_reduced_copies(_ t: Triangle, _ xs: [Triangle]) { + // FIXME: confusing variable names are chosen + let nt = t.nontrivial // expected-error {{accessing 'nt' produces a copy of it}} + var p: Pair = nt.a + for x in copy xs { + let xnt = x.nontrivial // expected-error {{accessing 'xnt' produces a copy of it}} + p = p.midpoint(xnt.a) + } + nt.a = p +} +@_manualOwnership +public func basic_loop_nontrivial_values_reduced_copies_fixed(_ t: Triangle, _ xs: [Triangle]) { + let nt = copy t.nontrivial + var p: Pair = nt.a + for x in copy xs { + let xnt = copy x.nontrivial + p = p.midpoint(xnt.a) + } + nt.a = p } diff --git a/test/SILGen/manual_ownership.swift b/test/SILGen/manual_ownership.swift new file mode 100644 index 0000000000000..c9298a5233734 --- /dev/null +++ b/test/SILGen/manual_ownership.swift @@ -0,0 +1,28 @@ +// RUN: %target-swift-frontend %s -emit-silgen -verify \ +// RUN: -enable-experimental-feature ManualOwnership -o %t.silgen + +// RUN: %FileCheck %s --input-file %t.silgen + +// REQUIRES: swift_feature_ManualOwnership + +class ShapeClass {} +class TriangleClass { + var shape = ShapeClass() +} + +// CHECK-LABEL: sil {{.*}} @basic_access_of_loadable +// CHECK: bb0(%0 : @guaranteed $TriangleClass): +// CHECK-NEXT: debug_value %0 +// CHECK-NEXT: [[M:%.*]] = class_method %0, #TriangleClass.shape!getter +// CHECK-NEXT: [[SHAPE:%.*]] = apply [[M]](%0) +// CHECK-NEXT: [[B:%.*]] = begin_borrow [[SHAPE]] +// CHECK-NEXT: [[COPY:%.*]] = explicit_copy_value [[B]] +// CHECK-NEXT: end_borrow [[B]] +// CHECK-NEXT: destroy_value [[SHAPE]] +// CHECK-NEXT: return [[COPY]] +// CHECK-NEXT: } // end sil function 'basic_access_of_loadable' +@_manualOwnership +@_silgen_name("basic_access_of_loadable") +func basic_access_of_loadable(_ t: TriangleClass) -> ShapeClass { + return copy t.shape +} From 0ec0292893c79c67313073766c9d23661ea91e41 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 13:10:16 -0700 Subject: [PATCH 2/7] ManualOwnership: disallow @_noImplicitCopy ManualOwnership effectively subsumes @_noImplicitCopy, by diagnosing all implicit copies in a function. --- include/swift/AST/DiagnosticsSema.def | 2 ++ lib/Sema/TypeCheckAttr.cpp | 5 +++++ test/Sema/manualownership_attr.swift | 14 ++++++++++++++ 3 files changed, 21 insertions(+) create mode 100644 test/Sema/manualownership_attr.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 0b1dfd6ba8da1..03e2bc6552a07 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2148,6 +2148,8 @@ ERROR(attr_static_exclusive_no_setters,none, // @_manualOwnership ERROR(attr_manual_ownership_experimental,none, "'@_manualOwnership' requires '-enable-experimental-feature ManualOwnership'", ()) +ERROR(attr_manual_ownership_noimplicitcopy,none, + "'@_noImplicitCopy' cannot be used with ManualOwnership", ()) // @extractConstantsFromMembers ERROR(attr_extractConstantsFromMembers_experimental,none, diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 7099f26ae5e02..555e3e48b51c9 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -491,6 +491,11 @@ void AttributeChecker::visitNoImplicitCopyAttr(NoImplicitCopyAttr *attr) { return; } + // Don't allow it to be combined with ManualOwnership. + if (D->getASTContext().LangOpts.hasFeature(Feature::ManualOwnership)) { + diagnoseAndRemoveAttr(attr, diag::attr_manual_ownership_noimplicitcopy); + } + if (auto *funcDecl = dyn_cast(D)) { if (visitOwnershipAttr(attr)) return; diff --git a/test/Sema/manualownership_attr.swift b/test/Sema/manualownership_attr.swift new file mode 100644 index 0000000000000..be99aeb1ccd09 --- /dev/null +++ b/test/Sema/manualownership_attr.swift @@ -0,0 +1,14 @@ +// RUN: %target-typecheck-verify-swift \ +// RUN: -enable-experimental-feature NoImplicitCopy \ +// RUN: -enable-experimental-feature ManualOwnership + +// REQUIRES: swift_feature_ManualOwnership +// REQUIRES: swift_feature_NoImplicitCopy + +class C {} + +@_manualOwnership +func hello() -> (C, C) { + @_noImplicitCopy let x = C() // expected-error {{'@_noImplicitCopy' cannot be used with ManualOwnership}} + return (x, x) +} From da496630797cc79488e8befd674b497680670b44 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 13:15:38 -0700 Subject: [PATCH 3/7] ManualOwnership: avoid implicit SILMoveOnlyWrappedType's These wrappers are detected by the move checking passes, to support the implicit no-implicit-copy check for parameters with ownership specifiers. ManualOwnership already does this kind of checking, and the two passes can interfere with each other, such as intercepting diagnostics that are meant to be emitted by ManualOwnership. --- lib/SILGen/SILGenBuilder.cpp | 4 +++ lib/SILGen/SILGenDecl.cpp | 6 +++- lib/SILGen/SILGenPattern.cpp | 5 +++- lib/SILGen/SILGenProlog.cpp | 9 ++++-- test/SIL/manual_ownership.swift | 20 +++++++++++++ test/SILGen/manual_ownership.swift | 46 ++++++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 4 deletions(-) diff --git a/lib/SILGen/SILGenBuilder.cpp b/lib/SILGen/SILGenBuilder.cpp index fae9e1dfc528e..ac7e831c0bcc3 100644 --- a/lib/SILGen/SILGenBuilder.cpp +++ b/lib/SILGen/SILGenBuilder.cpp @@ -537,6 +537,10 @@ static ManagedValue createInputFunctionArgument( isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Borrowing; isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Consuming; } + + // ManualOwnership checks everything for implicit copies already. + if (B.hasManualOwnershipAttr()) + isNoImplicitCopy = false; } if (isNoImplicitCopy) arg->setNoImplicitCopy(isNoImplicitCopy); diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp index 7b0949a381a98..65332d83edbe0 100644 --- a/lib/SILGen/SILGenDecl.cpp +++ b/lib/SILGen/SILGenDecl.cpp @@ -561,7 +561,11 @@ class LocalVariableInitialization : public SingleBufferInitialization { // If our instance type is not already @moveOnly wrapped, and it's a // no-implicit-copy parameter, wrap it. - if (!isNoImplicitCopy && instanceType->isCopyable()) { + // + // Unless the function is using ManualOwnership, which checks for + // no-implicit-copies using a different mechanism. + if (!isNoImplicitCopy && instanceType->isCopyable() && + !SGF.B.hasManualOwnershipAttr()) { if (auto *pd = dyn_cast(decl)) { isNoImplicitCopy = pd->isNoImplicitCopy(); isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Consuming; diff --git a/lib/SILGen/SILGenPattern.cpp b/lib/SILGen/SILGenPattern.cpp index 07c96d000e2d7..4513cf5c4e587 100644 --- a/lib/SILGen/SILGenPattern.cpp +++ b/lib/SILGen/SILGenPattern.cpp @@ -1405,7 +1405,10 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var, auto bindValue = value.asBorrowedOperand2(SGF, pattern).getFinalManagedValue(); // Borrow bindings of copyable type should still be no-implicit-copy. - if (!bindValue.getType().isMoveOnly()) { + // + // If we're relying on ManualOwnership for explicit-copies enforcement, + // we don't need the MoveOnlyWrapper. + if (!bindValue.getType().isMoveOnly() && !SGF.B.hasManualOwnershipAttr()) { if (bindValue.getType().isAddress()) { bindValue = ManagedValue::forBorrowedAddressRValue( SGF.B.createCopyableToMoveOnlyWrapperAddr(pattern, bindValue.getValue())); diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index f1c5ef8b80b46..385e621193c15 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -793,6 +793,10 @@ class ArgumentInitHelper { } } } + // If we're relying on ManualOwnership for explicit-copies enforcement, + // we don't need @noImplicitCopy / MoveOnlyWrapper. + if (SGF.B.hasManualOwnershipAttr()) + isNoImplicitCopy = false; // If we have a no implicit copy argument and the argument is trivial, // we need to use copyable to move only to convert it to its move only @@ -1216,8 +1220,9 @@ static void emitCaptureArguments(SILGenFunction &SGF, SILType ty = lowering.getLoweredType(); bool isNoImplicitCopy; - - if (ty.isTrivial(SGF.F) || ty.isMoveOnly()) { + + if (ty.isTrivial(SGF.F) || ty.isMoveOnly() || + SGF.B.hasManualOwnershipAttr()) { isNoImplicitCopy = false; } else if (VD->isNoImplicitCopy()) { isNoImplicitCopy = true; diff --git a/test/SIL/manual_ownership.swift b/test/SIL/manual_ownership.swift index da4943d83a6f8..64a5b8256f31f 100644 --- a/test/SIL/manual_ownership.swift +++ b/test/SIL/manual_ownership.swift @@ -63,6 +63,26 @@ public func basic_return3() -> Triangle { return Triangle() } +@_manualOwnership +func return_borrowed(_ t: borrowing Triangle) -> Triangle { + return t // expected-error {{ownership of 't' is demanded and cannot not be consumed}} +} +@_manualOwnership +func return_borrowed_fixed(_ t: borrowing Triangle) -> Triangle { + return copy t +} + +// FIXME: copy propagation isn't able to simplify this. No copy should be required. +@_manualOwnership +func return_consumingParam(_ t: consuming Triangle) -> Triangle { // expected-error {{accessing 't' produces a copy of it}} + return t +} + +@_manualOwnership +func return_owned(_ t: __owned Triangle) -> Triangle { + return t +} + @_manualOwnership func reassign_with_lets() -> Triangle { let x = Triangle() diff --git a/test/SILGen/manual_ownership.swift b/test/SILGen/manual_ownership.swift index c9298a5233734..7a1a5dbabda98 100644 --- a/test/SILGen/manual_ownership.swift +++ b/test/SILGen/manual_ownership.swift @@ -26,3 +26,49 @@ class TriangleClass { func basic_access_of_loadable(_ t: TriangleClass) -> ShapeClass { return copy t.shape } + +// CHECK-LABEL: sil {{.*}} [manual_ownership] [ossa] @return_borrowed +// CHECK: bb0(%0 : @guaranteed $TriangleClass): +// CHECK-NEXT: debug_value %0 +// CHECK-NEXT: [[IMPL_COPY:%.*]] = copy_value %0 +// CHECK-NEXT: return [[IMPL_COPY]] +// CHECK-NEXT: } // end sil function 'return_borrowed' +@_manualOwnership +@_silgen_name("return_borrowed") +func return_borrowed(_ t: borrowing TriangleClass) -> TriangleClass { + return t +} + +// CHECK-LABEL: sil {{.*}} [manual_ownership] [ossa] @return_consumingParam +// CHECK: bb0(%0 : @_eagerMove @owned $TriangleClass): +// CHECK-NEXT: alloc_box ${ var TriangleClass }, var, name "t" +// CHECK-NEXT: begin_borrow [var_decl] +// CHECK-NEXT: [[ADDR:%.*]] = project_box {{.*}}, 0 +// CHECK-NEXT: store %0 to [init] [[ADDR]] +// CHECK-NEXT: [[ACCESS:%.*]] = begin_access [read] [unknown] [[ADDR]] +// CHECK-NEXT: [[IMPL_COPY:%.*]] = load [copy] [[ACCESS]] +// CHECK-NEXT: end_access [[ACCESS]] +// CHECK-NEXT: end_borrow +// CHECK-NEXT: destroy_value +// CHECK-NEXT: return [[IMPL_COPY]] +// CHECK-NEXT: } // end sil function 'return_consumingParam' +@_manualOwnership +@_silgen_name("return_consumingParam") +func return_consumingParam(_ t: consuming TriangleClass) -> TriangleClass { + return t +} + +// CHECK-LABEL: sil {{.*}} [manual_ownership] [ossa] @return_owned +// CHECK: bb0(%0 : @owned $TriangleClass): +// CHECK-NEXT: debug_value %0 +// CHECK-NEXT: [[BORROW:%.*]] = begin_borrow %0 +// CHECK-NEXT: [[IMPL_COPY:%.*]] = copy_value [[BORROW]] +// CHECK-NEXT: end_borrow [[BORROW]] +// CHECK-NEXT: destroy_value %0 +// CHECK-NEXT: return [[IMPL_COPY]] +// CHECK-NEXT: } // end sil function 'return_owned' +@_manualOwnership +@_silgen_name("return_owned") +func return_owned(_ t: __owned TriangleClass) -> TriangleClass { + return t +} From 341d5310e1dc258f27a695c0ebccac31327cfc3d Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 13:40:33 -0700 Subject: [PATCH 4/7] nfc: simplify ManualOwnership check --- lib/SILGen/SILGenExpr.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 49b9be3fd596d..81409a0e9e86e 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -7282,8 +7282,7 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) { // If we're relying on ManualOwnership for explicit-copies enforcement, // avoid doing address-based emission for loadable types. - if (subType.isLoadableOrOpaque(SGF.F) && - SGF.F.getPerfConstraints() == PerformanceConstraints::ManualOwnership) { + if (subType.isLoadableOrOpaque(SGF.F) && SGF.B.hasManualOwnershipAttr()) { // Interpret this 'load' as a borrow + copy instead. LValue lv = SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedObjectRead); From 03da43dc98b0ac995177a64a02927395f63bb233 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 14:41:29 -0700 Subject: [PATCH 5/7] ManualOwnership: check closures --- lib/SILGen/SILGen.cpp | 18 ++++++++ test/SIL/manual_ownership.swift | 76 +++++++++++++++++++++++++-------- 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 60bbf50df7ba4..a9aa7c7c5ecd1 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1360,6 +1360,24 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F, // Set our actor isolation. F->setActorIsolation(constant.getActorIsolation()); + // Closures automatically infer [manual_ownership] based on outermost func. + // + // We need to add this constraint _prior_ to emitting the closure's body, + // because the output from SILGen slightly differs because of this constraint + // when it comes to the CopyExpr and SILMoveOnlyWrappedType usage. + // + // If ManualOwnership ends up subsuming those prior mechanisms for an + // explicit-copy mode, we can move this somewhere else, like postEmitFunction. + if (auto *ace = constant.getAbstractClosureExpr()) { + if (auto *dc = ace->getOutermostFunctionContext()) { + if (auto *decl = dc->getAsDecl()) { + if (decl->getAttrs().hasAttribute()) { + F->setPerfConstraints(PerformanceConstraints::ManualOwnership); + } + } + } + } + LLVM_DEBUG(llvm::dbgs() << "lowering "; F->printName(llvm::dbgs()); llvm::dbgs() << " : "; diff --git a/test/SIL/manual_ownership.swift b/test/SIL/manual_ownership.swift index 64a5b8256f31f..433972fa60e48 100644 --- a/test/SIL/manual_ownership.swift +++ b/test/SIL/manual_ownership.swift @@ -252,35 +252,52 @@ return (copy ref_result)[2] /// MARK: closures -// FIXME: (1) Closure capture lists need to support the short-hand [copy t] and produce explicit copies. -// We also need a better error message for when this is missed for closure captures. -// (2) Escaping closures need to be recursively checked by the PerformanceDiagnostics. -// We might just need to widen the propagation of [manual_ownership]? -// (3) Autoclosures have no ability to annotate captures. Is that OK? - @_manualOwnership func closure_basic(_ t: Triangle) -> () -> Triangle { - return { return t } // expected-error {{ownership of 't' is demanded by a closure}} + return { // expected-error {{ownership of 't' is demanded by a closure}} + return t // expected-error {{ownership of 't' is demanded and cannot not be consumed}} + } +} +@_manualOwnership +func closure_basic_almost_fixed_1(_ t: Triangle) -> () -> Triangle { + // FIXME: Closure capture lists need to support the short-hand [copy t] that makes the + // closure capture parameter @owned, rather than @guaranteed. Only can work for Copyable types! + return { [x = copy t] in + return x // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} + } } + +@_manualOwnership +func closure_basic_almost_fixed_2(_ t: Triangle) -> () -> Triangle { + return { // expected-error {{ownership of 't' is demanded by a closure}} + return copy t + } +} + @_manualOwnership func closure_basic_fixed(_ t: Triangle) -> () -> Triangle { - return { [t = copy t] in return t } + return { [x = copy t] in + return copy x + } } @_manualOwnership func closure_copies_in_body(_ t: Triangle) -> () -> Triangle { - return { [t = copy t] in - eat(t) // FIXME: missing required copies - eat(t) - return t } + return { [x = copy t] in + eat(x) // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} + use(x) + eat(x) // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} + return x // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} + } } @_manualOwnership func closure_copies_in_body_noescape(_ t: Triangle) -> Triangle { - let f = { [t = copy t] in - eat(t) // FIXME: missing required copies - eat(t) - return t + let f = { [x = copy t] in + eat(x) // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} + use(x) + eat(x) // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} + return x // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} } return f() } @@ -296,7 +313,32 @@ func try_to_assert(_ n: Int, _ names: [String]) { @_manualOwnership func copy_in_autoclosure(_ t: Triangle) { - simple_assert(consumingFunc(t)) // FIXME: missing required copies + simple_assert(consumingFunc(t)) // expected-error {{ownership of 't' is demanded and cannot not be consumed}} +} +@_manualOwnership +func copy_in_autoclosure_fixed(_ t: Triangle) { + simple_assert(consumingFunc(copy t)) +} + +@_manualOwnership +func nested_closures(_ t: Triangle) -> () -> (() -> Triangle) { + return { // expected-error {{ownership of 't' is demanded by a closure}} + { eat(t) }() // expected-error {{ownership of 't' is demanded and cannot not be consumed}} + return { // expected-error {{ownership of 't' is demanded by a closure}} + simple_assert(consumingFunc(t)) // expected-error {{ownership of 't' is demanded and cannot not be consumed}} + return t // expected-error {{ownership of 't' is demanded and cannot not be consumed}} + } + } +} +@_manualOwnership +func nested_closures_fixed(_ t: Triangle) -> () -> (() -> Triangle) { + return { [a = copy t] in + { eat(copy a) }() + return { [b = copy a] in + simple_assert(consumingFunc(copy b)) + return copy b + } + } } /// MARK: generics From 75bc2b5af21186353cbda31d3a3bc3c9e725f83c Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 15:56:46 -0700 Subject: [PATCH 6/7] ManualOwnership: more test coverage --- test/SIL/manual_ownership.swift | 46 +++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/SIL/manual_ownership.swift b/test/SIL/manual_ownership.swift index 433972fa60e48..34656b3bcab01 100644 --- a/test/SIL/manual_ownership.swift +++ b/test/SIL/manual_ownership.swift @@ -382,3 +382,49 @@ func copy_generic_fixed(_ t: T) { borrow_generic(t) consume_generic(copy t) } + +@_manualOwnership +func benchCaptureProp( + _ s: S, _ f: (S.Element, S.Element) -> S.Element) -> S.Element { + + var it = s.makeIterator() // expected-error {{explicit 'copy' required here}} + let initial = it.next()! + return + IteratorSequence(it) // expected-error {{explicit 'copy' required here}} + .reduce(initial, f) +} +@_manualOwnership +func benchCaptureProp_fixed( + _ s: S, _ f: (S.Element, S.Element) -> S.Element) -> S.Element { + + var it = (copy s).makeIterator() + let initial = it.next()! + return + IteratorSequence(copy it) + .reduce(initial, f) +} + +extension FixedWidthInteger { + @_manualOwnership + func leftRotate(_ distance: Int) -> Self { + return (self << distance) | (self >> (Self.bitWidth - distance)) + } + + @_manualOwnership + mutating func rotatedLeft(_ distance: Int) { + // FIXME: this doesn't appear to be solvable + self = (copy self).leftRotate(distance) // expected-error {{explicit 'copy' required here}} + } +} + +struct CollectionOf32BitLittleEndianIntegers where BaseCollection.Element == UInt8 { + var baseCollection: BaseCollection + + @_manualOwnership + init(_ baseCollection: BaseCollection) { + precondition(baseCollection.count % 4 == 0) + self.baseCollection = baseCollection // expected-error {{explicit 'copy' required here}} + } // expected-error {{explicit 'copy' required here}} + + // FIXME: the above initializer shouldn't have any diagnostics +} From 0734267053fb8da1dacf6eee65013eb640617dd4 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 17:00:24 -0700 Subject: [PATCH 7/7] ManualOwnership: fix simple var assign Was mistakenly counting a 'store' as a copying instruction, when it's only a consuming one. SILGen was not handling lvalues that are addresses for loadable types correctly, when emitting a CopyExpr in ManualOwnership. --- lib/SILGen/SILGenExpr.cpp | 11 +++++-- .../Mandatory/PerformanceDiagnostics.cpp | 7 ++++- test/SIL/manual_ownership.swift | 25 ++++++++++++++-- test/SILGen/manual_ownership.swift | 30 ++++++++++++++++--- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 81409a0e9e86e..7759ea8cb926f 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -7283,10 +7283,17 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) { // If we're relying on ManualOwnership for explicit-copies enforcement, // avoid doing address-based emission for loadable types. if (subType.isLoadableOrOpaque(SGF.F) && SGF.B.hasManualOwnershipAttr()) { - // Interpret this 'load' as a borrow + copy instead. + // Do a read on the lvalue. If we get back an address, do a load before + // emitting the explicit copy. LValue lv = SGF.emitLValue(li->getSubExpr(), SGFAccessKind::BorrowedObjectRead); - auto value = SGF.emitBorrowedLValue(li, std::move(lv)); + auto value = SGF.emitRawProjectedLValue(E, std::move(lv)); + if (value.getType().isAddress()) { + // We don't have 'load [explicit_copy]' so do this instead: + // %x = load [copy] %value + // %y = explicit_copy_value %x + value = SGF.emitManagedLoadCopy(E, value.getUnmanagedValue()); + } ManagedValue copy = SGF.B.createExplicitCopyValue(E, value); return RValue(SGF, {copy}, subType.getASTType()); } diff --git a/lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp b/lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp index 924c17a5f01a1..94e3006e05750 100644 --- a/lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp +++ b/lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp @@ -383,6 +383,7 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst, case SILInstructionKind::PartialApplyInst: case SILInstructionKind::DestroyAddrInst: case SILInstructionKind::DestroyValueInst: + case SILInstructionKind::StoreInst: break; // These modify reference counts, but aren't copies. case SILInstructionKind::ExplicitCopyAddrInst: case SILInstructionKind::ExplicitCopyValueInst: @@ -441,7 +442,11 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst, // There's no hope of borrowing access if there's a consuming use. for (auto op : svi->getUses()) { - if (op->getOperandOwnership() == OperandOwnership::ForwardingConsume) { + auto useKind = op->getOperandOwnership(); + + // Only some DestroyingConsume's, like 'store', are interesting. + if (useKind == OperandOwnership::ForwardingConsume + || isa(op->getUser())) { LLVM_DEBUG(llvm::dbgs() << "demanded by "<< *(op->getUser())); diagnose(loc, diag::manualownership_copy_demanded, *name); return false; diff --git a/test/SIL/manual_ownership.swift b/test/SIL/manual_ownership.swift index 34656b3bcab01..0b77959c192c9 100644 --- a/test/SIL/manual_ownership.swift +++ b/test/SIL/manual_ownership.swift @@ -74,7 +74,7 @@ func return_borrowed_fixed(_ t: borrowing Triangle) -> Triangle { // FIXME: copy propagation isn't able to simplify this. No copy should be required. @_manualOwnership -func return_consumingParam(_ t: consuming Triangle) -> Triangle { // expected-error {{accessing 't' produces a copy of it}} +func return_consumingParam(_ t: consuming Triangle) -> Triangle { // expected-error {{ownership of 't' is demanded and cannot not be consumed}} return t } @@ -156,13 +156,32 @@ func basic_function_call(_ t1: Triangle) { /// MARK: control-flow +@_manualOwnership +func check_vars(_ t: Triangle, _ b: Bool) -> Triangle { + var x = Triangle() + if b { x = t } // expected-error {{ownership of 't' is demanded and cannot not be consumed}} + return x // expected-error {{ownership of 'x' is demanded and cannot not be consumed}} +} +@_manualOwnership +func check_vars_fixed(_ t: Triangle, _ b: Bool) -> Triangle { + var x = Triangle() + if b { x = copy t } + return copy x +} -// FIXME: var assignments are somtimes impossible to satisfy with 'copy' +// FIXME: var's still have some issues. +// (1) MandatoryRedundantLoadElimination introduces a 'copy_value' in place of a 'load [copy]' +// @_manualOwnership +// func reassignments_0() -> Triangle { +// var t3 = Triangle() +// t3 = Triangle() +// return t3 +// } // @_manualOwnership // func reassignments_1() { // var t3 = Triangle() -// t3 = copy Triangle() // FIXME: should not be needed +// t3 = Triangle() // t3.borrowing() // } // @_manualOwnership diff --git a/test/SILGen/manual_ownership.swift b/test/SILGen/manual_ownership.swift index 7a1a5dbabda98..b18403eca9813 100644 --- a/test/SILGen/manual_ownership.swift +++ b/test/SILGen/manual_ownership.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend %s -emit-silgen -verify \ +// RUN: %target-swift-frontend %s -emit-silgen -verify -sil-verify-all \ // RUN: -enable-experimental-feature ManualOwnership -o %t.silgen // RUN: %FileCheck %s --input-file %t.silgen @@ -15,9 +15,7 @@ class TriangleClass { // CHECK-NEXT: debug_value %0 // CHECK-NEXT: [[M:%.*]] = class_method %0, #TriangleClass.shape!getter // CHECK-NEXT: [[SHAPE:%.*]] = apply [[M]](%0) -// CHECK-NEXT: [[B:%.*]] = begin_borrow [[SHAPE]] -// CHECK-NEXT: [[COPY:%.*]] = explicit_copy_value [[B]] -// CHECK-NEXT: end_borrow [[B]] +// CHECK-NEXT: [[COPY:%.*]] = explicit_copy_value [[SHAPE]] // CHECK-NEXT: destroy_value [[SHAPE]] // CHECK-NEXT: return [[COPY]] // CHECK-NEXT: } // end sil function 'basic_access_of_loadable' @@ -27,6 +25,30 @@ func basic_access_of_loadable(_ t: TriangleClass) -> ShapeClass { return copy t.shape } +// CHECK-LABEL: sil {{.*}} @var_assign +// CHECK: alloc_box ${ var TriangleClass }, var, name "x" +// CHECK-NEXT: begin_borrow [lexical] [var_decl] +// CHECK-NEXT: [[VAR:%.*]] = project_box {{.*}}, 0 +// CHECK: bb1: +// CHECK-NEXT: [[T_COPY:%.*]] = explicit_copy_value %0 +// CHECK-NEXT: [[ADDR1:%.*]] = begin_access [modify] [unknown] [[VAR]] +// CHECK-NEXT: assign [[T_COPY]] to [[ADDR1]] +// CHECK-NEXT: end_access [[ADDR1]] +// CHECK: bb3: +// CHECK-NEXT: [[ADDR2:%.*]] = begin_access [read] [unknown] [[VAR]] +// CHECK-NEXT: [[LOAD:%.*]] = load [copy] [[ADDR2]] +// CHECK-NEXT: [[X_COPY:%.*]] = explicit_copy_value [[LOAD]] +// CHECK-NEXT: end_access [[ADDR2]] +// CHECK: return [[X_COPY]] +// CHECK-NEXT: } // end sil function 'var_assign' +@_manualOwnership +@_silgen_name("var_assign") +func var_assign(_ t: TriangleClass, _ b: Bool) -> TriangleClass { + var x = TriangleClass() + if b { x = copy t } + return copy x +} + // CHECK-LABEL: sil {{.*}} [manual_ownership] [ossa] @return_borrowed // CHECK: bb0(%0 : @guaranteed $TriangleClass): // CHECK-NEXT: debug_value %0