Merge pull request #84504 from kavon/manual-ownership/usability-fixes-3

Usability improvements for ManualOwnership (part 3)

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -2141,6 +2141,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/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<ManualOwnershipAttr>()) {
+          F->setPerfConstraints(PerformanceConstraints::ManualOwnership);
+        }
+      }
+    }
+  }
+
   LLVM_DEBUG(llvm::dbgs() << "lowering ";
              F->printName(llvm::dbgs());
              llvm::dbgs() << " : ";

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

@@ -7271,6 +7271,25 @@ 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.B.hasManualOwnershipAttr()) {
+      // 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.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());
+    }
+
     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/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<StoreInst>(op->getUser())) {
                 LLVM_DEBUG(llvm::dbgs() << "demanded by "<< *(op->getUser()));
                 diagnose(loc, diag::manualownership_copy_demanded, *name);
                 return false;

lib/Sema/TypeCheckAttr.cpp

@@ -497,6 +497,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;