[no-implicit-copy] Update SILGen/move checker to work with new patterns from copyable_to_moveonly and friends.

This involved doing the following:

1. Update the move only checker to look for new patterns.

2. Teach emitSemanticStore to use a moveonlywrapper_to_copyable to store moveonly
   values into memory. The various checkers will validate that this code is
   correct.

3. When emitting an apply, always unwrap move only variables. In the
   future, I am going to avoid this if a parameter is explicitly marked as also
   being moveonly (e.x.: @moveOnly parameter or @NoEscape argument).

4. Convert from moveOnly -> copyable on return inst automatically in SILGen.

5. Fix SILGenLValue emission so we emit an error diagnostic later rather than
   crash. This is needed to keep SILGen emitting move only addresses (that is no
   implicit copy address only lets) in a form that the move only checker then
   will error upon. Without this change, SILGen crashes instead of emitting an
   error diagnostic in the following test:
   .//test/SILOptimizer/move_only_checker_addressonly_fail.swift

Author: gottesmm

lib/SILGen/SILGenApply.cpp

@@ -1784,8 +1784,33 @@ static void emitRawApply(SILGenFunction &SGF,
 
   // Gather the arguments.
   for (auto i : indices(args)) {
-    auto argValue = (inputParams[i].isConsumed() ? args[i].forward(SGF)
-                                                 : args[i].getValue());
+    SILValue argValue;
+    if (inputParams[i].isConsumed()) {
+      argValue = args[i].forward(SGF);
+      if (argValue->getType().isMoveOnlyWrapped()) {
+        argValue =
+            SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(loc, argValue);
+      }
+    } else {
+      ManagedValue arg = args[i];
+
+      // Move only is not represented in the Swift level type system, so if we
+      // have a move only value, convert it to a non-move only value. The
+      // move/is no escape checkers will ensure that it is legal to do this or
+      // will error. At this point we just want to make sure that the emitted
+      // types line up.
+      if (arg.getType().isMoveOnlyWrapped()) {
+        // We need to borrow so that we can convert from $@moveOnly T -> $T. Use
+        // a formal access borrow to ensure that we have tight scopes like we do
+        // when we borrow fn.
+        if (!arg.isPlusZero())
+          arg = arg.formalAccessBorrow(SGF, loc);
+        arg = SGF.B.createGuaranteedMoveOnlyWrapperToCopyableValue(loc, arg);
+      }
+
+      argValue = arg.getValue();
+    }
+
 #ifndef NDEBUG
     auto inputTy =
         substFnConv.getSILType(inputParams[i], SGF.getTypeExpansionContext());

lib/SILGen/SILGenBuilder.cpp

@@ -12,6 +12,7 @@
 
 #include "SILGenBuilder.h"
 #include "ArgumentSource.h"
+#include "Cleanup.h"
 #include "RValue.h"
 #include "SILGenFunction.h"
 #include "Scope.h"
@@ -450,6 +451,9 @@ static ManagedValue createInputFunctionArgument(SILGenBuilder &B, SILType type,
   case SILArgumentConvention::Direct_Unowned:
     // Unowned parameters are only guaranteed at the instant of the call, so we
     // must retain them even if we're in a context that can accept a +0 value.
+    //
+    // NOTE: If we have a trivial value, the copy will do nothing, so this is
+    // just a convenient way to avoid writing conditional code.
     return ManagedValue::forUnmanaged(arg).copy(SGF, loc);
 
   case SILArgumentConvention::Direct_Owned:
@@ -882,3 +886,37 @@ ManagedValue SILGenBuilder::createMarkDependence(SILLocation loc,
                                    base.forward(getSILGenFunction()));
   return cloner.clone(mdi);
 }
+
+ManagedValue SILGenBuilder::createBeginBorrow(SILLocation loc,
+                                              ManagedValue value,
+                                              bool isLexical) {
+  auto *newValue =
+      SILBuilder::createBeginBorrow(loc, value.getValue(), isLexical);
+  SGF.emitManagedBorrowedRValueWithCleanup(newValue);
+  return ManagedValue::forUnmanaged(newValue);
+}
+
+ManagedValue SILGenBuilder::createMoveValue(SILLocation loc,
+                                            ManagedValue value) {
+  assert(value.isPlusOne(SGF) && "Must be +1 to be moved!");
+  CleanupCloner cloner(*this, value);
+  auto *mdi = createMoveValue(loc, value.forward(getSILGenFunction()));
+  return cloner.clone(mdi);
+}
+
+ManagedValue
+SILGenBuilder::createOwnedMoveOnlyWrapperToCopyableValue(SILLocation loc,
+                                                         ManagedValue value) {
+  assert(value.isPlusOne(SGF) && "Argument must be at +1!");
+  CleanupCloner cloner(*this, value);
+  auto *mdi = createOwnedMoveOnlyWrapperToCopyableValue(
+      loc, value.forward(getSILGenFunction()));
+  return cloner.clone(mdi);
+}
+
+ManagedValue SILGenBuilder::createGuaranteedMoveOnlyWrapperToCopyableValue(
+    SILLocation loc, ManagedValue value) {
+  auto *mdi =
+      createGuaranteedMoveOnlyWrapperToCopyableValue(loc, value.getValue());
+  return ManagedValue::forUnmanaged(mdi);
+}

lib/SILGen/SILGenBuilder.h

@@ -26,8 +26,9 @@
 #include "JumpDest.h"
 #include "ManagedValue.h"
 #include "RValue.h"
-#include "swift/SIL/SILBuilder.h"
 #include "swift/Basic/ProfileCounter.h"
+#include "swift/SIL/SILBuilder.h"
+#include "swift/SIL/SILLocation.h"
 
 namespace swift {
 namespace Lowering {
@@ -362,7 +363,19 @@ class SILGenBuilder : public SILBuilder {
   BranchInst *createBranch(SILLocation Loc, SILBasicBlock *TargetBlock,
                            ArrayRef<ManagedValue> Args);
 
-  using SILBuilder::createReturn;
+  ReturnInst *createReturn(SILLocation Loc, SILValue ReturnValue) {
+    // If we have a move only type as our "result type", convert it back to
+    // being a copyable type. Move only types are never returned today and we
+    // will rely on the SIL level move only and no escape checker to validate
+    // that this is a correct usage. So just make the types line up.
+    if (ReturnValue->getType().isMoveOnlyWrapped()) {
+      auto cvtLoc = RegularLocation::getAutoGeneratedLocation();
+      ReturnValue =
+          createOwnedMoveOnlyWrapperToCopyableValue(cvtLoc, ReturnValue);
+    }
+    return SILBuilder::createReturn(Loc, ReturnValue);
+  }
+
   ReturnInst *createReturn(SILLocation Loc, ManagedValue ReturnValue);
 
   ReturnInst *createReturn(SILLocation Loc, SILValue ReturnValue,
@@ -385,6 +398,22 @@ class SILGenBuilder : public SILBuilder {
   using SILBuilder::createMarkDependence;
   ManagedValue createMarkDependence(SILLocation loc, ManagedValue value,
                                     ManagedValue base);
+
+  using SILBuilder::createBeginBorrow;
+  ManagedValue createBeginBorrow(SILLocation loc, ManagedValue value,
+                                 bool isLexical = false);
+
+  using SILBuilder::createMoveValue;
+  ManagedValue createMoveValue(SILLocation loc, ManagedValue value);
+
+  using SILBuilder::createOwnedMoveOnlyWrapperToCopyableValue;
+  ManagedValue createOwnedMoveOnlyWrapperToCopyableValue(SILLocation loc,
+                                                         ManagedValue value);
+
+  using SILBuilder::createGuaranteedMoveOnlyWrapperToCopyableValue;
+  ManagedValue
+  createGuaranteedMoveOnlyWrapperToCopyableValue(SILLocation loc,
+                                                 ManagedValue value);
 };
 
 } // namespace Lowering

lib/SILGen/SILGenDecl.cpp

@@ -443,8 +443,15 @@ class LetValueInitialization : public Initialization {
 
 public:
   LetValueInitialization(VarDecl *vd, SILGenFunction &SGF) : vd(vd) {
-    auto &lowering = SGF.getTypeLowering(vd->getType());
-    
+    const TypeLowering *lowering = nullptr;
+    if (SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly) &&
+        vd->getAttrs().hasAttribute<NoImplicitCopyAttr>()) {
+      lowering = &SGF.getTypeLowering(
+          SILMoveOnlyType::get(vd->getType()->getCanonicalType()));
+    } else {
+      lowering = &SGF.getTypeLowering(vd->getType());
+    }
+
     // Decide whether we need a temporary stack buffer to evaluate this 'let'.
     // There are four cases we need to handle here: parameters, initialized (or
     // bound) decls, uninitialized ones, and async let declarations.
@@ -469,13 +476,13 @@ class LetValueInitialization : public Initialization {
       // If this is a let with an initializer or bound value, we only need a
       // buffer if the type is address only.
       needsTemporaryBuffer =
-          lowering.isAddressOnly() && SGF.silConv.useLoweredAddresses();
+          lowering->isAddressOnly() && SGF.silConv.useLoweredAddresses();
     }
 
     // Make sure that we have a non-address only type when binding a
     // @_noImplicitCopy let.
-    if (SGF.getASTContext().LangOpts.hasFeature(Feature::MoveOnly) &&
-        lowering.isAddressOnly() &&
+    if (SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly) &&
+        lowering->isAddressOnly() &&
         vd->getAttrs().hasAttribute<NoImplicitCopyAttr>()) {
       auto d = diag::noimplicitcopy_used_on_generic_or_existential;
       diagnose(SGF.getASTContext(), vd->getLoc(), d);
@@ -485,13 +492,13 @@ class LetValueInitialization : public Initialization {
       bool isLexical =
           SGF.getASTContext().SILOpts.supportsLexicalLifetimes(SGF.getModule());
       address =
-          SGF.emitTemporaryAllocation(vd, lowering.getLoweredType(),
+          SGF.emitTemporaryAllocation(vd, lowering->getLoweredType(),
                                       false /*hasDynamicLifetime*/, isLexical);
       if (isUninitialized)
         address = SGF.B.createMarkUninitializedVar(vd, address);
-      DestroyCleanup = SGF.enterDormantTemporaryCleanup(address, lowering);
+      DestroyCleanup = SGF.enterDormantTemporaryCleanup(address, *lowering);
       SGF.VarLocs[vd] = SILGenFunction::VarLoc::get(address);
-    } else if (!lowering.isTrivial()) {
+    } else if (!lowering->isTrivial()) {
       // Push a cleanup to destroy the let declaration.  This has to be
       // inactive until the variable is initialized: if control flow exits the
       // before the value is bound, we don't want to destroy the value.
@@ -554,31 +561,68 @@ class LetValueInitialization : public Initialization {
       address = value;
     SILLocation PrologueLoc(vd);
 
-    if (SGF.getASTContext().SILOpts.supportsLexicalLifetimes(SGF.getModule()) &&
-        value->getOwnershipKind() != OwnershipKind::None) {
-      if (!SGF.getASTContext().LangOpts.hasFeature(Feature::MoveOnly)) {
-        value = SILValue(
-            SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true));
-      } else {
-        // If we have an owned value that had a cleanup, then create a
-        // move_value that acts as a consuming use of the value. The reason why
-        // we want this is even if we are only performing a borrow for our
-        // lexical lifetime, we want to ensure that our defs see this
-        // initialization as consuming this value.
-        if (value->getOwnershipKind() == OwnershipKind::Owned) {
-          assert(wasPlusOne);
-          value = SILValue(SGF.B.createMoveValue(PrologueLoc, value));
-        }
+    if (SGF.getASTContext().SILOpts.supportsLexicalLifetimes(SGF.getModule())) {
+      if (value->getOwnershipKind() != OwnershipKind::None) {
+        if (!SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly)) {
+          value =
+              SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true);
+        } else {
+          // If we have an owned moveonly value that had a cleanup, then create
+          // a move_value that acts as a consuming use of the value. The reason
+          // why we want this is even if we are only performing a borrow for our
+          // lexical lifetime, we want to ensure that our defs see this
+          // initialization as consuming this value.
+          if (value->getType().isMoveOnlyWrapped() &&
+              value->getOwnershipKind() == OwnershipKind::Owned) {
+            assert(wasPlusOne);
+            // NOTE: If our type is trivial when not wrapped in a
+            // SILMoveOnlyType, this will return a trivial value. We rely on the
+            // checker to determine if this is an acceptable use of the value.
+            value = SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(PrologueLoc,
+                                                                    value);
+          }
 
-        if (vd->getAttrs().hasAttribute<NoImplicitCopyAttr>()) {
-          value = SILValue(SGF.B.createBeginBorrow(PrologueLoc, value,
-                                                   /*isLexical*/ true));
-          value = SGF.B.createCopyValue(PrologueLoc, value);
+          // If we still have a non-trivial thing, emit code that will need to
+          // be cleaned up. If we are now trivial, we do not need to cleanup
+          // anything.
+          if (!value->getType().isTrivial(SGF.F)) {
+            if (vd->getAttrs().hasAttribute<NoImplicitCopyAttr>()) {
+              value = SGF.B.createBeginBorrow(PrologueLoc, value,
+                                              /*isLexical*/ true);
+              value = SGF.B.createCopyValue(PrologueLoc, value);
+              value = SGF.B.createCopyableToMoveOnlyWrapperValue(PrologueLoc,
+                                                                 value);
+              value = SGF.B.createMarkMustCheckInst(
+                  PrologueLoc, value,
+                  MarkMustCheckInst::CheckKind::NoImplicitCopy);
+            } else {
+              value = SGF.B.createBeginBorrow(PrologueLoc, value,
+                                              /*isLexical*/ true);
+            }
+          }
+        }
+      } else {
+        if (SGF.getASTContext().LangOpts.Features.count(Feature::MoveOnly) &&
+            vd->getAttrs().hasAttribute<NoImplicitCopyAttr>() &&
+            value->getType().isTrivial(SGF.F)) {
+          // We are abusing this. This should be a separate instruction just for
+          // converting from copyable trivial to move only. I am abusing it
+          // here by using it multiple times in different ways.
+          value =
+              SGF.B.createCopyableToMoveOnlyWrapperValue(PrologueLoc, value);
+          value = SGF.B.createBeginBorrow(PrologueLoc, value,
+                                          /*isLexical*/ true);
+          // We use an explicit copy value since:
+          //
+          // 1. We already have a move only type here. So we can't use a normal
+          // copy here due to the pattern we are creating. We could avoid this,
+          // but it is not worth fixing b/c of point 2.
+          //
+          // 2. Since this is a trivial value, when we remove their move only
+          // ness, this will become a no-op meaning no-overhead.
+          value = SGF.B.createExplicitCopyValue(PrologueLoc, value);
           value = SGF.B.createMarkMustCheckInst(
               PrologueLoc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
-        } else {
-          value = SILValue(
-              SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true));
         }
       }
     }
@@ -608,7 +652,8 @@ class LetValueInitialization : public Initialization {
       // Disable the rvalue expression cleanup, since the let value
       // initialization has a cleanup that lives for the entire scope of the
       // let declaration.
-      bindValue(value.forward(SGF), SGF, value.isPlusOne(SGF));
+      bool isPlusOne = value.isPlusOne(SGF);
+      bindValue(value.forward(SGF), SGF, isPlusOne);
     } else {
       // Disable the expression cleanup of the copy, since the let value
       // initialization has a cleanup that lives for the entire scope of the
@@ -631,8 +676,9 @@ class LetValueInitialization : public Initialization {
       SGF.Cleanups.forwardCleanup(cleanup);
 
     // Activate the destroy cleanup.
-    if (DestroyCleanup != CleanupHandle::invalid())
+    if (DestroyCleanup != CleanupHandle::invalid()) {
       SGF.Cleanups.setCleanupState(DestroyCleanup, CleanupState::Active);
+    }
 
     DidFinish = true;
   }
@@ -1795,6 +1841,9 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
     return;
   }
 
+  // This handles any case where we copy + begin_borrow or copyable_to_moveonly
+  // + begin_borrow. In either case we just need to end the lifetime of the
+  // begin_borrow's operand.
   if (auto *bbi = dyn_cast<BeginBorrowInst>(Val.getDefiningInstruction())) {
     B.createEndBorrow(silLoc, bbi);
     B.emitDestroyValueOperation(silLoc, bbi->getOperand());
@@ -1814,6 +1863,31 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
             }
           }
         }
+
+        if (auto *copyToMove = dyn_cast<CopyableToMoveOnlyWrapperValueInst>(
+                mvi->getOperand())) {
+          if (auto *cvi = dyn_cast<CopyValueInst>(copyToMove->getOperand())) {
+            if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
+              if (bbi->isLexical()) {
+                B.emitDestroyValueOperation(silLoc, mvi);
+                B.createEndBorrow(silLoc, bbi);
+                B.emitDestroyValueOperation(silLoc, bbi->getOperand());
+                return;
+              }
+            }
+          }
+        }
+
+        if (auto *cvi = dyn_cast<ExplicitCopyValueInst>(mvi->getOperand())) {
+          if (auto *bbi = dyn_cast<BeginBorrowInst>(cvi->getOperand())) {
+            if (bbi->isLexical()) {
+              B.emitDestroyValueOperation(silLoc, mvi);
+              B.createEndBorrow(silLoc, bbi);
+              B.emitDestroyValueOperation(silLoc, bbi->getOperand());
+              return;
+            }
+          }
+        }
       }
     }
   }