Merge pull request #61964 from gottesmm/pr-140968cb0578adfb1af5718b8468c8c391695642

[move-only] Teach the object/address move operator checker to not check move only types.

Author: gottesmm

lib/SILGen/SILGenExpr.cpp

@@ -41,6 +41,7 @@
 #include "swift/AST/Types.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/type_traits.h"
+#include "swift/SIL/Consumption.h"
 #include "swift/SIL/DynamicCasts.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILInstruction.h"
@@ -5934,7 +5935,11 @@ RValue RValueEmitter::visitMoveExpr(MoveExpr *E, SGFContext C) {
   }
 
   // If we aren't loadable, then create a temporary initialization and
-  // mark_unresolved_move into that.
+  // mark_unresolved_move into that if we have a copyable type if we have a move
+  // only, just add a copy_addr init.
+  //
+  // The reason why we do this is that we only use mark_unresolved_move_addr and
+  // the move operator checker for copyable values.
   std::unique_ptr<TemporaryInitialization> optTemp;
   optTemp = SGF.emitTemporary(E, SGF.getTypeLowering(subType));
   SILValue toAddr = optTemp->getAddressForInPlaceInitialization(SGF, E);
@@ -5945,7 +5950,12 @@ RValue RValueEmitter::visitMoveExpr(MoveExpr *E, SGFContext C) {
       SGF.emitRValue(subExpr, SGFContext(SGFContext::AllowImmediatePlusZero))
           .getAsSingleValue(SGF, subExpr);
   assert(mv.getType().isAddress());
-  SGF.B.createMarkUnresolvedMoveAddr(subExpr, mv.getValue(), toAddr);
+  if (mv.getType().isMoveOnly()) {
+    SGF.B.createCopyAddr(subExpr, mv.getValue(), toAddr, IsNotTake,
+                         IsInitialization);
+  } else {
+    SGF.B.createMarkUnresolvedMoveAddr(subExpr, mv.getValue(), toAddr);
+  }
   optTemp->finishInitialization(SGF);
   return RValue(SGF, {optTemp->getManagedAddress()}, subType.getASTType());
 }

lib/SILGen/SILGenLValue.cpp

@@ -30,6 +30,7 @@
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/PropertyWrappers.h"
 #include "swift/AST/TypeCheckRequests.h"
+#include "swift/SIL/Consumption.h"
 #include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/PrettyStackTrace.h"
