Merge pull request #62567 from gottesmm/pr-aff949c3a3e3aaf7839d4462bd96b8eb60c06f97

[move-only-object] Teach move only object verifier how to emit proper errors for closure/defer uses

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -731,6 +731,8 @@ ERROR(sil_moveonlychecker_value_used_after_consume, none,
       "'%0' used after consume. Lifetime extension of variable requires a copy", (StringRef))
 ERROR(sil_moveonlychecker_guaranteed_value_consumed, none,
       "'%0' has guaranteed ownership but was consumed", (StringRef))
+ERROR(sil_moveonlychecker_guaranteed_value_captured_by_closure, none,
+      "'%0' has guaranteed ownership but was consumed due to being captured by a closure", (StringRef))
 ERROR(sil_moveonlychecker_inout_not_reinitialized_before_end_of_function, none,
       "'%0' consumed but not reinitialized before end of function", (StringRef))
 ERROR(sil_moveonlychecker_value_consumed_in_a_loop, none,
@@ -740,6 +742,8 @@ ERROR(sil_moveonlychecker_exclusivity_violation, none,
 
 NOTE(sil_moveonlychecker_consuming_use_here, none,
      "consuming use", ())
+NOTE(sil_moveonlychecker_consuming_closure_use_here, none,
+     "closure capture", ())
 NOTE(sil_moveonlychecker_nonconsuming_use_here, none,
      "non-consuming use", ())
 ERROR(sil_moveonlychecker_not_understand_no_implicit_copy, none,

lib/SILGen/SILGenBuilder.cpp

@@ -952,3 +952,12 @@ SILGenBuilder::createMarkMustCheckInst(SILLocation loc, ManagedValue value,
       loc, value.forward(getSILGenFunction()), kind);
   return cloner.clone(mdi);
 }
+
+ManagedValue SILGenBuilder::emitCopyValueOperation(SILLocation loc,
+                                                   ManagedValue value) {
+  auto cvi = SILBuilder::emitCopyValueOperation(loc, value.getValue());
+  // Trivial type.
+  if (cvi == value.getValue())
+    return value;
+  return SGF.emitManagedRValueWithCleanup(cvi);
+}

lib/SILGen/SILGenBuilder.h

@@ -430,6 +430,9 @@ class SILGenBuilder : public SILBuilder {
   using SILBuilder::createMarkMustCheckInst;
   ManagedValue createMarkMustCheckInst(SILLocation loc, ManagedValue value,
                                        MarkMustCheckInst::CheckKind kind);
+
+  using SILBuilder::emitCopyValueOperation;
+  ManagedValue emitCopyValueOperation(SILLocation Loc, ManagedValue v);
 };
 
 } // namespace Lowering

lib/SILGen/SILGenProlog.cpp

@@ -520,34 +520,47 @@ static void emitCaptureArguments(SILGenFunction &SGF,
     auto &lowering = SGF.getTypeLowering(type);
     // Constant decls are captured by value.
     SILType ty = lowering.getLoweredType();
-    SILValue val = SGF.F.begin()->createFunctionArgument(ty, VD);
-
-    bool NeedToDestroyValueAtExit = false;
+    ManagedValue val = ManagedValue::forUnmanaged(
+        SGF.F.begin()->createFunctionArgument(ty, VD));
 
     // If the original variable was settable, then Sema will have treated the
     // VarDecl as an lvalue, even in the closure's use.  As such, we need to
     // allow formation of the address for this captured value.  Create a
     // temporary within the closure to provide this address.
     if (VD->isSettable(VD->getDeclContext())) {
-      auto addr = SGF.emitTemporaryAllocation(VD, ty);
+      auto addr = SGF.emitTemporary(VD, lowering);
       // We have created a copy that needs to be destroyed.
       val = SGF.B.emitCopyValueOperation(Loc, val);
-      NeedToDestroyValueAtExit = true;
-      lowering.emitStore(SGF.B, VD, val, addr, StoreOwnershipQualifier::Init);
-      val = addr;
+      // We use the SILValue version of this because the SILGenBuilder version
+      // will create a cloned cleanup, which we do not want since our temporary
+      // already has a cleanup.
+      //
+      // MG: Is this the right semantics for createStore? Seems like that
+      // should be potentially a different API.
+      SGF.B.emitStoreValueOperation(VD, val.forward(SGF), addr->getAddress(),
+                                    StoreOwnershipQualifier::Init);
+      addr->finishInitialization(SGF);
+      val = addr->getManagedAddress();
+    }
+
+    // If this constant is a move only type, we need to add no_copy checking to
+    // ensure that we do not consume this captured value in the function. This
+    // is because closures can be invoked multiple times which is inconsistent
+    // with consuming the move only type.
+    if (val.getType().isMoveOnly()) {
+      val = val.ensurePlusOne(SGF, Loc);
+      val = SGF.B.createMarkMustCheckInst(Loc, val,
+                                          MarkMustCheckInst::CheckKind::NoCopy);
     }
 
-    SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(val);
-    if (auto *AllocStack = dyn_cast<AllocStackInst>(val))
+    SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(val.getValue());
+    if (auto *AllocStack = dyn_cast<AllocStackInst>(val.getValue())) {
       AllocStack->setArgNo(ArgNo);
-    else {
+    } else {
       SILDebugVariable DbgVar(VD->isLet(), ArgNo);
-      SGF.B.createDebugValue(Loc, val, DbgVar);
+      SGF.B.createDebugValue(Loc, val.getValue(), DbgVar);
     }
 
-    // TODO: Closure contexts should always be guaranteed.
-    if (NeedToDestroyValueAtExit && !lowering.isTrivial())
-      SGF.enterDestroyCleanup(val);
     break;
   }