Don't check sendability of witnesses for @preconcurrency conformances

When a witness is part of a `@preconcurrency` conformance, suppress
Sendable checking for that witness because we assume that the caller is
correctly invoking this witness from within the right isolation
domain. This property is dynamically checked.

Fixes #74057.

Author: DougGregor

lib/Sema/TypeCheckProtocol.cpp

@@ -3090,7 +3090,8 @@ static bool hasExplicitGlobalActorAttr(ValueDecl *decl) {
 
 std::optional<ActorIsolation>
 ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
-                                        ValueDecl *witness) {
+                                        ValueDecl *witness,
+                                        bool &usesPreconcurrency) {
 
   // Determine the isolation of the requirement itself.
   auto requirementIsolation = getActorIsolation(requirement);
@@ -3136,9 +3137,16 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
     return std::nullopt;
 
   case ActorReferenceResult::ExitsActorToNonisolated:
-    diagnoseNonSendableTypesInReference(
-        /*base=*/nullptr, getDeclRefInContext(witness),
-        DC, loc, SendableCheckReason::Conformance);
+    if (!isPreconcurrency) {
+      diagnoseNonSendableTypesInReference(
+          /*base=*/nullptr, getDeclRefInContext(witness),
+          DC, loc, SendableCheckReason::Conformance);
+    } else {
+      // We depended on @preconcurrency since we were exiting an isolation
+      // domain.
+      usesPreconcurrency = true;
+    }
+
     return std::nullopt;
   case ActorReferenceResult::EntersActor:
     // Handled below.
@@ -3220,19 +3228,24 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
         witness->getAttrs().hasAttribute<NonisolatedAttr>())
       return std::nullopt;
 
-    // Check that the results of the witnessing method are sendable
-    diagnoseNonSendableTypesInReference(
-        /*base=*/nullptr, getDeclRefInContext(witness),
-        DC, loc, SendableCheckReason::Conformance,
-        getActorIsolation(witness), FunctionCheckKind::Results);
-
-    // If this requirement is a function, check that its parameters are Sendable as well
-    if (isa<AbstractFunctionDecl>(requirement)) {
+    if (!isPreconcurrency) {
+      // Check that the results of the witnessing method are sendable
       diagnoseNonSendableTypesInReference(
-          /*base=*/nullptr, getDeclRefInContext(requirement),
-          requirement->getInnermostDeclContext(), requirement->getLoc(),
-          SendableCheckReason::Conformance, getActorIsolation(witness),
-          FunctionCheckKind::Params, loc);
+          /*base=*/nullptr, getDeclRefInContext(witness),
+          DC, loc, SendableCheckReason::Conformance,
+          getActorIsolation(witness), FunctionCheckKind::Results);
+
+      // If this requirement is a function, check that its parameters are Sendable as well
+      if (isa<AbstractFunctionDecl>(requirement)) {
+        diagnoseNonSendableTypesInReference(
+            /*base=*/nullptr, getDeclRefInContext(requirement),
+            requirement->getInnermostDeclContext(), requirement->getLoc(),
+            SendableCheckReason::Conformance, getActorIsolation(witness),
+            FunctionCheckKind::Params, loc);
+      }
+    } else {
+      // We depended on @preconcurrency to suppress Sendable checking.
+      usesPreconcurrency = true;
     }
 
     // If the witness is accessible across actors, we don't need to consider it
