[move-only] Do not attempt to process mark_must_check if we detect they have a partial_apply that was identified by an earlier diagnostic as escaping.

Previously, we would emit a "compiler doesn't understand error" since we would
detect the escape and fail. That is the correct behavior here if the
partial_apply is not already identified as escaping by an earlier pass. But in
the case where we see that the partial_apply's callee was marked with
semantics::NO_MOVEONLY_DIAGNOSTICS, then we:

1. Suppress the "compiler doesn't understand error" for this specific
   mark_must_check.

2. Suppress function wide the "copy of noncopyable type" error. Since we stopped
   processing the mark_must_check that was passed to the partial_apply, we may
   have left copies of noncopyable types on that mark_must_check value. This is
   ok since the user will get the error, will recompile, and if any further show
   up after they fix the inout escaping issue, they will be emitted
   appropriately.

(cherry picked from commit 07979f0)

Author: gottesmm

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerTester.cpp

@@ -105,20 +105,18 @@ class MoveOnlyAddressCheckerTesterPass : public SILFunctionTransform {
                << '\n');
 
     bool madeChange = false;
-    unsigned diagCount = 0;
     if (moveIntroducersToProcess.empty()) {
       LLVM_DEBUG(llvm::dbgs() << "No move introducers found?!\n");
     } else {
       borrowtodestructure::IntervalMapAllocator allocator;
       MoveOnlyAddressChecker checker{getFunction(), diagnosticEmitter,
                                      allocator, domTree, poa};
       madeChange = checker.check(moveIntroducersToProcess);
-      diagCount = checker.diagnosticEmitter.getDiagnosticCount();
     }
 
     // If we did not emit any diagnostics, emit a diagnostic if we missed any
     // copies.
-    if (!diagCount) {
+    if (!diagnosticEmitter.emittedDiagnostic()) {
       emitCheckerMissedCopyOfNonCopyableTypeErrors(getFunction(),
                                                    diagnosticEmitter);
     }

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1902,6 +1902,8 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       if (fArg->getArgumentConvention().isInoutConvention() &&
           pas->getCalleeFunction()->hasSemanticsAttr(
               semantics::NO_MOVEONLY_DIAGNOSTICS)) {
+        diagnosticEmitter.emitEarlierPassEmittedDiagnostic(markedValue);
+        return false;
       }
     }
 
@@ -2652,9 +2654,19 @@ bool MoveOnlyAddressChecker::check(
     LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *markedValue);
 
     // Perform our address check.
+    unsigned diagnosticEmittedByEarlierPassCount =
+      diagnosticEmitter.getDiagnosticEmittedByEarlierPassCount();
     if (!pimpl.performSingleCheck(markedValue)) {
-      LLVM_DEBUG(llvm::dbgs()
-                 << "Failed to perform single check! Emitting error!\n");
+      if (diagnosticEmittedByEarlierPassCount !=
+          diagnosticEmitter.getDiagnosticEmittedByEarlierPassCount()) {
+        LLVM_DEBUG(
+            llvm::dbgs()
+            << "Failed to perform single check but found earlier emitted "
+               "error. Not emitting checker doesn't understand diagnostic!\n");
+        continue;
+      }
+      LLVM_DEBUG(llvm::dbgs() << "Failed to perform single check! Emitting "
+                                 "compiler doesn't understand diagnostic!\n");
       // If we fail the address check in some way, set the diagnose!
       diagnosticEmitter.emitCheckerDoesntUnderstandDiagnostic(markedValue);
     }

lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

@@ -179,7 +179,7 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
     // remain. If we emitted a diagnostic, we just want to rewrite all of the
     // non-copyable copies into explicit variants below and let the user
     // recompile.
