[Concurrency] Fix preconcurrency downgrade behavior for Sendable closures.

Sendable violations inside `@preconcurrency @Sendable` closures should be
suppressed in minimal checking, and diagnosed as warnings under complete
checking, including the Swift 6 language mode.

(cherry picked from commit d2d317a)

Author: hborla

include/swift/AST/DiagnosticEngine.h

@@ -656,6 +656,30 @@ namespace swift {
     InFlightDiagnostic &limitBehaviorUntilSwiftVersion(
         DiagnosticBehavior limit, unsigned majorVersion);
 
+    /// Limits the diagnostic behavior to \c limit accordingly if
+    /// preconcurrency applies. Otherwise, the behavior limit only applies
+    /// prior to the given language mode.
+    ///
+    /// `@preconcurrency` applied to a nominal declaration or an import
+    /// statement will limit concurrency diagnostic behavior based on the
+    /// strict concurrency checking level. Under minimal checking,
+    /// preconcurrency will suppress the diagnostics. With strict concurrency
+    /// checking, including when building with the Swift 6 language mode,
+    /// preconcurrency errors are downgraded to warnings. This allows libraries
+    /// to stage in concurrency annotations even after their clients have
+    /// migrated to Swift 6 using `@preconcurrency` alongside the newly added
+    /// `@Sendable` or `@MainActor` annotations.
+    InFlightDiagnostic
+    &limitBehaviorWithPreconcurrency(DiagnosticBehavior limit,
+                                     bool preconcurrency,
+                                     unsigned languageMode = 6) {
+      if (preconcurrency) {
+        return limitBehavior(limit);
+      }
+
+      return limitBehaviorUntilSwiftVersion(limit, languageMode);
+    }
+
     /// Limit the diagnostic behavior to warning until the specified version.
     ///
     /// This helps stage in fixes for stricter diagnostics as warnings

lib/Sema/TypeCheckConcurrency.cpp

@@ -788,7 +788,10 @@ static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
   });
 }
 
