[Executors ]dont use loc validity to determine if we should diagnose

instead, error if all implementations are "default from stdlib" because end-user MUST implement at least one of them

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

@@ -6646,7 +6646,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

@@ -1283,12 +1283,13 @@ void swift::tryDiagnoseExecutorConformance(ASTContext &C,
   auto &diags = C.Diags;
   auto module = nominal->getParentModule();
   Type nominalTy = nominal->getDeclaredInterfaceType();
+  NominalTypeDecl *executorDecl = C.getExecutorDecl();
 
   // enqueue(_:)
   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);
@@ -1331,18 +1332,17 @@ 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(_:)
-  ConcreteDeclRef unownedEnqueueWitness = concreteConformance->getWitnessDeclRef(unownedEnqueueRequirement);
-  ValueDecl *unownedEnqueueWitnessDecl = unownedEnqueueWitness.getDecl();
+  ValueDecl *unownedEnqueueWitnessDecl = concreteConformance->getWitnessDecl(unownedEnqueueRequirement);
   ValueDecl *moveOnlyEnqueueWitnessDecl = nullptr;
   ValueDecl *legacyMoveOnlyEnqueueWitnessDecl = nullptr;
 
   if (moveOnlyEnqueueRequirement) {
-    moveOnlyEnqueueWitnessDecl = concreteConformance->getWitnessDeclRef(
-        moveOnlyEnqueueRequirement).getDecl();
+    moveOnlyEnqueueWitnessDecl = concreteConformance->getWitnessDecl(
+        moveOnlyEnqueueRequirement);
   }
   if (legacyMoveOnlyEnqueueRequirement) {
-    legacyMoveOnlyEnqueueWitnessDecl = concreteConformance->getWitnessDeclRef(
-        legacyMoveOnlyEnqueueRequirement).getDecl();
+    legacyMoveOnlyEnqueueWitnessDecl = concreteConformance->getWitnessDecl(
+        legacyMoveOnlyEnqueueRequirement);
   }
 
   // --- Diagnose warnings and errors
@@ -1365,26 +1365,62 @@ 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 && unownedEnqueueWitnessDecl->getLoc().isValid() &&
-      moveOnlyEnqueueWitnessDecl && moveOnlyEnqueueWitnessDecl->getLoc().isValid()) {
-    diags.diagnose(moveOnlyEnqueueWitnessDecl->getLoc(), diag::executor_enqueue_unused_implementation);
+  if (!canRemoveOldDecls && unownedEnqueueWitnessDecl && moveOnlyEnqueueWitnessDecl) {
+    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 && unownedEnqueueWitnessDecl->getLoc().isValid()) {
-    diags.diagnose(unownedEnqueueWitnessDecl->getLoc(), diag::executor_enqueue_unowned_implementation, nominalTy);
+  if (canRemoveOldDecls && unownedEnqueueWitnessDecl) {
+    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 && legacyMoveOnlyEnqueueWitnessDecl->getLoc().isValid()) {
-    diags.diagnose(legacyMoveOnlyEnqueueWitnessDecl->getLoc(), diag::executor_enqueue_deprecated_owned_job_implementation, nominalTy);
+  if (legacyMoveOnlyEnqueueWitnessDecl) {
+    if (!isStdlibDefaultImplDecl(legacyMoveOnlyEnqueueWitnessDecl)) {
+      diags.diagnose(legacyMoveOnlyEnqueueWitnessDecl->getLoc(),
+                     diag::executor_enqueue_deprecated_owned_job_implementation,
+                     nominalTy);
+    }
   }
 
-  if ((!unownedEnqueueWitnessDecl || unownedEnqueueWitnessDecl->getLoc().isInvalid()) &&
-      (!moveOnlyEnqueueWitnessDecl || moveOnlyEnqueueWitnessDecl->getLoc().isInvalid()) &&
-      (!legacyMoveOnlyEnqueueWitnessDecl || legacyMoveOnlyEnqueueWitnessDecl->getLoc().isInvalid())) {
+  bool unownedEnqueueWitnessIsDefaultImpl = isStdlibDefaultImplDecl(unownedEnqueueWitnessDecl);
+  bool moveOnlyEnqueueWitnessIsDefaultImpl = isStdlibDefaultImplDecl(moveOnlyEnqueueWitnessDecl);
+  bool legacyMoveOnlyEnqueueWitnessDeclIsDefaultImpl = isStdlibDefaultImplDecl(legacyMoveOnlyEnqueueWitnessDecl);
+
+  auto missingWitness = !unownedEnqueueWitnessDecl &&
+                        !moveOnlyEnqueueWitnessDecl &&
+                        !legacyMoveOnlyEnqueueWitnessDecl;
+  auto allWitnessesAreDefaultImpls = unownedEnqueueWitnessIsDefaultImpl &&
+                                     moveOnlyEnqueueWitnessIsDefaultImpl &&
+                                     legacyMoveOnlyEnqueueWitnessDeclIsDefaultImpl;
+  if ((missingWitness) ||
+      (!missingWitness && allWitnessesAreDefaultImpls)) {
     // Neither old nor new implementation have been found, but we provide default impls for them
     // that are mutually recursive, so we must error and suggest implementing the right requirement.
     //

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 -enable-experimental-move-only -disable-availability-checking
+// 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.
 //