Further hardening: new compiler + old SDK still diagnose correctly

Author: ktoso

include/swift/AST/Decl.h

@@ -3884,6 +3884,8 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
   /// This method should be deprecated and removed
   /// Get the move-only `enqueue(Job)` protocol requirement function on the `Executor` protocol.
   AbstractFunctionDecl *getExecutorLegacyOwnedEnqueueFunction() const;
+  /// Get the move-only `enqueue(UnownedJob)` protocol requirement function on the `Executor` protocol.
+  AbstractFunctionDecl *getExecutorLegacyUnownedEnqueueFunction() const;
 
   /// Collect the set of protocols to which this type should implicitly
   /// conform, such as AnyObject (for classes).

lib/AST/Decl.cpp

@@ -5330,7 +5330,8 @@ NominalTypeDecl::getExecutorOwnedEnqueueFunction() const {
       if (params->size() != 1)
         continue;
 
-      if (params->get(0)->getSpecifier() == ParamSpecifier::LegacyOwned && // TODO: make this Consuming
+      if ((params->get(0)->getSpecifier() == ParamSpecifier::LegacyOwned ||
+           params->get(0)->getSpecifier() == ParamSpecifier::Consuming) &&
           params->get(0)->getInterfaceType()->isEqual(executorJobDecl->getDeclaredInterfaceType())) {
         return funcDecl;
       }
@@ -5368,7 +5369,8 @@ NominalTypeDecl::getExecutorLegacyOwnedEnqueueFunction() const {
       if (params->size() != 1)
         continue;
 
-      if (params->get(0)->getSpecifier() == ParamSpecifier::LegacyOwned && // TODO: make this Consuming
+      if ((params->get(0)->getSpecifier() == ParamSpecifier::LegacyOwned ||
+          params->get(0)->getSpecifier() == ParamSpecifier::Consuming) &&
           params->get(0)->getType()->isEqual(legacyJobDecl->getDeclaredInterfaceType())) {
         return funcDecl;
       }
@@ -5378,6 +5380,43 @@ NominalTypeDecl::getExecutorLegacyOwnedEnqueueFunction() const {
   return nullptr;
 }
 
+AbstractFunctionDecl *
+NominalTypeDecl::getExecutorLegacyUnownedEnqueueFunction() const {
+  auto &C = getASTContext();
+  StructDecl *unownedJobDecl = C.getUnownedJobDecl();
+  if (!unownedJobDecl)
+    return nullptr;
+
+  auto proto = dyn_cast<ProtocolDecl>(this);
+  if (!proto)
+    return nullptr;
+
+  llvm::SmallVector<ValueDecl *, 2> results;
+  lookupQualified(getSelfNominalTypeDecl(),
+                  DeclNameRef(C.Id_enqueue),
+                  NL_ProtocolMembers,
+                  results);
+
+  for (auto candidate: results) {
+    // we're specifically looking for the Executor protocol requirement
+    if (!isa<ProtocolDecl>(candidate->getDeclContext()))
+      continue;
+
+    if (auto *funcDecl = dyn_cast<AbstractFunctionDecl>(candidate)) {
+      auto params = funcDecl->getParameters();
+
+      if (params->size() != 1)
+        continue;
+
+      if (params->get(0)->getType()->isEqual(unownedJobDecl->getDeclaredInterfaceType())) {
+        return funcDecl;
+      }
+    }
+  }
+
+  return nullptr;
+}
+
 ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
                      ArrayRef<InheritedEntry> Inherited,
                      GenericParamList *GenericParams, DeclContext *Parent,

lib/Sema/TypeCheckConcurrency.cpp

@@ -1343,14 +1343,24 @@ void swift::tryDiagnoseExecutorConformance(ASTContext &C,
       (!legacyMoveOnlyEnqueueWitnessDecl || legacyMoveOnlyEnqueueWitnessDecl->getLoc().isInvalid())) {
     // 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.
-    auto ownedRequirement = C.getExecutorDecl()->getExecutorOwnedEnqueueFunction();
-    nominal->diagnose(diag::type_does_not_conform, nominalTy, proto->getDeclaredInterfaceType());
-    ownedRequirement->diagnose(diag::no_witnesses,
-                               getProtocolRequirementKind(ownedRequirement),
-                               ownedRequirement->getName(),
-                               ownedRequirement->getParameters()->get(0)->getInterfaceType(),
-                               /*AddFixIt=*/true);
-    return;
+    //
+    // If we're running against an SDK that does not have the ExecutorJob enqueue function,
+    // try to diagnose using the next-best one available.
+    auto missingRequirement = C.getExecutorDecl()->getExecutorOwnedEnqueueFunction();
+    if (!missingRequirement)
+      missingRequirement = C.getExecutorDecl()->getExecutorLegacyOwnedEnqueueFunction();
+    if (!missingRequirement)
+      missingRequirement = C.getExecutorDecl()->getExecutorLegacyUnownedEnqueueFunction();
+
+    if (missingRequirement) {
+      nominal->diagnose(diag::type_does_not_conform, nominalTy, proto->getDeclaredInterfaceType());
+      missingRequirement->diagnose(diag::no_witnesses,
+                                   getProtocolRequirementKind(missingRequirement),
+                                   missingRequirement->getName(),
+                                   missingRequirement->getParameters()->get(0)->getInterfaceType(),
+                                   /*AddFixIt=*/true);
+      return;
+    }
   }
 }
 

test/Concurrency/Runtime/custom_executors_tryDiagnoseExecutorConformance_with_sdk_typealias_struct_job_types.swift

@@ -0,0 +1,20 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-concurrency-typealias-struct-job) -typecheck -parse-as-library %s -verify
+
+// REQUIRES: concurrency
+// REQUIRES: libdispatch
+
+// rdar://106849189 move-only types should be supported in freestanding mode
+// UNSUPPORTED: freestanding
+
+// UNSUPPORTED: back_deployment_runtime
+// REQUIRES: concurrency_runtime
+
+import _Concurrency
+
+final class FakeExecutor1: SerialExecutor {
+  func enqueue(_ job: __owned ExecutorJob) {}
+}
+
+final class FakeExecutor2: SerialExecutor {
+  func enqueue(_ job: __owned Job) {}
+}

test/Inputs/clang-importer-sdk/swift-modules-concurrency-typealias-struct-job/_Concurrency.swift

@@ -0,0 +1,9 @@
+
+@_moveOnly
+public struct ExecutorJob {}
+
+public typealias Job = ExecutorJob
+
+public protocol SerialExecutor {
+  func enqueue(_ job: __owned ExecutorJob)
+}

test/lit.cfg

@@ -611,15 +611,22 @@ config.substitutions.append(('%build-clang-importer-objc-overlays',
 # FIXME: BEGIN -enable-source-import hackaround
 config.substitutions.append(('%clang-importer-sdk-concurrency-without-job-path',
                              '%r' % (make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk'))))
-
 config.substitutions.append(('%clang-importer-sdk-concurrency-without-job-nosource',
                              '-sdk %r' % (make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk'))))
 # FIXME: END -enable-source-import hackaround
-
 config.substitutions.append(('%clang-importer-sdk-concurrency-without-job',
                              '-enable-source-import -sdk %r -I %r ' % (make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk'),
                                                                        make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk', 'swift-modules-concurrency-without-job'))))
 
+# FIXME: BEGIN -enable-source-import hackaround
+config.substitutions.append(('%clang-importer-sdk-concurrency-typealias-struct-job-path',
+                             '%r' % (make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk'))))
+config.substitutions.append(('%clang-importer-sdk-concurrency-typealias-struct-job-nosource',
+                             '-sdk %r' % (make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk'))))
+# FIXME: END -enable-source-import hackaround
+config.substitutions.append(('%clang-importer-sdk-concurrency-typealias-struct-job',
+                             '-enable-source-import -sdk %r -I %r ' % (make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk'),
+                                                                       make_path(config.test_source_root, 'Inputs', 'clang-importer-sdk', 'swift-modules-concurrency-typealias-struct-job'))))
 
 # FIXME: BEGIN -enable-source-import hackaround
 config.substitutions.append(('%clang-importer-sdk-path',