Usability improvements for ManualOwnership (part 2)

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,

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);

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<ParamDecl>(decl)) {
         isNoImplicitCopy = pd->isNoImplicitCopy();
         isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Consuming;

lib/SILGen/SILGenExpr.cpp

@@ -7279,6 +7279,19 @@ RValue RValueEmitter::visitCopyExpr(CopyExpr *E, SGFContext C) {
 
   if (auto *li = dyn_cast<LoadExpr>(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));

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()));

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;

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<FuncDecl>(D)) {
     if (visitOwnershipAttr(attr))
       return;

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()
@@ -172,9 +192,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 +204,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)
+  }
+  (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 {
-    p = p.midpoint((copy x.nontrivial).a) // expected-error {{accessing 'x.nontrivial' produces a copy of it}}
+    let xnt = copy x.nontrivial
+    p = p.midpoint(xnt.a)
   }
-  (copy t.nontrivial).a = p // expected-error {{accessing 't.nontrivial' produces a copy of it}}
+  nt.a = p
 }
 
 

test/SILGen/manual_ownership.swift

@@ -0,0 +1,74 @@
+// 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
+}
+
+// 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
+}

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)
+}