@@ -4955,7 +4968,7 @@ hasInvalidTypeInConformanceContext(const ValueDecl *requirement,
 }
 
 void ConformanceChecker::resolveValueWitnesses() {
-  bool usesPreconcurrencyConformance = false;
+  bool usesPreconcurrency = false;
 
   for (auto *requirement : Proto->getProtocolRequirements()) {
     // Associated type requirements handled elsewhere.
@@ -4974,16 +4987,8 @@ void ConformanceChecker::resolveValueWitnesses() {
 
       // Check actor isolation. If we need to enter into the actor's
       // isolation within the witness thunk, record that.
-      if (auto enteringIsolation = checkActorIsolation(requirement, witness)) {
-        // Only @preconcurrency conformances allow entering isolation
-        // when neither requirement nor witness are async by adding a
-        // runtime precondition that witness is always called on the
-        // expected executor.
-        if (Conformance->isPreconcurrency()) {
-          usesPreconcurrencyConformance =
-              !isAsyncDecl(requirement) && !isAsyncDecl(witness);
-        }
-
+      if (auto enteringIsolation = checkActorIsolation(requirement, witness,
+                                                       usesPreconcurrency)) {
         Conformance->overrideWitness(
             requirement,
             Conformance->getWitnessUncached(requirement)
@@ -5134,7 +5139,7 @@ void ConformanceChecker::resolveValueWitnesses() {
     }
   }
 
-  if (Conformance->isPreconcurrency() && !usesPreconcurrencyConformance) {
+  if (Conformance->isPreconcurrency() && !usesPreconcurrency) {
     auto diag = DC->getASTContext().Diags.diagnose(
         Conformance->getLoc(), diag::preconcurrency_conformance_not_used,
         Proto->getDeclaredInterfaceType());

lib/Sema/TypeCheckProtocol.h

@@ -130,11 +130,15 @@ class ConformanceChecker : public WitnessChecker {
 
   /// Check that the witness and requirement have compatible actor contexts.
   ///
+  /// \param usesPreconcurrency Will be set true if the conformance is
+  /// @preconcurrency and we made use of that fact.
+  ///
   /// \returns the isolation that needs to be enforced to invoke the witness
   /// from the requirement, used when entering an actor-isolated synchronous
   /// witness from an asynchronous requirement.
   std::optional<ActorIsolation> checkActorIsolation(ValueDecl *requirement,
-                                                    ValueDecl *witness);
+                                                    ValueDecl *witness,
+                                                    bool &usesPreconcurrency);
 
   /// Enforce restrictions on non-final classes witnessing requirements
   /// involving the protocol 'Self' type.

test/Concurrency/preconcurrency_conformances.swift

@@ -35,28 +35,36 @@ extension TestPreconcurrencyAttr : @preconcurrency Q { // Ok
 }
 
 class NonSendable {}
-// expected-note@-1 6 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
 
 protocol TestSendability {
   var x: NonSendable { get }
   func test(_: NonSendable?) -> [NonSendable]
 }
 
-// Make sure that preconcurrency conformances don't suppress Sendable diagnostics
+// Make sure that preconcurrency conformances suppress Sendable diagnostics,
+// because @preconcurrency assumes that the witness will always be called
+// from within the same isolation domain (with a dynamic check).
 @MainActor
 struct Value : @preconcurrency TestSendability {
   var x: NonSendable { NonSendable() }
-  // expected-warning@-1 {{non-sendable type 'NonSendable' in conformance of main actor-isolated property 'x' to protocol requirement cannot cross actor boundary}}
-  // expected-note@-2 2 {{property declared here}}
+  // expected-note@-1 2 {{property declared here}}
 
-  // expected-warning@+2 {{non-sendable type '[NonSendable]' returned by main actor-isolated instance method 'test' satisfying protocol requirement cannot cross actor boundary}}
-  // expected-warning@+1 {{non-sendable type 'NonSendable?' in parameter of the protocol requirement satisfied by main actor-isolated instance method 'test' cannot cross actor boundary}}
   func test(_: NonSendable?) -> [NonSendable] {
     // expected-note@-1 2 {{calls to instance method 'test' from outside of its actor context are implicitly asynchronous}}
     []
   }
 }
 
+// Make sure we don't spuriously say the @preconcurrency is unnecessary.
+@MainActor
+struct OtherValue : @preconcurrency TestSendability {
+  var x: NonSendable { NonSendable() }
+
+  nonisolated func test(_: NonSendable?) -> [NonSendable] {
+    []
+  }
+}
+
 // Make sure that references to actor isolated witness is diagnosed
 
 // expected-note@+1 2 {{add '@MainActor' to make global function 'test(value:)' part of global actor 'MainActor'}}
@@ -73,10 +81,7 @@ actor MyActor {
 
 extension MyActor : @preconcurrency TestSendability {
   var x: NonSendable { NonSendable() }
-  // expected-warning@-1 {{non-sendable type 'NonSendable' in conformance of actor-isolated property 'x' to protocol requirement cannot cross actor boundary}}
 
-  // expected-warning@+2 {{non-sendable type '[NonSendable]' returned by actor-isolated instance method 'test' satisfying protocol requirement cannot cross actor boundary}}
-  // expected-warning@+1 {{non-sendable type 'NonSendable?' in parameter of the protocol requirement satisfied by actor-isolated instance method 'test' cannot cross actor boundary}}
   func test(_: NonSendable?) -> [NonSendable] {
     []
   }