[Isolated conformances] Diagnose conflict with isolated conformances and Sendable

DougGregor · DougGregor · commit ffca6968eddf · 2025-02-28T13:08:35.000-08:00
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -2722,23 +2722,8 @@ WARNING(add_predates_concurrency_import,none,
 GROUPED_WARNING(remove_predates_concurrency_import,PreconcurrencyImport,
                 DefaultIgnore,
                 "'@preconcurrency' attribute on module %0 has no effect", (Identifier))
-NOTE(add_isolated_to_conformance,none,
-     "add 'isolated' to the %0 conformance to restrict it to %1 code",
-    (DeclName, ActorIsolation))
 NOTE(add_preconcurrency_to_conformance,none,
      "add '@preconcurrency' to the %0 conformance to defer isolation checking to run time", (DeclName))
-ERROR(isolated_conformance_not_global_actor_isolated,none,
-      "isolated conformance is only permitted on global-actor-isolated types",
-      ())
-ERROR(isolated_conformance_experimental_feature,none,
-      "isolated conformances require experimental feature "
-      " 'IsolatedConformances'", ())
-ERROR(nonisolated_conformance_depends_on_isolated_conformance,none,
-      "conformance of %0 to %1 depends on %2 conformance of %3 to %4; mark it as 'isolated'",
-      (Type, DeclName, ActorIsolation, Type, DeclName))
-ERROR(isolated_conformance_mismatch_with_associated_isolation,none,
-      "%0 conformance of %1 to %2 cannot depend on %3 conformance of %4 to %5",
-      (ActorIsolation, Type, DeclName, ActorIsolation, Type, DeclName))
 WARNING(remove_public_import,none,
         "public import of %0 was not used in public declarations or inlinable code",
         (Identifier))
@@ -8309,6 +8294,29 @@ ERROR(attr_abi_incompatible_with_silgen_name,none,
       "the same purpose",
       (DescriptiveDeclKind))
 
+//===----------------------------------------------------------------------===//
+// MARK: Isolated conformances
+//===----------------------------------------------------------------------===//
+ERROR(isolated_conformance_not_global_actor_isolated,none,
+      "isolated conformance is only permitted on global-actor-isolated types",
+      ())
+ERROR(isolated_conformance_experimental_feature,none,
+      "isolated conformances require experimental feature "
+      " 'IsolatedConformances'", ())
+ERROR(nonisolated_conformance_depends_on_isolated_conformance,none,
+      "conformance of %0 to %1 depends on %2 conformance of %3 to %4; mark it as 'isolated'",
+      (Type, DeclName, ActorIsolation, Type, DeclName))
+ERROR(isolated_conformance_mismatch_with_associated_isolation,none,
+      "%0 conformance of %1 to %2 cannot depend on %3 conformance of %4 to %5",
+      (ActorIsolation, Type, DeclName, ActorIsolation, Type, DeclName))
+NOTE(add_isolated_to_conformance,none,
+     "add 'isolated' to the %0 conformance to restrict it to %1 code",
+    (DeclName, ActorIsolation))
+ERROR(isolated_conformance_with_sendable,none,
+      "isolated conformance of %0 to %1 cannot be used to satisfy conformance "
+      "requirement for a %select{`Sendable`|`SendableMetatype`}2 type "
+      "parameter %3", (Type, DeclName, bool, Type))
+
 //===----------------------------------------------------------------------===//
 // MARK: @execution Attribute
 //===----------------------------------------------------------------------===//
diff --git a/include/swift/AST/ProtocolConformanceRef.h b/include/swift/AST/ProtocolConformanceRef.h
@@ -140,6 +140,14 @@ class ProtocolConformanceRef {
   bool forEachMissingConformance(
       llvm::function_ref<bool(BuiltinProtocolConformance *missing)> fn) const;
 
+  /// Enumerate all of the isolated conformances in the given conformance.
+  ///
+  /// The given `body` will be called on each isolated conformance. If it ever
+  /// returns `true`, this function will abort the search and return `true`.
+  bool forEachIsolatedConformance(
+      llvm::function_ref<bool(ProtocolConformance*)> body
+  ) const;
+
   using OpaqueValue = void*;
   OpaqueValue getOpaqueValue() const { return Union.getOpaqueValue(); }
   static ProtocolConformanceRef getFromOpaqueValue(OpaqueValue value) {
diff --git a/include/swift/AST/Requirement.h b/include/swift/AST/Requirement.h
@@ -187,9 +187,13 @@ class Requirement {
   /// \param subReqs An out parameter initialized to a list of simpler
   /// requirements which the caller must check to ensure this
   /// requirement is completely satisfied.
+  /// \param isolatedConformances If non-NULL, will be provided with all of the
+  /// isolated conformances that
   CheckRequirementResult checkRequirement(
       SmallVectorImpl<Requirement> &subReqs,
-      bool allowMissing = false) const;
+      bool allowMissing = false,
+      SmallVectorImpl<ProtocolConformance *> *isolatedConformances = nullptr
+  ) const;
 
   /// Determines if this substituted requirement can ever be satisfied,
   /// possibly with additional substitutions.
diff --git a/lib/AST/ProtocolConformanceRef.cpp b/lib/AST/ProtocolConformanceRef.cpp
@@ -386,6 +386,42 @@ bool ProtocolConformanceRef::forEachMissingConformance(
   return false;
 }
 
+bool ProtocolConformanceRef::forEachIsolatedConformance(
+    llvm::function_ref<bool(ProtocolConformance*)> body
+) const {
+  if (isInvalid() || isAbstract())
+    return false;
+
+  if (isPack()) {
+    auto pack = getPack()->getPatternConformances();
+    for (auto conformance : pack) {
+      if (conformance.forEachIsolatedConformance(body))
+        return true;
+    }
+
+    return false;
+  }
+
+  // Is this an isolated conformance?
+  auto concrete = getConcrete();
+  if (auto normal =
+          dyn_cast<NormalProtocolConformance>(concrete->getRootConformance())) {
+    if (normal->isIsolated()) {
+      if (body(concrete))
+        return true;
+    }
+  }
+
+  // Check conformances that are part of this conformance.
+  auto subMap = concrete->getSubstitutionMap();
+  for (auto conformance : subMap.getConformances()) {
+    if (conformance.forEachIsolatedConformance(body))
+      return true;
+  }
+
+  return false;
+}
+
 void swift::simple_display(llvm::raw_ostream &out, ProtocolConformanceRef conformanceRef) {
   if (conformanceRef.isAbstract()) {
     simple_display(out, conformanceRef.getAbstract());
diff --git a/lib/AST/Requirement.cpp b/lib/AST/Requirement.cpp
@@ -82,7 +82,9 @@ ProtocolDecl *Requirement::getProtocolDecl() const {
 
 CheckRequirementResult Requirement::checkRequirement(
     SmallVectorImpl<Requirement> &subReqs,
-    bool allowMissing) const {
+    bool allowMissing,
+    SmallVectorImpl<ProtocolConformance *> *isolatedConformances
+) const {
   if (hasError())
     return CheckRequirementResult::SubstitutionFailure;
 
@@ -122,6 +124,15 @@ CheckRequirementResult Requirement::checkRequirement(
     if (!conformance)
       return CheckRequirementResult::RequirementFailure;
 
+    // Collect isolated conformances.
+    if (isolatedConformances) {
+      conformance.forEachIsolatedConformance(
+          [&](ProtocolConformance *isolatedConformance) {
+            isolatedConformances->push_back(isolatedConformance);
+            return false;
+          });
+    }
+
     auto condReqs = conformance.getConditionalRequirements();
     if (condReqs.empty())
       return CheckRequirementResult::Success;
diff --git a/lib/AST/RequirementMachine/RequirementLowering.cpp b/lib/AST/RequirementMachine/RequirementLowering.cpp
@@ -300,7 +300,7 @@ static void desugarSuperclassRequirement(
 
   SmallVector<Requirement, 2> subReqs;
 
-  switch (req.checkRequirement(subReqs)) {
+  switch (req.checkRequirement(subReqs, /*allowMissing=*/false)) {
   case CheckRequirementResult::Success:
   case CheckRequirementResult::PackRequirement:
     break;
@@ -333,7 +333,7 @@ static void desugarLayoutRequirement(
 
   SmallVector<Requirement, 2> subReqs;
 
-  switch (req.checkRequirement(subReqs)) {
+  switch (req.checkRequirement(subReqs, /*allowMissing=*/false)) {
   case CheckRequirementResult::Success:
   case CheckRequirementResult::PackRequirement:
     break;
diff --git a/lib/IDE/IDETypeChecking.cpp b/lib/IDE/IDETypeChecking.cpp
@@ -363,7 +363,7 @@ struct SynthesizedExtensionAnalyzer::Implementation {
 
         while (!subReqs.empty()) {
           auto req = subReqs.pop_back_val();
-          switch (req.checkRequirement(subReqs)) {
+          switch (req.checkRequirement(subReqs, /*allowMissing=*/false)) {
           case CheckRequirementResult::Success:
           case CheckRequirementResult::PackRequirement:
           case CheckRequirementResult::ConditionalConformance:
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -7669,43 +7669,6 @@ bool swift::diagnoseNonSendableFromDeinit(
                                 var->getDescriptiveKind(), var->getName());
 }
 
-bool swift::forEachIsolatedConformance(
-    ProtocolConformanceRef conformance,
-    llvm::function_ref<bool(ProtocolConformance*)> body
-) {
-  if (conformance.isInvalid() || conformance.isAbstract())
-    return false;
-
-  if (conformance.isPack()) {
-    auto pack = conformance.getPack()->getPatternConformances();
-    for (auto conformance : pack) {
-      if (forEachIsolatedConformance(conformance, body))
-        return true;
-    }
-
-    return false;
-  }
-
-  // Is this an isolated conformance?
-  auto concrete = conformance.getConcrete();
-  if (auto normal =
-          dyn_cast<NormalProtocolConformance>(concrete->getRootConformance())) {
-    if (normal->isIsolated()) {
-      if (body(concrete))
-        return true;
-    }
-  }
-
-  // Check conformances that are part of this conformance.
-  auto subMap = concrete->getSubstitutionMap();
-  for (auto conformance : subMap.getConformances()) {
-    if (forEachIsolatedConformance(conformance, body))
-      return true;
-  }
-
-  return false;
-}
-
 ActorIsolation swift::getConformanceIsolation(ProtocolConformance *conformance) {
   auto rootNormal =
       dyn_cast<NormalProtocolConformance>(conformance->getRootConformance());
diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h
@@ -699,15 +699,6 @@ void introduceUnsafeInheritExecutorReplacements(
 void introduceUnsafeInheritExecutorReplacements(
     const DeclContext *dc, Type base, SourceLoc loc, LookupResult &result);
 
-/// Enumerate all of the isolated conformances in the given conformance.
-/// 
-/// The given `body` will be called on each isolated conformance. If it ever
-/// returns `true`, this function will abort the search and return `true`.
-bool forEachIsolatedConformance(
-    ProtocolConformanceRef conformance, 
-    llvm::function_ref<bool(ProtocolConformance*)> body
-);
-
 /// Determine the isolation of the given conformance. This only applies to
 /// the immediate conformance, not any conformances on which it depends.
 ActorIsolation getConformanceIsolation(ProtocolConformance *conformance);
diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp
@@ -905,7 +905,7 @@ void TypeChecker::diagnoseRequirementFailure(
   const auto &req = reqFailureInfo.Req;
   const auto &substReq = reqFailureInfo.SubstReq;
 
-  Diag<Type, Type, Type> diagnostic;
+  std::optional<Diag<Type, Type, Type>> diagnostic;
   Diag<Type, Type, StringRef> diagnosticNote;
 
   const auto reqKind = req.getKind();
@@ -923,6 +923,24 @@ void TypeChecker::diagnoseRequirementFailure(
     break;
 
   case RequirementKind::Conformance: {
+    // If this was a failure due to isolated conformances conflicting with
+    // a Sendable or MetatypeSendable requirement, diagnose that.
+    if (reqFailureInfo.IsolatedConformanceProto) {
+      ASTContext &ctx =
+          reqFailureInfo.IsolatedConformanceProto->getASTContext();
+      auto isolatedConformance = reqFailureInfo.IsolatedConformances.front();
+      ctx.Diags.diagnose(
+          errorLoc, diag::isolated_conformance_with_sendable,
+          isolatedConformance->getType(),
+          isolatedConformance->getProtocol()->getName(),
+          reqFailureInfo
+            .IsolatedConformanceProto->isSpecificProtocol(
+              KnownProtocolKind::SendableMetatype),
+          req.getFirstType());
+      diagnosticNote = diag::type_does_not_inherit_or_conform_requirement;
+      break;
+    }
+
     diagnoseConformanceFailure(substReq.getFirstType(),
                                substReq.getProtocolDecl(), nullptr, errorLoc);
 
@@ -951,9 +969,12 @@ void TypeChecker::diagnoseRequirementFailure(
   }
 
   ASTContext &ctx = targetTy->getASTContext();
-  // FIXME: Poor source-location information.
-  ctx.Diags.diagnose(errorLoc, diagnostic, targetTy, substReq.getFirstType(),
-                     substSecondTy);
+
+  if (diagnostic) {
+    // FIXME: Poor source-location information.
+    ctx.Diags.diagnose(errorLoc, *diagnostic, targetTy, substReq.getFirstType(),
+                       substSecondTy);
+  }
 
   const auto genericParamBindingsText = gatherGenericParamBindingsText(
       {req.getFirstType(), secondTy}, genericParams, substitutions);
@@ -966,6 +987,7 @@ void TypeChecker::diagnoseRequirementFailure(
 }
 
 CheckGenericArgumentsResult TypeChecker::checkGenericArgumentsForDiagnostics(
+    GenericSignature signature,
     ArrayRef<Requirement> requirements,
     TypeSubstitutionFn substitutions) {
   using ParentConditionalConformances =
@@ -1004,7 +1026,9 @@ CheckGenericArgumentsResult TypeChecker::checkGenericArgumentsForDiagnostics(
     auto substReq = item.SubstReq;
 
     SmallVector<Requirement, 2> subReqs;
-    switch (substReq.checkRequirement(subReqs, /*allowMissing=*/true)) {
+    SmallVector<ProtocolConformance *, 2> isolatedConformances;
+    switch (substReq.checkRequirement(subReqs, /*allowMissing=*/true,
+                                      &isolatedConformances)) {
     case CheckRequirementResult::Success:
       break;
 
@@ -1036,6 +1060,38 @@ CheckGenericArgumentsResult TypeChecker::checkGenericArgumentsForDiagnostics(
       hadSubstFailure = true;
       break;
     }
+
+    if (!isolatedConformances.empty()) {
+      // Dig out the original type parameter for the requirement.
+      // FIXME: req might not be the right pre-substituted requirement,
+      // if this came from a conditional requirement.
+      auto firstType = req.getFirstType();
+
+      // An isolated conformance cannot be used in a context where the type
+      // parameter can escape the isolation domain in which the conformance
+      // was formed. To establish this, we look for Sendable or SendableMetatype
+      // requirements on the type parameter itself.
+      ASTContext &ctx = firstType->getASTContext();
+      auto sendableProto = ctx.getProtocol(KnownProtocolKind::Sendable);
+      auto sendableMetatypeProto =
+          ctx.getProtocol(KnownProtocolKind::SendableMetatype);
+      if (firstType->isTypeParameter()) {
+        std::optional<ProtocolDecl *> failedProtocol;
+        if (sendableProto &&
+            signature->requiresProtocol(firstType, sendableProto))
+          failedProtocol = sendableProto;
+        else if (sendableMetatypeProto &&
+                 signature->requiresProtocol(firstType, sendableMetatypeProto))
+          failedProtocol = sendableMetatypeProto;
+
+        if (failedProtocol) {
+          return CheckGenericArgumentsResult::createIsolatedConformanceFailure(
+            req, substReq,
+            TinyPtrVector<ProtocolConformance *>(isolatedConformances),
+            *failedProtocol);
+        }
+      }
+    }
   }
 
   if (hadSubstFailure)
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -5100,6 +5100,7 @@ static void ensureRequirementsAreSatisfied(ASTContext &ctx,
   }
 
   const auto result = TypeChecker::checkGenericArgumentsForDiagnostics(
+      proto->getGenericSignature(),
       reqSig, QuerySubstitutionMap{substitutions});
   switch (result.getKind()) {
   case CheckRequirementsResult::Success:
@@ -5233,8 +5234,8 @@ static void ensureRequirementsAreSatisfied(ASTContext &ctx,
       }
 
       if (!diagnosedIsolatedConformanceIssue) {
-        bool foundIssue = forEachIsolatedConformance(
-            ProtocolConformanceRef(assocConf),
+        bool foundIssue = ProtocolConformanceRef(assocConf)
+          .forEachIsolatedConformance(
             [&](ProtocolConformance *isolatedConformance) {
               // If the conformance we're checking isn't isolated at all, it
               // needs "isolated".
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
diff --git a/test/Concurrency/isolated_conformance.swift b/test/Concurrency/isolated_conformance.swift
