[move-only] Emit an error if we /ever/ partially consume a noncopyable type.

The reason why I am doing this is that this was not part of the original
evolution proposal (it was called an extension) and after some discussion it was
realized that partial consumption would benefit from discussion on the forums.

rdar://111353459

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -785,6 +785,9 @@ ERROR(sil_movechecking_notconsumable_but_assignable_was_consumed, none,
 ERROR(sil_movechecking_cannot_destructure_has_deinit, none,
       "cannot partially consume '%0' when it has a deinitializer",
       (StringRef))
+ERROR(sil_movechecking_cannot_destructure, none,
+      "cannot partially consume '%0'",
+      (StringRef))
 ERROR(sil_movechecking_discard_missing_consume_self, none,
       "must consume 'self' before exiting method that discards self", ())
 ERROR(sil_movechecking_reinit_after_discard, none,

include/swift/Basic/Features.def

@@ -134,6 +134,7 @@ EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
 EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
 EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
 EXPERIMENTAL_FEATURE(MoveOnlyResilientTypes, true)
+EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)
 
 EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
 EXPERIMENTAL_FEATURE(TypeWitnessSystemInference, false)

lib/AST/ASTPrinter.cpp

@@ -3310,6 +3310,11 @@ static bool usesFeatureMoveOnlyResilientTypes(Decl *decl) {
   return false;
 }
 
+static bool usesFeatureMoveOnlyPartialConsumption(Decl *decl) {
+  // Partial consumption does not affect declarations directly.
+  return false;
+}
+
 static bool usesFeatureOneWayClosureParameters(Decl *decl) {
   return false;
 }

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1452,10 +1452,12 @@ struct CopiedLoadBorrowEliminationVisitor final
 //                  MARK: DestructureThroughDeinit Checking
 //===----------------------------------------------------------------------===//
 
-static void
-checkForDestructureThroughDeinit(MarkMustCheckInst *rootAddress, Operand *use,
-                                 TypeTreeLeafTypeRange usedBits,
-                                 DiagnosticEmitter &diagnosticEmitter) {
+/// When partial consumption is enabled, we only allow for destructure through
+/// deinits. When partial consumption is disabled, we error on /all/ partial
+/// consumption.
+static void checkForDestructure(MarkMustCheckInst *rootAddress, Operand *use,
+                                TypeTreeLeafTypeRange usedBits,
+                                DiagnosticEmitter &diagnosticEmitter) {
   LLVM_DEBUG(llvm::dbgs() << "    DestructureNeedingUse: " << *use->getUser());
 
   SILFunction *fn = rootAddress->getFunction();
@@ -1473,6 +1475,29 @@ checkForDestructureThroughDeinit(MarkMustCheckInst *rootAddress, Operand *use,
   if (iterType.isMoveOnlyWrapped())
     return;
 
+  // If we are not allowing for any partial consumption, just emit an error
+  // immediately.
+  if (!rootAddress->getModule().getASTContext().LangOpts.hasFeature(
+          Feature::MoveOnlyPartialConsumption)) {
+    // If the types equal, just bail early.
+    if (iterType == targetType)
+      return;
+
+    // Otherwise, build up the path string and emit the error.
+    SmallString<128> pathString;
+    auto rootType = rootAddress->getType();
+    if (iterType != rootType) {
+      llvm::raw_svector_ostream os(pathString);
+      pair.constructPathString(iterType, {rootType, fn}, rootType, fn, os);
+    }
+
+    diagnosticEmitter.emitCannotDestructureNominalError(
+        rootAddress, pathString, nullptr /*nominal*/, use->getUser(),
+        false /*is for deinit error*/);
+    return;
+  }
+
+  // Otherwise, walk the type looking for the deinit.
   while (iterType != targetType) {
     // If we have a nominal type as our parent type, see if it has a
     // deinit. We know that it must be non-copyable since copyable types
@@ -1491,8 +1516,9 @@ checkForDestructureThroughDeinit(MarkMustCheckInst *rootAddress, Operand *use,
           pair.constructPathString(iterType, {rootType, fn}, rootType, fn, os);
         }
 
-        diagnosticEmitter.emitCannotDestructureDeinitNominalError(
-            rootAddress, pathString, nom, use->getUser());
+        diagnosticEmitter.emitCannotDestructureNominalError(
+            rootAddress, pathString, nom, use->getUser(),
+            true /*is for deinit error*/);
         break;
       }
     }
@@ -1702,8 +1728,10 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
 
     // TODO: Add borrow checking here like below.
 
-    // TODO: Add destructure deinit checking here once address only checking is
-    // completely brought up.
+    // If we have a copy_addr, we are either going to have a take or a
+    // copy... in either case, this copy_addr /is/ going to be a consuming
+    // operation. Make sure to check if we semantically destructure.
+    checkForDestructure(markedValue, op, *leafRange, diagnosticEmitter);
 
     if (copyAddr->isTakeOfSrc()) {
       LLVM_DEBUG(llvm::dbgs() << "Found take: " << *user);
@@ -1771,17 +1799,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       return false;
     }
 
-    checkForDestructureThroughDeinit(markedValue, op, *leafRange,
-                                     diagnosticEmitter);
-
-    // If we emitted an error diagnostic, do not transform further and instead
-    // mark that we emitted an early diagnostic and return true.
-    if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
-      LLVM_DEBUG(llvm::dbgs()
-                 << "Emitting destructure through deinit error!\n");
-      return true;
-    }
-
     // Canonicalize the lifetime of the load [take], load [copy].
     LLVM_DEBUG(llvm::dbgs() << "Running copy propagation!\n");
     moveChecker.changed |= moveChecker.canonicalizer.canonicalize();
@@ -1882,6 +1899,19 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
         useState.recordLivenessUse(dvi, *leafRange);
       }
     } else {
+      // Now that we know that we are going to perform a take, perform a
+      // checkForDestructure.
+      checkForDestructure(markedValue, op, *leafRange, diagnosticEmitter);
+
+      // If we emitted an error diagnostic, do not transform further and instead
+      // mark that we emitted an early diagnostic and return true.
+      if (numDiagnostics !=
+          moveChecker.diagnosticEmitter.getDiagnosticCount()) {
+        LLVM_DEBUG(llvm::dbgs()
+                   << "Emitting destructure through deinit error!\n");
+        return true;
+      }
+
       // If we had a load [copy], store this into the copy list. These are the
       // things that we must merge into destroy_addr or reinits after we are
       // done checking. The load [take] are already complete and good to go.
@@ -1926,8 +1956,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
     // error.
     unsigned numDiagnostics =
         moveChecker.diagnosticEmitter.getDiagnosticCount();
-    checkForDestructureThroughDeinit(markedValue, op, *leafRange,
-                                     diagnosticEmitter);
+    checkForDestructure(markedValue, op, *leafRange, diagnosticEmitter);
     if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
       LLVM_DEBUG(llvm::dbgs()
                  << "Emitting destructure through deinit error!\n");

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

@@ -856,3 +856,38 @@ void DiagnosticEmitter::emitCannotDestructureDeinitNominalError(
       deinitedNominal->getValueTypeDestructor()->getLoc(/*SerializedOK=*/false))
     astContext.Diags.diagnose(deinitLoc, diag::sil_movechecking_deinit_here);
 }