-    if (!checker.diagnosticEmitter.getDiagnosticCount()) {
+    if (!checker.diagnosticEmitter.emittedDiagnostic()) {
       emitCheckerMissedCopyOfNonCopyableTypeErrors(getFunction(),
                                                    checker.diagnosticEmitter);
     }

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.h

@@ -52,6 +52,23 @@ class DiagnosticEmitter {
 
   bool emittedCheckerDoesntUnderstandDiagnostic = false;
 
+  /// This is incremented every time that the checker determines that an earlier
+  /// pass emitted a diagnostic while processing a mark_must_check. In such a
+  /// case, we want to suppress:
+  ///
+  /// 1. Emitting the compiler doesn't understand how to check error for the
+  ///    specific mark_must_check.
+  ///
+  /// 2. The "copy of noncopyable type" error over the entire function since us
+  ///    stopping processing at some point may have left copies.
+  ///
+  /// We use a counter rather than a boolean here so that a caller that is
+  /// processing an individual mark_must_check can determine if the checker
+  /// identified such an earlier pass diagnostic for the specific allocation so
+  /// that we can still emit "compiler doesn't understand" errors for other
+  /// allocations.
+  unsigned diagnosticEmittedByEarlierPassCount = 0;
+
 public:
   DiagnosticEmitter(SILFunction *inputFn) : fn(inputFn) {}
 
@@ -67,11 +84,37 @@ class DiagnosticEmitter {
     return *canonicalizer.get();
   }
 
+  /// Returns true if when processing any allocation in the current function:
+  ///
+  /// 1. This diagnostic emitter emitted a diagnostic.
+  /// 2. The user of the diagnostic emitter signaled to the diagnostic emitter
+  ///    that it detected an earlier diagnostic was emitted that prevented it
+  ///    from performing checking.
+  ///
+  /// DISCUSSION: This is used by the checker to decide whether or not it should
+  /// emit "found copy of a noncopyable type" error. If the checker emitted one
+  /// of these diagnostics, then the checker may have stopped processing early
+  /// and left copies since it was no longer able to check. In such a case, we
+  /// want the user to fix the pre-existing errors and re-run.
+  bool emittedDiagnostic() const {
+    return getDiagnosticCount() || getDiagnosticEmittedByEarlierPassCount();
+  }
+
   unsigned getDiagnosticCount() const { return diagnosticCount; }
+
   bool didEmitCheckerDoesntUnderstandDiagnostic() const {
     return emittedCheckerDoesntUnderstandDiagnostic;
   }
 
+  bool getDiagnosticEmittedByEarlierPassCount() const {
+    return diagnosticEmittedByEarlierPassCount;
+  }
+
+  void emitEarlierPassEmittedDiagnostic(MarkMustCheckInst *mmci) {
+    ++diagnosticEmittedByEarlierPassCount;
+    registerDiagnosticEmitted(mmci);
+  }
+
   /// Used at the end of the MoveOnlyAddressChecker to tell the user in a nice
   /// way to file a bug.
   void emitCheckedMissedCopyError(SILInstruction *copyInst);

test/SILGen/moveonly_escaping_closure.swift

@@ -618,7 +618,6 @@ func testConsumingEscapeClosureCaptureLet(_ f: consuming @escaping () -> ()) {
 var globalClosureCaptureInOut: () -> () = {}
 func testGlobalClosureCaptureInOut(_ x: inout SingleElt) {
     // expected-note @-1 {{'x' is declared 'inout'}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check}}
     globalClosureCaptureInOut = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
         borrowVal(x) // expected-note {{captured here}}
         consumeVal(x) // expected-note {{captured here}}
@@ -708,7 +707,6 @@ func testLocalLetClosureCaptureInOut(_ x: inout SingleElt) {
 // CHECK: } // end sil function '$s16moveonly_closure31testLocalVarClosureCaptureInOutyyAA9SingleEltVzFyycfU_'
 func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
     // expected-note @-1 {{'x' is declared 'inout'}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check}}
     var f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
         borrowVal(x) // expected-note {{captured here}}
         consumeVal(x) // expected-note {{captured here}}
@@ -757,7 +755,6 @@ func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
 // CHECK: } // end sil function '$s16moveonly_closure026testInOutVarClosureCapturedE0yyyycz_AA9SingleEltVztFyycfU_'
 func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt) {
     // expected-note @-1 {{'x' is declared 'inout'}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check}}
     f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
         borrowVal(x) // expected-note {{captured here}}
         consumeVal(x) // expected-note {{captured here}}
@@ -810,7 +807,6 @@ func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt)
 // CHECK: } // end sil function '$s16moveonly_closure38testConsumingEscapeClosureCaptureInOutyyyycn_AA9SingleEltVztFyycfU_'
 func testConsumingEscapeClosureCaptureInOut(_ f: consuming @escaping () -> (), _ x: inout SingleElt) {
     // expected-note @-1 {{'x' is declared 'inout'}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check}}
     f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
         borrowVal(x) // expected-note {{captured here}}
         consumeVal(x) // expected-note {{captured here}}

test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

@@ -3235,13 +3235,8 @@ public func closureVarCaptureClassUseAfterConsumeError() {
     let _ = x3
 }
 
-// TODO: The reason why we emit the moveonly type is that correctly the checker
-// realizes that the partial_apply has escaped, but does not understand that
-// this case has already been diagnosed properly by the escaping inout
-// diagnostic, so we shouldn't try to process it.
 public func closureVarCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
     // expected-note @-1 {{'x2' is declared 'inout'}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
     var f = {}
     f = {
         // expected-error @-1 {{escaping closure captures 'inout' parameter 'x2'}}

test/SILOptimizer/moveonly_objectchecker_diagnostics.swift

@@ -2956,10 +2956,8 @@ public func closureVarCaptureClassUseAfterConsume1(_ x: borrowing Klass) { // ex
     f()
 }
 
-// TODO: MG
 public func closureVarCaptureClassUseAfterConsume2(_ x2: inout Klass) {
     // expected-note @-1 {{'x2' is declared 'inout'}}
-    // expected-error @-2 {{Usage of a move only type that the move checker does not know how to check!}}
     var f = {}
     f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
         borrowVal(x2) // expected-note {{captured here}}