Don't trigger warnings about stdlib itself when user code might trigger them

The standard library's enqueue() does not play by the same rules -- we
provide "deprecated" implementations in  order to remain source/binary
compatible, and showing warnings about this when users make mistake will
only be misleading.

Author: ktoso

include/swift/AST/DiagnosticsSema.def

@@ -6687,7 +6687,7 @@ WARNING(hashvalue_implementation,Deprecation,
         "conform type %0 to 'Hashable' by implementing 'hash(into:)' instead",
         (Type))
 
-WARNING(executor_enqueue_unowned_implementation,Deprecation,
+WARNING(executor_enqueue_deprecated_unowned_implementation,Deprecation,
         "'Executor.enqueue(UnownedJob)' is deprecated as a protocol requirement; "
         "conform type %0 to 'Executor' by implementing 'func enqueue(ExecutorJob)' instead",
         (Type))

lib/Sema/TypeCheckConcurrency.cpp

@@ -1282,7 +1282,7 @@ void swift::tryDiagnoseExecutorConformance(ASTContext &C,
   auto enqueueDeclName = DeclName(C, DeclBaseName(C.Id_enqueue), { Identifier() });
 
   FuncDecl *moveOnlyEnqueueRequirement = nullptr;
-  FuncDecl *legacyMoveOnlyEnqueueRequirement = nullptr; // TODO: preferably we'd want to remove handling of `enqueue(Job)` when able to
+  FuncDecl *legacyMoveOnlyEnqueueRequirement = nullptr;
   FuncDecl *unownedEnqueueRequirement = nullptr;
   for (auto req: proto->getProtocolRequirements()) {
     auto *funcDecl = dyn_cast<FuncDecl>(req);
@@ -1325,8 +1325,7 @@ void swift::tryDiagnoseExecutorConformance(ASTContext &C,
   assert(unownedEnqueueRequirement && "could not find the enqueue(UnownedJob) requirement, which should be always there");
 
   // try to find at least a single implementations of enqueue(_:)
-//  auto unownedEnqueueWitness = concreteConformance->getWitnessDeclRef(unownedEnqueueRequirement);
-  ValueDecl *unownedEnqueueWitnessDecl = concreteConformance->getWitnessDecl(unownedEnqueueRequirement); // unownedEnqueueWitness.getDecl();
+  ValueDecl *unownedEnqueueWitnessDecl = concreteConformance->getWitnessDecl(unownedEnqueueRequirement);
   ValueDecl *moveOnlyEnqueueWitnessDecl = nullptr;
   ValueDecl *legacyMoveOnlyEnqueueWitnessDecl = nullptr;
 
@@ -1359,39 +1358,57 @@ void swift::tryDiagnoseExecutorConformance(ASTContext &C,
     canRemoveOldDecls = declInfo.isContainedIn(requirementInfo);
   }
 
+  auto concurrencyModule = C.getLoadedModule(C.Id_Concurrency);
+  auto isStdlibDefaultImplDecl = [executorDecl, concurrencyModule](ValueDecl *witness) -> bool {
+    if (auto declContext = witness->getDeclContext()) {
+      if (auto *extension = dyn_cast<ExtensionDecl>(declContext)) {
+        auto extensionModule = extension->getParentModule();
+        if (extensionModule != concurrencyModule) {
+          return false;
+        }
+
+        if (auto extendedNominal = extension->getExtendedNominal()) {
+          return extendedNominal->getDeclaredInterfaceType()->isEqual(
+              executorDecl->getDeclaredInterfaceType());
+        }
+      }
+    }
+    return false;
+  };
+
   // If both old and new enqueue are implemented, but the old one cannot be removed,
   // emit a warning that the new enqueue is unused.
   if (!canRemoveOldDecls && unownedEnqueueWitnessDecl && moveOnlyEnqueueWitnessDecl) {
-    diags.diagnose(moveOnlyEnqueueWitnessDecl->getLoc(), diag::executor_enqueue_unused_implementation);
+    if (!isStdlibDefaultImplDecl(moveOnlyEnqueueWitnessDecl)) {
+      diags.diagnose(moveOnlyEnqueueWitnessDecl->getLoc(),
+                     diag::executor_enqueue_unused_implementation);
+    }
   }
 
   // Old UnownedJob based impl is present, warn about it suggesting the new protocol requirement.
   if (canRemoveOldDecls && unownedEnqueueWitnessDecl) {
-    diags.diagnose(unownedEnqueueWitnessDecl->getLoc(), diag::executor_enqueue_unowned_implementation, nominalTy);
+    if (!isStdlibDefaultImplDecl(unownedEnqueueWitnessDecl)) {
+      diags.diagnose(unownedEnqueueWitnessDecl->getLoc(),
+                     diag::executor_enqueue_deprecated_unowned_implementation,
+                     nominalTy);
+    }
   }
   // Old Job based impl is present, warn about it suggesting the new protocol requirement.
   if (legacyMoveOnlyEnqueueWitnessDecl) {
-    diags.diagnose(legacyMoveOnlyEnqueueWitnessDecl->getLoc(), diag::executor_enqueue_deprecated_owned_job_implementation, nominalTy);
-  }
-
-  auto isStdlibDefaultImplDecl = [executorDecl](ValueDecl *witness) -> bool {
-    if (auto declContext = witness->getDeclContext()) {
-      if (auto *extension = dyn_cast<ExtensionDecl>(declContext)) {
-        if (auto extendedNominal = extension->getExtendedNominal()) {
-          return extendedNominal->getDeclaredInterfaceType()->isEqual(
-                  executorDecl->getDeclaredInterfaceType());
-        }
-      }
+    if (!isStdlibDefaultImplDecl(legacyMoveOnlyEnqueueWitnessDecl)) {
+      diags.diagnose(legacyMoveOnlyEnqueueWitnessDecl->getLoc(),
+                     diag::executor_enqueue_deprecated_owned_job_implementation,
+                     nominalTy);
     }
-    return false;
-  };
-  auto missingWitness = !unownedEnqueueWitnessDecl &&
-                        !moveOnlyEnqueueWitnessDecl &&
-                        !legacyMoveOnlyEnqueueWitnessDecl;
+  }
 
   bool unownedEnqueueWitnessIsDefaultImpl = isStdlibDefaultImplDecl(unownedEnqueueWitnessDecl);
   bool moveOnlyEnqueueWitnessIsDefaultImpl = isStdlibDefaultImplDecl(moveOnlyEnqueueWitnessDecl);
   bool legacyMoveOnlyEnqueueWitnessDeclIsDefaultImpl = isStdlibDefaultImplDecl(legacyMoveOnlyEnqueueWitnessDecl);
+
+  auto missingWitness = !unownedEnqueueWitnessDecl &&
+                        !moveOnlyEnqueueWitnessDecl &&
+                        !legacyMoveOnlyEnqueueWitnessDecl;
   auto allWitnessesAreDefaultImpls = unownedEnqueueWitnessIsDefaultImpl &&
                                      moveOnlyEnqueueWitnessIsDefaultImpl &&
                                      legacyMoveOnlyEnqueueWitnessDeclIsDefaultImpl;

test/Concurrency/custom_executor_enqueue_deprecation_on_executor_extension.swift

@@ -0,0 +1,22 @@
+// RUN: %target-typecheck-verify-swift -disable-availability-checking
+// REQUIRES: concurrency
+
+// rdar://106849189 move-only types should be supported in freestanding mode
+// UNSUPPORTED: freestanding
+
+
+extension Executor {
+  func enqueue(_ job: UnownedJob) { // expected-warning{{'Executor.enqueue(UnownedJob)' is deprecated as a protocol requirement; conform type 'NoneExecutor' to 'Executor' by implementing 'func enqueue(ExecutorJob)' instead}}
+    fatalError()
+  }
+}
+
+final class NoneExecutor: SerialExecutor {
+
+  // the enqueue from the extension is properly picked up,
+  // even though we do warn about deprecation on it in the extension.
+
+  func asUnownedSerialExecutor() -> UnownedSerialExecutor {
+    UnownedSerialExecutor(ordinary: self)
+  }
+}

test/Concurrency/custom_executor_enqueue_impls.swift

@@ -1,12 +1,9 @@
-// RUN: %target-typecheck-verify-swift -disable-availability-checking -verify-ignore-unknown
+// RUN: %target-typecheck-verify-swift -disable-availability-checking
 // REQUIRES: concurrency
 
 // rdar://106849189 move-only types should be supported in freestanding mode
 // UNSUPPORTED: freestanding
 
-// FIXME: rdar://107112715 test failing on iOS simulator, investigating
-// UNSUPPORTED: OS=ios
-
 // Such type may be encountered since Swift 5.5 (5.1 backdeployed) if someone implemented the
 // not documented, but public Executor types back then already.
 //