Merge pull request swiftlang#83420 from xedin/rdar-157061896

[Concurrency] Downgrade errors when isolated member is referenced from a preconcurrency context

Author: xedin

lib/Sema/TypeCheckConcurrency.cpp

@@ -40,6 +40,7 @@
 #include "swift/Strings.h"
 
 using namespace swift;
+using namespace swift::version;
 
 static ActorIsolation getOverriddenIsolationFor(const ValueDecl *value);
 
@@ -567,7 +568,7 @@ static bool varIsSafeAcrossActors(const ModuleDecl *fromModule, VarDecl *var,
     if (var->isGlobalStorage() && var->isLazilyInitializedGlobal()) {
       // Compiler versions <= 5.9 accepted this code, so downgrade to a
       // warning prior to Swift 6.
-      options = ActorReferenceResult::Flags::Preconcurrency;
+      options = ActorReferenceResult::Flags::CompatibilityDowngrade;
       return false;
     }
 
@@ -588,7 +589,7 @@ static bool varIsSafeAcrossActors(const ModuleDecl *fromModule, VarDecl *var,
       // so downgrade async access errors in the effects checker to
       // warnings prior to Swift 6.
       if (accessWithinModule)
-        options = ActorReferenceResult::Flags::Preconcurrency;
+        options = ActorReferenceResult::Flags::CompatibilityDowngrade;
 
       return false;
     }
@@ -4242,11 +4243,10 @@ namespace {
 
           applyErrors[key].push_back(mismatch);
         } else {
-          ctx.Diags.diagnose(
-            apply->getLoc(),
-            diagnostic.getID(),
-            diagnostic.getArgs())
-              .warnUntilSwiftVersionIf(preconcurrency, 6);
+          ctx.Diags
+              .diagnose(apply->getLoc(), diagnostic.getID(),
+                        diagnostic.getArgs())
+              .limitBehaviorIf(preconcurrency, DiagnosticBehavior::Warning);
 
           if (calleeDecl) {
             auto calleeIsolation = getInferredActorIsolation(calleeDecl);
@@ -4590,9 +4590,10 @@ namespace {
               break;
             }
 
-            bool downgrade = isolation.isGlobalActor() ||
-              options.contains(
-                  ActorReferenceResult::Flags::Preconcurrency);
+            bool downgrade =
+                isolation.isGlobalActor() ||
+                options.contains(
+                    ActorReferenceResult::Flags::CompatibilityDowngrade);
 
             ctx.Diags.diagnose(
                 component.getLoc(), diag::actor_isolated_keypath_component,
@@ -4782,6 +4783,10 @@ namespace {
         // Does the reference originate from a @preconcurrency context?
         bool preconcurrencyContext =
           result.options.contains(ActorReferenceResult::Flags::Preconcurrency);
+        bool shouldDowngradeToWarning =
+            preconcurrencyContext ||
+            result.options.contains(
+                ActorReferenceResult::Flags::CompatibilityDowngrade);
 
         Type derivedConformanceType;
         DeclName requirementName;
@@ -4816,14 +4821,21 @@ namespace {
             refErrors.insert(std::make_pair(keyPair, list));
           }
         } else {
-          ctx.Diags.diagnose(
-              loc, diag::actor_isolated_non_self_reference,
-              decl, useKind,
-              refKind + 1, refGlobalActor,
-              result.isolation)
-          .warnUntilSwiftVersionIf(preconcurrencyContext, 6);
-          maybeNoteMutatingMethodSuggestion(ctx, decl, loc, getDeclContext(), result.isolation,
-                                            kindOfUsage(decl, context).value_or(VarRefUseEnv::Read));
+          {
+            auto diagnostic = ctx.Diags.diagnose(
+                loc, diag::actor_isolated_non_self_reference, decl, useKind,
+                refKind + 1, refGlobalActor, result.isolation);
+
+            // For compatibility downgrades - the error is downgraded until
+            // Swift 6, for preconcurrency - always.
+            if (shouldDowngradeToWarning)
+              diagnostic.limitBehaviorWithPreconcurrency(
+                  DiagnosticBehavior::Warning, preconcurrencyContext);
+          }
+
+          maybeNoteMutatingMethodSuggestion(
+              ctx, decl, loc, getDeclContext(), result.isolation,
+              kindOfUsage(decl, context).value_or(VarRefUseEnv::Read));
 
           if (derivedConformanceType) {
             auto *decl = dyn_cast<ValueDecl>(getDeclContext()->getAsDecl());
@@ -8319,7 +8331,7 @@ ActorReferenceResult ActorReferenceResult::forReference(
         // Treat the decl isolation as 'preconcurrency' to downgrade violations
         // to warnings, because violating Sendable here is accepted by the
         // Swift 5.9 compiler.
-        options |= Flags::Preconcurrency;
+        options |= Flags::CompatibilityDowngrade;
         return forEntersActor(declIsolation, options);
       }
     }
@@ -8358,8 +8370,10 @@ ActorReferenceResult ActorReferenceResult::forReference(
   // This is a cross-actor reference.
 
   // Note if the reference originates from a @preconcurrency-isolated context.
-  if (contextIsolation.preconcurrency() || declIsolation.preconcurrency())
+  if (contextIsolation.preconcurrency() || declIsolation.preconcurrency()) {
     options |= Flags::Preconcurrency;
+    options |= Flags::CompatibilityDowngrade;
+  }
 
   // If the declaration isn't asynchronous, promote to async.
   if (!decl->isAsync())

lib/Sema/TypeCheckConcurrency.h

@@ -208,12 +208,20 @@ struct ActorReferenceResult {
     /// potentially from a different node, so it must be marked 'distributed'.
     Distributed = 1 << 2,
 
-    /// The declaration is being accessed from a @preconcurrency context.
+    /// The declaration is marked as `@preconcurrency` or being accessed
+    /// from a @preconcurrency context.
     Preconcurrency = 1 << 3,
 
     /// Only arguments cross an isolation boundary, e.g. because they
     /// escape into an actor in a nonisolated actor initializer.
     OnlyArgsCrossIsolation = 1 << 4,
+
+    /// The reference to the declaration is invalid but has to be downgraded
+    /// to a warning because it was accepted by the older compilers or because
+    /// the declaration predates concurrency and is marked as such.
+    ///
+    /// NOTE: This flag is set for `Preconcurrency` declarations.
+    CompatibilityDowngrade = 1 << 5,
   };
 
   using Options = OptionSet<Flags>;

lib/Sema/TypeCheckEffects.cpp

@@ -1488,7 +1488,8 @@ class ApplyClassifier {
         module = var->getDeclContext()->getParentModule();
       }
       if (!isLetAccessibleAnywhere(module, var, options)) {
-        return options.contains(ActorReferenceResult::Flags::Preconcurrency);
+        return options.contains(
+            ActorReferenceResult::Flags::CompatibilityDowngrade);
       }
     }
 

lib/Sema/TypeCheckProtocol.cpp

@@ -3588,7 +3588,7 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
     isLetAccessibleAnywhere(
         witness->getDeclContext()->getParentModule(),
         var, options);
-    if (options.contains(ActorReferenceResult::Flags::Preconcurrency)) {
+    if (options.contains(ActorReferenceResult::Flags::CompatibilityDowngrade)) {
       behavior = DiagnosticBehavior::Warning;
     }
   }

test/ClangImporter/objc_async.swift

@@ -392,7 +392,7 @@ extension SomeWrapper: Sendable where T: Sendable {}
   func makeCall(slowServer: SlowServer) {
     slowServer.doSomethingSlow("churn butter") { (_ : Int) in
       let _ = self.isolatedThing
-      // expected-warning@-1 {{main actor-isolated property 'isolatedThing' can not be referenced from a Sendable closure; this is an error in the Swift 6 language mode}}
+      // expected-warning@-1 {{main actor-isolated property 'isolatedThing' can not be referenced from a Sendable closure}}
     }
   }
 }

test/Concurrency/assume_mainactor_typechecker_errors.swift

@@ -15,7 +15,7 @@ class SendableData : @unchecked Sendable {}
 // expected-swift5-note@-1 {{calls to initializer 'init()' from outside of its actor context are implicitly asynchronous}}
 
 nonisolated func getDataFromSocket() -> SendableData { SendableData() }
-// expected-swift5-warning@-1 {{call to main actor-isolated initializer 'init()' in a synchronous nonisolated context; this is an error in the Swift 6 language mode}}
+// expected-swift5-warning@-1 {{call to main actor-isolated initializer 'init()' in a synchronous nonisolated context}}
 
 class Klass { // expected-swift5-note 3 {{}} expected-swift6-note 3 {{}}
   let s = SendableData()

test/Concurrency/objc_async_overload.swift

@@ -58,7 +58,7 @@ extension Delegate {
   func handle(_ req: Request, with delegate: Delegate) {
     delegate.makeRequest1(req) {
       self.finish()
-      // expected-warning@-1 {{call to main actor-isolated instance method 'finish()' in a synchronous nonisolated context; this is an error in the Swift 6 language mode}}
+      // expected-warning@-1 {{call to main actor-isolated instance method 'finish()' in a synchronous nonisolated context}}
     }
   }
 }

test/Concurrency/predates_concurrency.swift

@@ -386,7 +386,7 @@ do {
     func run() async {
       await test {
         if let value {
-          // expected-warning@-1 {{main actor-isolated property 'value' can not be referenced from a Sendable closure; this is an error in the Swift 6 language mode}}
+          // expected-warning@-1 {{main actor-isolated property 'value' can not be referenced from a Sendable closure}}
           print(value)
         }
       }

test/Concurrency/predates_concurrency_swift6.swift

@@ -63,13 +63,13 @@ func testElsewhere(x: X) {
 
 func testCalls(x: X) {
   // expected-note@-1 2{{add '@MainActor' to make global function 'testCalls(x:)' part of global actor 'MainActor'}}
-  onMainActorAlways() // expected-error{{call to main actor-isolated global function 'onMainActorAlways()' in a synchronous nonisolated context}}
+  onMainActorAlways() // expected-warning{{call to main actor-isolated global function 'onMainActorAlways()' in a synchronous nonisolated context}}
 
   let _: () -> Void = onMainActorAlways // expected-error{{converting function value of type '@MainActor @Sendable () -> ()' to '() -> Void' loses global actor 'MainActor'}}
 
   let c = MyModelClass() // okay, synthesized init() is 'nonisolated'
 
-  c.f() // expected-error{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  c.f() // expected-warning{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
 }
 
 func testCallsWithAsync() async {
@@ -338,3 +338,47 @@ func testSendableMetatypeDowngrades() {
     acceptsSendableMetatype(t) // Ok (because P is `Sendable`
   }
 }
+
+// Test that Sendable issues are downgraded to warnings in `@preconcurrency` context.
+do {
+  @preconcurrency nonisolated func f(callback: @Sendable () -> Void) {}
+
+  @MainActor
+  struct Env {
+    var v: Int = 0
+  }
+
+  @MainActor
+  class IsolatedTest {
+    var env = Env()
+    // expected-note@-1 {{property declared here}}
+
+    func onMain() {}
+    // expected-note@-1 {{calls to instance method 'onMain()' from outside of its actor context are implicitly asynchronous}}
+
+    func testProperty() {
+      f {
+        _ = self.env.v
+        // expected-warning@-1 {{main actor-isolated property 'env' can not be referenced from a Sendable closure}}
+      }
+    }
+
+    func testMethod() {
+      f {
+        self.onMain()
+        // expected-warning@-1 {{call to main actor-isolated instance method 'onMain()' in a synchronous nonisolated context}}
+      }
+    }
+  }
+
+  class Test { // expected-note {{class 'Test' does not conform to the 'Sendable' protocol}}
+    var env = Env()
+
+    func testProperty() async {
+      f {
+        _ = self.env.v
+        // expected-warning@-1 {{capture of 'self' with non-Sendable type 'Test' in a '@Sendable' closure}}
+      }
+    }
+  }
+}