Merge pull request #81296 from DougGregor/infer-nonisolated-conformances-from-witnesses

[SE-0470] Prohibit inference of isolated conformances with nonisolated witnesses

Author: DougGregor

include/swift/AST/Evaluator.h

@@ -276,9 +276,7 @@ class Evaluator {
            typename std::enable_if<Request::hasSplitCache>::type* = nullptr>
   void cacheNonEmptyOutput(const Request &request,
                            typename Request::OutputType &&output) {
-    bool inserted = cache.insert<Request>(request, std::move(output));
-    assert(inserted && "Request result was already cached");
-    (void) inserted;
+    (void)cache.insert<Request>(request, std::move(output));
   }
 
   /// Consults the request evaluator's cache for a split-cached request.

include/swift/AST/ProtocolConformance.h

@@ -131,6 +131,8 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   /// conformance definition.
   Type ConformingType;
 
+  friend class ConformanceIsolationRequest;
+
 protected:
   // clang-format off
   //
@@ -139,9 +141,13 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   union { uint64_t OpaqueBits;
 
     SWIFT_INLINE_BITFIELD_BASE(ProtocolConformance,
+                               1+
                                bitmax(NumProtocolConformanceKindBits, 8),
       /// The kind of protocol conformance.
-      Kind : bitmax(NumProtocolConformanceKindBits, 8)
+      Kind : bitmax(NumProtocolConformanceKindBits, 8),
+
+      /// Whether the computed actor isolation is nonisolated.
+      IsComputedNonisolated : 1
     );
 
     SWIFT_INLINE_BITFIELD_EMPTY(RootProtocolConformance, ProtocolConformance);
@@ -161,9 +167,6 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
       /// this conformance.
       IsPreconcurrencyEffectful : 1,
 
-      /// Whether the computed actor isolation is nonisolated.
-      IsComputedNonisolated : 1,
-
       /// Whether there is an explicit global actor specified for this
       /// conformance.
       HasExplicitGlobalActor : 1,
@@ -198,6 +201,15 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   ProtocolConformance(ProtocolConformanceKind kind, Type conformingType)
     : ConformingType(conformingType) {
     Bits.ProtocolConformance.Kind = unsigned(kind);
+    Bits.ProtocolConformance.IsComputedNonisolated = false;
+  }
+
+  bool isComputedNonisolated() const {
+    return Bits.ProtocolConformance.IsComputedNonisolated;
+  }
+
+  void setComputedNonnisolated(bool value = true) {
+    Bits.ProtocolConformance.IsComputedNonisolated = value;
   }
 
 public:
@@ -587,14 +599,6 @@ class NormalProtocolConformance : public RootProtocolConformance,
   // Record the explicitly-specified global actor isolation.
   void setExplicitGlobalActorIsolation(TypeExpr *typeExpr);
 
-  bool isComputedNonisolated() const {
-    return Bits.NormalProtocolConformance.IsComputedNonisolated;
-  }
-
-  void setComputedNonnisolated(bool value = true) {
-    Bits.NormalProtocolConformance.IsComputedNonisolated = value;
-  }
-
 public:
   NormalProtocolConformance(Type conformingType, ProtocolDecl *protocol,
                             SourceLoc loc, DeclContext *dc,
@@ -618,7 +622,6 @@ class NormalProtocolConformance : public RootProtocolConformance,
     Bits.NormalProtocolConformance.HasComputedAssociatedConformances = false;
     Bits.NormalProtocolConformance.SourceKind =
         unsigned(ConformanceEntryKind::Explicit);
-    Bits.NormalProtocolConformance.IsComputedNonisolated = false;
     Bits.NormalProtocolConformance.HasExplicitGlobalActor = false;
     setExplicitGlobalActorIsolation(options.getGlobalActorIsolationType());
   }

lib/AST/TypeCheckRequests.cpp

@@ -1373,31 +1373,25 @@ ConformanceIsolationRequest::getCachedResult() const {
   // everything else, which is nearly every conformance, this request quickly
   // returns "nonisolated" so there is no point in caching it.
   auto conformance = std::get<0>(getStorage());
-  auto rootNormal =
-      dyn_cast<NormalProtocolConformance>(conformance->getRootConformance());
-  if (!rootNormal)
-    return ActorIsolation::forNonisolated(false);
 
   // Was actor isolation non-isolated?
-  if (rootNormal->isComputedNonisolated())
+  if (conformance->isComputedNonisolated())
     return ActorIsolation::forNonisolated(false);
 
-  ASTContext &ctx = rootNormal->getDeclContext()->getASTContext();
+  ASTContext &ctx = conformance->getDeclContext()->getASTContext();
   return ctx.evaluator.getCachedNonEmptyOutput(*this);
 }
 
 void ConformanceIsolationRequest::cacheResult(ActorIsolation result) const {
   auto conformance = std::get<0>(getStorage());
-  auto rootNormal =
-      cast<NormalProtocolConformance>(conformance->getRootConformance());
 
   // Common case: conformance is nonisolated.
   if (result.isNonisolated()) {
-    rootNormal->setComputedNonnisolated();
+    conformance->setComputedNonnisolated();
     return;
   }
 
-  ASTContext &ctx = rootNormal->getDeclContext()->getASTContext();
+  ASTContext &ctx = conformance->getDeclContext()->getASTContext();
   ctx.evaluator.cacheNonEmptyOutput(*this, std::move(result));
 }
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -7910,6 +7910,9 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
   if (!rootNormal)
     return ActorIsolation::forNonisolated(false);
 
+  if (conformance != rootNormal)
+    return rootNormal->getIsolation();
+
   // If the conformance is explicitly non-isolated, report that.
   if (rootNormal->getOptions().contains(ProtocolConformanceFlags::Nonisolated))
     return ActorIsolation::forNonisolated(false);
@@ -7947,17 +7950,55 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
   if (getActorIsolation(rootNormal->getProtocol()).isActorIsolated())
     return ActorIsolation::forNonisolated(false);
 
-  // If we are inferring isolated conformances and the conforming type is
-  // isolated to a global actor, use the conforming type's isolation.
+  // @preconcurrency disables isolation inference.
+  if (rootNormal->isPreconcurrency())
+    return ActorIsolation::forNonisolated(false);
+
+  // Isolation inference rules follow. If we aren't inferring isolated conformances,
+  // we're done.
+  if (!ctx.LangOpts.hasFeature(Feature::InferIsolatedConformances))
+    return ActorIsolation::forNonisolated(false);
+
   auto nominal = dc->getSelfNominalTypeDecl();
-  if (ctx.LangOpts.hasFeature(Feature::InferIsolatedConformances) &&
-      nominal) {
-    auto nominalIsolation = getActorIsolation(nominal);
-    if (nominalIsolation.isGlobalActor())
-      return nominalIsolation;
+  if (!nominal) {
+    return ActorIsolation::forNonisolated(false);
+  }
+
+  // If we are inferring isolated conformances and the conforming type is
+  // isolated to a global actor, we may use the conforming type's isolation.
+  auto nominalIsolation = getActorIsolation(nominal);
+  if (!nominalIsolation.isGlobalActor()) {
+    return ActorIsolation::forNonisolated(false);
+  }
+
+  // If all of the value witnesses are nonisolated, then we should not infer
+  // global actor isolation.
+  bool anyIsolatedWitness = false;
+  auto protocol = conformance->getProtocol();
+  for (auto requirement : protocol->getMembers()) {
+    if (isa<TypeDecl>(requirement))
+      continue;
+
+    auto valueReq = dyn_cast<ValueDecl>(requirement);
+    if (!valueReq)
+      continue;
+
+    auto witness = conformance->getWitnessDecl(valueReq);
+    if (!witness)
+      continue;
+
+    auto witnessIsolation = getActorIsolation(witness);
+    if (witnessIsolation.isActorIsolated()) {
+      anyIsolatedWitness = true;
+      break;
+    }
+  }
+
+  if (!anyIsolatedWitness) {
+    return ActorIsolation::forNonisolated(false);
   }
 
-  return ActorIsolation::forNonisolated(false);
+  return nominalIsolation;
 }
 
 namespace {

test/Concurrency/isolated_conformance_default_actor.swift

@@ -46,8 +46,8 @@ func acceptSendablePMeta<T: Sendable & P>(_: T.Type) { }
 func acceptSendableQMeta<T: Sendable & Q>(_: T.Type) { }
 
 nonisolated func testConformancesFromNonisolated() {
-  let _: any P = CExplicitMainActor() // expected-error{{main actor-isolated conformance of 'CExplicitMainActor' to 'P' cannot be used in nonisolated context}}
-  let _: any P = CImplicitMainActor() // expected-error{{main actor-isolated conformance of 'CImplicitMainActor' to 'P' cannot be used in nonisolated context}}
+  let _: any P = CExplicitMainActor() // okay
+  let _: any P = CImplicitMainActor() // okay
 
   let _: any P = CNonIsolated()
   let _: any P = CImplicitMainActorNonisolatedConformance()

test/Concurrency/isolated_conformance_inference.swift

@@ -32,6 +32,11 @@ extension CExplicit: Q {
   func g() { }
 }
 
+@SomeGlobalActor
+class CViaNonisolatedWitness: P {
+  nonisolated func f() { } // okay! conformance above is nonisolated via this witness
+}
+
 // expected-error@+3{{conformance of 'CNonIsolated' to protocol 'P' crosses into global actor 'SomeGlobalActor'-isolated code and can cause data races}}
 // expected-note@+2{{turn data races into runtime errors with '@preconcurrency'}}
 // expected-note@+1{{isolate this conformance to the global actor 'SomeGlobalActor' with '@SomeGlobalActor'}}{{33-33=@SomeGlobalActor }}
@@ -42,11 +47,29 @@ nonisolated class CNonIsolated: P {
 func acceptSendablePMeta<T: Sendable & P>(_: T.Type) { }
 func acceptSendableQMeta<T: Sendable & Q>(_: T.Type) { }
 
-nonisolated func testConformancesFromNonisolated() {
+// @preconcurrency suppresses actor isolation inference
+struct NotSendable: Equatable, Hashable {
+}
+
+@available(*, unavailable)
+extension NotSendable: Sendable {}
+
+extension NotSendable : Codable {}
+
+@MainActor
+struct TestDerivedCodable : @preconcurrency Codable {
+  var x: NotSendable
+}
+
+nonisolated func testConformancesFromNonisolated(tdc: TestDerivedCodable) {
   let _: any P = CExplicit() // expected-error{{global actor 'SomeGlobalActor'-isolated conformance of 'CExplicit' to 'P' cannot be used in nonisolated context}}
 
   let _: any P = CNonIsolated()
 
   // Okay, these are nonisolated conformances.
   let _: any Q = CExplicit()
+
+  let _: any P = CViaNonisolatedWitness()
+
+  let _: any Codable = tdc
 }

test/Macros/Inputs/syntax_macro_definitions.swift

@@ -2881,6 +2881,58 @@ public struct HangingMacro: PeerMacro {
   }
 }
 
+public struct PWithNonisolatedFuncMacro: ExtensionMacro {
+  public static var inferNonisolatedConformances: Bool { false }
+
+  public static func expansion(
+    of node: AttributeSyntax,
+    attachedTo decl: some DeclGroupSyntax,
+    providingExtensionsOf type: some TypeSyntaxProtocol,
+    conformingTo protocols: [TypeSyntax],
+    in context: some MacroExpansionContext
+  ) throws -> [ExtensionDeclSyntax] {
+    if (protocols.isEmpty) {
+      return []
+    }
+
+    let decl: DeclSyntax =
+      """
+      extension \(raw: type.trimmedDescription): P {
+        nonisolated static func requirement() { }
+      }
+      """
+
+    return [
+      decl.cast(ExtensionDeclSyntax.self)
+    ]
+  }
+}
+
+public struct NonisolatedPWithNonisolatedFuncMacro: ExtensionMacro {
+  public static func expansion(
+    of node: AttributeSyntax,
+    attachedTo decl: some DeclGroupSyntax,
+    providingExtensionsOf type: some TypeSyntaxProtocol,
+    conformingTo protocols: [TypeSyntax],
+    in context: some MacroExpansionContext
+  ) throws -> [ExtensionDeclSyntax] {
+    if (protocols.isEmpty) {
+      return []
+    }
+
+    let decl: DeclSyntax =
+      """
+      extension \(raw: type.trimmedDescription): P {
+        nonisolated static func requirement() { }
+      }
+      """
+
+    return [
+      decl.cast(ExtensionDeclSyntax.self)
+    ]
+  }
+}
+
 public struct BigEndianAccessorMacro: AccessorMacro {
     public static func expansion(
         of node: AttributeSyntax,

test/Macros/macro_expand_extensions.swift

@@ -283,3 +283,15 @@ struct HasNestedType {
 // extensions of nested types when the outer type has an
 // attached macro that can add other nested types.
 extension HasNestedType.Inner {}
+
+@attached(extension, conformances: P, names: named(requirement))
+macro AddPWithNonisolated() = #externalMacro(module: "MacroDefinition", type: "PWithNonisolatedFuncMacro")
+
+@attached(extension, conformances: P, names: named(requirement))
+macro AddNonisolatedPWithNonisolated() = #externalMacro(module: "MacroDefinition", type: "NonisolatedPWithNonisolatedFuncMacro")
+
+@AddNonisolatedPWithNonisolated
+struct MakeMeNonisolated { }
+
+@AddPWithNonisolated
+struct KeepMeIsolated { }