From da4f27e31bd94d6f04ec1dd4fc21a0a5356faff6 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Wed, 24 Sep 2025 11:50:39 -0700 Subject: [PATCH 1/3] 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/3] 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/3] 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 +}