+
+void DiagnosticEmitter::emitCannotDestructureNominalError(
+    MarkMustCheckInst *markedValue, StringRef pathString,
+    NominalTypeDecl *nominal, SILInstruction *consumingUser,
+    bool isDueToDeinit) {
+  auto &astContext = fn->getASTContext();
+
+  SmallString<64> varName;
+  getVariableNameForValue(markedValue, varName);
+
+  if (!pathString.empty())
+    varName.append(pathString);
+
+  assert(
+      astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ==
+          isDueToDeinit &&
+      "Should only emit deinit error if we have partial consumption enabled");
+  if (isDueToDeinit) {
+    diagnose(astContext, consumingUser,
+             diag::sil_movechecking_cannot_destructure_has_deinit, varName);
+  } else {
+    diagnose(astContext, consumingUser,
+             diag::sil_movechecking_cannot_destructure, varName);
+  }
+  registerDiagnosticEmitted(markedValue);
+
+  if (!isDueToDeinit)
+    return;
+
+  // Point to the deinit if we know where it is.
+  assert(nominal);
+  if (auto deinitLoc =
+          nominal->getValueTypeDestructor()->getLoc(/*SerializedOK=*/false))
+    astContext.Diags.diagnose(deinitLoc, diag::sil_movechecking_deinit_here);
+}

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.h

@@ -169,6 +169,11 @@ class DiagnosticEmitter {
                                                StringRef pathString,
                                                NominalTypeDecl *deinitedNominal,
                                                SILInstruction *consumingUser);
+  void emitCannotDestructureNominalError(MarkMustCheckInst *markedValue,
+                                         StringRef pathString,
+                                         NominalTypeDecl *nominal,
+                                         SILInstruction *consumingUser,
+                                         bool isDueToDeinit);
 
 private:
   /// Emit diagnostics for the final consuming uses and consuming uses needing

test/Interpreter/moveonly_address_maximize.swift

@@ -1,5 +1,5 @@
-// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all) | %FileCheck %s
-// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all) | %FileCheck %s
+// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption) | %FileCheck %s
+// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption) | %FileCheck %s
 
 // REQUIRES: executable_test
 

test/SILGen/moveonly_library_evolution.swift

@@ -1,5 +1,5 @@
-// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s | %FileCheck %s
-// RUN: %target-swift-emit-sil -O -sil-verify-all -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s
+// RUN: %target-swift-emit-silgen -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s | %FileCheck %s
+// RUN: %target-swift-emit-sil -O -sil-verify-all -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyResilientTypes -enable-library-evolution %s
 
 ////////////////////////
 // MARK: Declarations //

test/SILOptimizer/moveonly_addresschecker.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -module-name moveonly_addresschecker -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-sil-verify-all %s | %FileCheck %s
+// RUN: %target-sil-opt -module-name moveonly_addresschecker -sil-move-only-address-checker -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyPartialConsumption -enable-sil-verify-all %s | %FileCheck %s
 
 sil_stage raw
 

test/SILOptimizer/moveonly_addresschecker_destructure_through_deinit_diagnostics.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyTuples %s
+// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyTuples %s
 
 // This test validates that we properly emit errors if we partially invalidate
 // through a type with a deinit.