@@ -3914,11 +3915,21 @@ LValue SILGenLValue::visitMoveExpr(MoveExpr *e, SGFAccessKind accessKind,
 
   ManagedValue addr = SGF.emitAddressOfLValue(e, std::move(baseLV));
 
-  // Now create the temporary and
+  // Now create the temporary and move our value into there.
   auto temp =
       SGF.emitFormalAccessTemporary(e, SGF.F.getTypeLowering(addr.getType()));
   auto toAddr = temp->getAddressForInPlaceInitialization(SGF, e);
-  SGF.B.createMarkUnresolvedMoveAddr(e, addr.getValue(), toAddr);
+
+  // If we have a move only type, we use a copy_addr that will be handled by the
+  // address move only checker. If we have a copyable type, we need to use a
+  // mark_unresolved_move_addr to ensure that the move operator checker performs
+  // the relevant checking.
+  if (addr.getType().isMoveOnly()) {
+    SGF.B.createCopyAddr(e, addr.getValue(), toAddr, IsNotTake,
+                         IsInitialization);
+  } else {
+    SGF.B.createMarkUnresolvedMoveAddr(e, addr.getValue(), toAddr);
+  }
   temp->finishInitialization(SGF);
 
   // Now return the temporary in a value component.

lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp

@@ -400,7 +400,8 @@ bool MoveKillsCopyableValuesChecker::check() {
   SmallSetVector<SILValue, 32> valuesToCheck;
 
   for (auto *arg : fn->getEntryBlock()->getSILFunctionArguments()) {
-    if (arg->getOwnershipKind() == OwnershipKind::Owned) {
+    if (arg->getOwnershipKind() == OwnershipKind::Owned &&
+        !arg->getType().isMoveOnly()) {
       LLVM_DEBUG(llvm::dbgs() << "Found owned arg to check: " << *arg);
       valuesToCheck.insert(arg);
     }
@@ -409,7 +410,7 @@ bool MoveKillsCopyableValuesChecker::check() {
   for (auto &block : *fn) {
     for (auto &ii : block) {
       if (auto *bbi = dyn_cast<BeginBorrowInst>(&ii)) {
-        if (bbi->isLexical()) {
+        if (bbi->isLexical() && !bbi->getType().isMoveOnly()) {
           LLVM_DEBUG(llvm::dbgs()
                      << "Found lexical lifetime to check: " << *bbi);
           valuesToCheck.insert(bbi);
@@ -551,19 +552,28 @@ class MoveKillsCopyableValuesCheckerPass : public SILFunctionTransform {
           SILAnalysis::InvalidationKind::BranchesAndInstructions);
     }
 
-    // Now search through our function one last time and any move_value
-    // [allows_diagnostics] that remain are ones that we did not know how to
-    // check so emit a diagnostic so the user doesn't assume that they have
-    // guarantees.
+    // Now search through our function one last time and:
+    //
+    // 1. Given any move_value on a move only type, just unset the allows
+    //    diagnostics flag. The move checker will have emitted any errors caused
+    //    by our move [allows_diagnostic] earlier in the compilation pipeline.
+    //
+    // 2. Any move_value [allows_diagnostics] that remain that are not on a move
+    //    only type are ones that we did not know how to check so emit a
+    //    diagnostic so the user doesn't assume that they have guarantees.
     //
     // TODO: Emit specific diagnostics here (e.x.: _move of global).
-    if (DisableUnhandledMoveDiagnostic)
-      return;
     for (auto &block : *fn) {
       for (auto &inst : block) {
         if (auto *mvi = dyn_cast<MoveValueInst>(&inst)) {
           if (mvi->getAllowDiagnostics()) {
-            emitUnsupportedUseCaseError(mvi);
+            if (mvi->getType().isMoveOnly()) {
+              mvi->setAllowsDiagnostics(false);
+              continue;
+            }
+
+            if (!DisableUnhandledMoveDiagnostic)
+              emitUnsupportedUseCaseError(mvi);
           }
         }
       }

test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

@@ -2061,3 +2061,15 @@ public func closureAndClosureCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
     }
     f()
 }
+
+/////////////////////////////
+// Tests For Move Operator //
+/////////////////////////////
+
+func moveOperatorTest(_ k: __owned Klass) {
+    var k2 = k // expected-error {{'k2' consumed more than once}}
+    k2 = Klass()
+    let k3 = _move k2 // expected-note {{consuming use}}
+    let _ = _move k2 // expected-note {{consuming use}}
+    let _ = k3
+}

test/SILOptimizer/moveonly_objectchecker_diagnostics.swift

@@ -2326,3 +2326,14 @@ public func closureAndClosureCaptureClassOwnedArgUseAfterConsume2(_ x2: __owned
     f()
     print(x2) // expected-note {{consuming use}}
 }
+
+/////////////////////////////
+// Tests For Move Operator //
+/////////////////////////////
+
+func moveOperatorTest(_ k: __owned Klass) {
+    let k2 = k // expected-error {{'k2' consumed more than once}}
+    let k3 = _move k2 // expected-note {{consuming use}}
+    let _ = _move k2 // expected-note {{consuming use}}
+    let _ = k3
+}

test/SILOptimizer/noimplicitcopy.swift

@@ -2212,3 +2212,14 @@ public func closureAndClosureCaptureClassOwnedArgUseAfterConsume2(@_noImplicitCo
     f()
     print(x2) // expected-note {{consuming use}}
 }
+
+/////////////////////////////
+// Tests For Move Operator //
+/////////////////////////////
+
+func moveOperatorTest(_ k: __owned Klass) {
+    @_noImplicitCopy let k2 = k // expected-error {{'k2' consumed more than once}}
+    @_noImplicitCopy let k3 = _move k2 // expected-note {{consuming use}}
+    let _ = _move k2 // expected-note {{consuming use}}
+    let _ = k3
+}