-bool SendableCheckContext::isExplicitSendableConformance() const {
+bool SendableCheckContext::warnInMinimalChecking() const {
+  if (preconcurrencyContext)
+    return false;
+
   if (!conformanceCheck)
     return false;
 
@@ -806,7 +809,7 @@ bool SendableCheckContext::isExplicitSendableConformance() const {
 DiagnosticBehavior SendableCheckContext::defaultDiagnosticBehavior() const {
   // If we're not supposed to diagnose existing data races from this context,
   // ignore the diagnostic entirely.
-  if (!isExplicitSendableConformance() &&
+  if (!warnInMinimalChecking() &&
       !shouldDiagnoseExistingDataRaces(fromDC))
     return DiagnosticBehavior::Ignore;
 
@@ -825,9 +828,7 @@ SendableCheckContext::implicitSendableDiagnosticBehavior() const {
     LLVM_FALLTHROUGH;
 
   case StrictConcurrency::Minimal:
-    // Explicit Sendable conformances always diagnose, even when strict
-    // strict checking is disabled.
-    if (isExplicitSendableConformance())
+    if (warnInMinimalChecking())
       return DiagnosticBehavior::Warning;
 
     return DiagnosticBehavior::Ignore;
@@ -868,6 +869,13 @@ bool swift::hasExplicitSendableConformance(NominalTypeDecl *nominal,
 /// nominal type.
 DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
     NominalTypeDecl *nominal) const {
+  // If we're in a preconcurrency context, don't override the default behavior
+  // based on explicit conformances. For example, a @preconcurrency @Sendable
+  // closure should not warn about an explicitly unavailable Sendable
+  // conformance in minimal checking.
+  if (preconcurrencyContext)
+    return defaultDiagnosticBehavior();
+
   if (hasExplicitSendableConformance(nominal))
     return DiagnosticBehavior::Warning;
 
@@ -2780,11 +2788,16 @@ namespace {
           continue;
 
         auto *closure = localFunc.getAbstractClosureExpr();
+        auto *explicitClosure = dyn_cast_or_null<ClosureExpr>(closure);
+
+        bool preconcurrency = false;
+        if (explicitClosure) {
+          preconcurrency = explicitClosure->isIsolatedByPreconcurrency();
+        }
 
         // Diagnose a `self` capture inside an escaping `sending`
         // `@Sendable` closure in a deinit, which almost certainly
         // means `self` would escape deinit at runtime.
-        auto *explicitClosure = dyn_cast_or_null<ClosureExpr>(closure);
         auto *dc = getDeclContext();
         if (explicitClosure && isa<DestructorDecl>(dc) &&
             !explicitClosure->getType()->isNoEscape() &&
@@ -2794,7 +2807,8 @@ namespace {
           if (var && var->isSelfParameter()) {
             ctx.Diags.diagnose(explicitClosure->getLoc(),
                                diag::self_capture_deinit_task)
-                .warnUntilSwiftVersion(6);
+                .limitBehaviorWithPreconcurrency(DiagnosticBehavior::Warning,
+                                                 preconcurrency);
           }
         }
 
@@ -2822,6 +2836,9 @@ namespace {
         if (type->hasError())
           continue;
 
+        SendableCheckContext sendableContext(getDeclContext(),
+                                             preconcurrency);
+
         if (closure && closure->isImplicit()) {
           auto *patternBindingDecl = getTopPatternBindingDecl();
           if (patternBindingDecl && patternBindingDecl->isAsyncLet()) {
@@ -2843,21 +2860,21 @@ namespace {
           } else {
             // Fallback to a generic implicit capture missing sendable
             // conformance diagnostic.
-            diagnoseNonSendableTypes(type, getDeclContext(),
+            diagnoseNonSendableTypes(type, sendableContext,
                                      /*inDerivedConformance*/Type(),
                                      capture.getLoc(),
                                      diag::implicit_non_sendable_capture,
                                      decl->getName());
           }
         } else if (fnType->isSendable()) {
-          diagnoseNonSendableTypes(type, getDeclContext(),
+          diagnoseNonSendableTypes(type, sendableContext,
                                    /*inDerivedConformance*/Type(),
                                    capture.getLoc(),
                                    diag::non_sendable_capture,
                                    decl->getName(),
                                    /*closure=*/closure != nullptr);
         } else {
-          diagnoseNonSendableTypes(type, getDeclContext(),
+          diagnoseNonSendableTypes(type, sendableContext,
                                    /*inDerivedConformance*/Type(),
                                    capture.getLoc(),
                                    diag::non_sendable_isolated_capture,
@@ -4090,7 +4107,12 @@ namespace {
       if (!mayExecuteConcurrentlyWith(dc, findCapturedDeclContext(value)))
         return false;
 
-      SendableCheckContext sendableBehavior(dc);
+      bool preconcurrency = false;
+      if (auto *closure = dyn_cast<ClosureExpr>(dc)) {
+        preconcurrency = closure->isIsolatedByPreconcurrency();
+      }
+
+      SendableCheckContext sendableBehavior(dc, preconcurrency);
       auto limit = sendableBehavior.defaultDiagnosticBehavior();
 
       // Check whether this is a local variable, in which case we can
@@ -4120,7 +4142,7 @@ namespace {
             ctx.Diags
                 .diagnose(loc, diag::concurrent_access_of_inout_param,
                           param->getName())
-                .limitBehaviorUntilSwiftVersion(limit, 6);
+                .limitBehaviorWithPreconcurrency(limit, preconcurrency);
             return true;
           }
         }
@@ -4135,7 +4157,7 @@ namespace {
             loc, diag::concurrent_access_of_local_capture,
             parent.dyn_cast<LoadExpr *>(),
             var)
-          .limitBehaviorUntilSwiftVersion(limit, 6);
+          .limitBehaviorWithPreconcurrency(limit, preconcurrency);
         return true;
       }
 
@@ -4145,7 +4167,7 @@ namespace {
 
         func->diagnose(diag::local_function_executed_concurrently, func)
           .fixItInsert(func->getAttributeInsertionLoc(false), "@Sendable ")
-          .limitBehaviorUntilSwiftVersion(limit, 6);
+          .limitBehaviorWithPreconcurrency(limit, preconcurrency);
 
         // Add the @Sendable attribute implicitly, so we don't diagnose
         // again.
@@ -4155,7 +4177,8 @@ namespace {
       }
 
       // Concurrent access to some other local.
-      ctx.Diags.diagnose(loc, diag::concurrent_access_local, value);
+      ctx.Diags.diagnose(loc, diag::concurrent_access_local, value)
+        .limitBehaviorWithPreconcurrency(limit, preconcurrency);
       value->diagnose(
           diag::kind_declared_here, value->getDescriptiveKind());
       return true;
@@ -5980,7 +6003,8 @@ bool swift::contextRequiresStrictConcurrencyChecking(
     const DeclContext *dc,
     llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
     llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
-  switch (dc->getASTContext().LangOpts.StrictConcurrencyLevel) {
+  auto concurrencyLevel = dc->getASTContext().LangOpts.StrictConcurrencyLevel;
+  switch (concurrencyLevel) {
   case StrictConcurrency::Complete:
     return true;
 
@@ -6000,7 +6024,20 @@ bool swift::contextRequiresStrictConcurrencyChecking(
 
         // Don't take any more cues if this only got its type information by
         // being provided to a `@preconcurrency` operation.
+        //
+        // FIXME: contextRequiresStrictConcurrencyChecking is called from
+        // within the constraint system, but closures are only set to be isolated
+        // by preconcurrency in solution application because it's dependent on
+        // overload resolution. The constraint system either needs to check its
+        // own state on the current path, or not make type inference decisions based
+        // on concurrency checking level.
         if (isolatedByPreconcurrency(explicitClosure)) {
+          // If we're in minimal checking, preconcurrency always suppresses
+          // diagnostics. Targeted checking will still produce diagnostics if
+          // the outer context has adopted explicit concurrency features.
+          if (concurrencyLevel == StrictConcurrency::Minimal)
+            return false;
+
           dc = dc->getParent();
           continue;
         }

lib/Sema/TypeCheckConcurrency.h

@@ -374,12 +374,23 @@ static inline bool isImplicitSendableCheck(SendableCheck check) {
 /// Describes the context in which a \c Sendable check occurs.
 struct SendableCheckContext {
   const DeclContext * const fromDC;
+  bool preconcurrencyContext;
   const std::optional<SendableCheck> conformanceCheck;
 
   SendableCheckContext(
       const DeclContext *fromDC,
       std::optional<SendableCheck> conformanceCheck = std::nullopt)
-      : fromDC(fromDC), conformanceCheck(conformanceCheck) {}
+      : fromDC(fromDC),
+        preconcurrencyContext(false),
+        conformanceCheck(conformanceCheck) {}
+
+  SendableCheckContext(
+      const DeclContext *fromDC,
+      bool preconcurrencyContext,
+      std::optional<SendableCheck> conformanceCheck = std::nullopt)
+      : fromDC(fromDC),
+        preconcurrencyContext(preconcurrencyContext),
+        conformanceCheck(conformanceCheck) {}
 
   /// Determine the default diagnostic behavior for a missing/unavailable
   /// Sendable conformance in this context.
@@ -396,8 +407,8 @@ struct SendableCheckContext {
       Decl *decl,
       bool ignoreExplicitConformance = false) const;
 
-  /// Whether we are in an explicit conformance to Sendable.
-  bool isExplicitSendableConformance() const;
+  /// Whether to warn about a Sendable violation even in minimal checking.
+  bool warnInMinimalChecking() const;
 };
 
 /// Diagnose any non-Sendable types that occur within the given type, using
@@ -443,11 +454,14 @@ bool diagnoseNonSendableTypes(
   return diagnoseNonSendableTypes(
       type, fromContext, derivedConformance, typeLoc,
       [&](Type specificType, DiagnosticBehavior behavior) {
+        // FIXME: Reconcile preconcurrency declaration vs preconcurrency
+        // import behavior.
         auto preconcurrency =
           fromContext.preconcurrencyBehavior(specificType->getAnyNominal());
 
         ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
-            .limitBehaviorUntilSwiftVersion(behavior, 6)
+            .limitBehaviorWithPreconcurrency(behavior,
+                                             fromContext.preconcurrencyContext)
             .limitBehaviorIf(preconcurrency);
 
         return (behavior == DiagnosticBehavior::Ignore ||

test/Concurrency/Runtime/async_stream.swift

@@ -21,12 +21,12 @@ class NotSendable {}
 @MainActor func testWarnings() {
   var x = 0
   _ = AsyncStream {
-    x += 1 // expected-warning {{mutation of captured var 'x' in concurrently-executing code; this is an error in the Swift 6 language mode}}
+    x += 1 // expected-warning {{mutation of captured var 'x' in concurrently-executing code}}
     return 0
   }
 
   _ = AsyncThrowingStream {
-    x += 1 // expected-warning {{mutation of captured var 'x' in concurrently-executing code; this is an error in the Swift 6 language mode}}
+    x += 1 // expected-warning {{mutation of captured var 'x' in concurrently-executing code}}
     return
   }
 }

test/Concurrency/preconcurrency_typealias.swift

@@ -1,7 +1,6 @@
-// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s
-// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s -strict-concurrency=targeted
-// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s -verify-additional-prefix complete-tns- -strict-concurrency=complete
-// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s -verify-additional-prefix complete-tns- -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation
+// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s -strict-concurrency=minimal
+// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s -strict-concurrency=targeted -verify-additional-prefix targeted-complete-
+// RUN: %target-swift-frontend -emit-sil -o /dev/null -verify %s -strict-concurrency=complete -verify-additional-prefix targeted-complete- -verify-additional-prefix complete-tns-
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
@@ -31,7 +30,7 @@ func test() {
   var mutableVariable = 0
   preconcurrencyFunc {
     mutableVariable += 1 // no sendable warning unless we have complete
-    // expected-complete-tns-warning @-1 {{mutation of captured var 'mutableVariable' in concurrently-executing code; this is an error in the Swift 6 language mode}}
+    // expected-complete-tns-warning @-1 {{mutation of captured var 'mutableVariable' in concurrently-executing code}}
   }
   mutableVariable += 1
 }
@@ -49,7 +48,7 @@ func testAsync() async {
 
   var mutableVariable = 0
   preconcurrencyFunc {
-    mutableVariable += 1 // expected-warning{{mutation of captured var 'mutableVariable' in concurrently-executing code; this is an error in the Swift 6 language mode}}
+    mutableVariable += 1 // expected-targeted-complete-warning{{mutation of captured var 'mutableVariable' in concurrently-executing code}}
   }
   mutableVariable += 1
 }

test/Concurrency/sendable_checking_captures_swift5.swift

@@ -51,3 +51,21 @@ do {
     }
   }
 }
+
+class NotSendable {} // expected-complete-note {{class 'NotSendable' does not conform to the 'Sendable' protocol}}
+
+@available(*, unavailable)
+extension NotSendable : @unchecked Sendable {}
+
+@preconcurrency func withSendableClosure(_: @Sendable () -> Void) {}
+
+func testPreconcurrencyDowngrade(ns: NotSendable) {
+  var x = 0
+  withSendableClosure {
+    _ = ns
+    // expected-complete-warning@-1 {{capture of 'ns' with non-sendable type 'NotSendable' in a `@Sendable` closure}}
+
+    x += 1
+    // expected-complete-warning@-1 {{mutation of captured var 'x' in concurrently-executing code}}
+  }
+}

test/Concurrency/sendable_checking_captures_swift6.swift

@@ -45,8 +45,8 @@ do {
 
     sendable_preconcurrency {
       test.update()
-      // expected-error@-1 {{capture of 'test' with non-sendable type 'Test' in a `@Sendable` closure}}
-      // expected-error@-2 {{mutable capture of 'inout' parameter 'test' is not allowed in concurrently-executing code}}
+      // expected-warning@-1 {{capture of 'test' with non-sendable type 'Test' in a `@Sendable` closure}}
+      // expected-warning@-2 {{mutable capture of 'inout' parameter 'test' is not allowed in concurrently-executing code}}
     }
   }
 }