Merge pull request #37304 from DougGregor/revert-nonisolated

[Concurrency] Revert 'nonisolated let' change.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4448,9 +4448,6 @@ NOTE(actor_isolated_sync_func,none,
 NOTE(actor_mutable_state,none,
      "mutation of this %0 is only permitted within the actor",
      (DescriptiveDeclKind))
-NOTE(actor_isolated_let,none,
-     "use `nonisolated` to allow synchronous access to 'let' from outside "
-     "the actor", ())
 WARNING(shared_mutable_state_access,none,
         "reference to %0 %1 is not concurrency-safe because it involves "
         "shared mutable state", (DescriptiveDeclKind, DeclName))
@@ -4483,12 +4480,13 @@ WARNING(non_concurrent_property_type,none,
 WARNING(non_concurrent_keypath_capture,none,
         "cannot form key path that captures non-sendable type %0",
         (Type))
+WARNING(non_concurrent_keypath_access,none,
+        "cannot form key path that accesses non-sendable type %0",
+        (Type))
 ERROR(non_concurrent_type_member,none,
       "%select{stored property %1|associated value %1}0 of "
       "'Sendable'-conforming %2 %3 has non-sendable type %4",
       (bool, DeclName, DescriptiveDeclKind, DeclName, Type))
-ERROR(non_sendable_nonisolated_let,none,
-      "non-isolated let property %0 has non-Sendable type %1", (DeclName, Type))
 ERROR(concurrent_value_class_mutable_property,none,
       "stored property %0 of 'Sendable'-conforming %1 %2 is mutable",
       (DeclName, DescriptiveDeclKind, DeclName))
@@ -4504,13 +4502,21 @@ ERROR(concurrent_value_inherit,none,
       "%select{| other than 'NSObject'}0",
       (bool, DeclName))
 
+ERROR(actorindependent_let,none,
+      "'@actorIndependent' is meaningless on 'let' declarations because "
+      "they are immutable",
+      ())
 ERROR(actorindependent_mutable_storage,none,
       "'@actorIndependent' can not be applied to stored properties",
       ())
 ERROR(actorindependent_local_var,none,
       "'@actorIndependent' can not be applied to local variables",
       ())
 
+ERROR(nonisolated_let,none,
+      "'nonisolated' is meaningless on 'let' declarations because "
+      "they are immutable",
+      ())
 ERROR(nonisolated_mutable_storage,none,
       "nonisolated' can not be applied to stored properties",
       ())

lib/Sema/TypeCheckAttr.cpp

@@ -5420,14 +5420,17 @@ void AttributeChecker::visitActorIndependentAttr(ActorIndependentAttr *attr) {
   // that do not have storage.
   auto dc = D->getDeclContext();
   if (auto var = dyn_cast<VarDecl>(D)) {
-    // @actorIndependent can not be applied to mutable stored properties, unless if
+    // @actorIndependent is meaningless on a `let`.
+    if (var->isLet()) {
+      diagnoseAndRemoveAttr(attr, diag::actorindependent_let);
+      return;
+    }
+
+    // @actorIndependent can not be applied to stored properties, unless if
     // the 'unsafe' option was specified
     if (var->hasStorage()) {
       switch (attr->getKind()) {
         case ActorIndependentKind::Safe:
-          if (var->isLet())
-            break;
-
           diagnoseAndRemoveAttr(attr, diag::actorindependent_mutable_storage);
           return;
 
@@ -5459,20 +5462,16 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
   // that do not have storage.
   auto dc = D->getDeclContext();
   if (auto var = dyn_cast<VarDecl>(D)) {
-    // 'nonisolated' can only be applied to 'let' stored properties.
-    // Those must be Sendable.
-    if (var->hasStorage()) {
-      if (!var->isLet()) {
-        diagnoseAndRemoveAttr(attr, diag::nonisolated_mutable_storage);
-        return;
-      }
+    // 'nonisolated' is meaningless on a `let`.
+    if (var->isLet()) {
+      diagnoseAndRemoveAttr(attr, diag::nonisolated_let);
+      return;
+    }
 
-      // nonisolated lets must have Sendable type.
-      if (shouldDiagnoseNonSendableViolations(dc->getASTContext().LangOpts) &&
-          !isSendableType(dc, var->getType())) {
-        var->diagnose(
-            diag::non_sendable_nonisolated_let, var->getName(), var->getType());
-      }
+    // 'nonisolated' can not be applied to stored properties.
+    if (var->hasStorage()) {
+      diagnoseAndRemoveAttr(attr, diag::nonisolated_mutable_storage);
+      return;
     }
 
     // @actorIndependent can not be applied to local properties.

lib/Sema/TypeCheckConcurrency.cpp

@@ -602,14 +602,21 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
     if (cast<ValueDecl>(decl)->isLocalCapture())
       return forUnrestricted();
 
+    // 'let' declarations are immutable, so they can be accessed across
+    // actors.
+    bool isAccessibleAcrossActors = false;
+    if (auto var = dyn_cast<VarDecl>(decl)) {
+      if (var->isLet())
+        isAccessibleAcrossActors = true;
+    }
+
     // A function that provides an asynchronous context has no restrictions
     // on its access.
     //
     // FIXME: technically, synchronous functions are allowed to be cross-actor.
     // The call-sites are just conditionally async based on where they appear
     // (outside or inside the actor). This suggests that the implicitly-async
     // concept could be merged into the CrossActorSelf concept.
-    bool isAccessibleAcrossActors = false;
     if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
       if (func->isAsyncContext())
         isAccessibleAcrossActors = true;
@@ -922,7 +929,8 @@ static bool diagnoseNonConcurrentProperty(
 
 /// Whether we should diagnose cases where Sendable conformances are
 /// missing.
-bool swift::shouldDiagnoseNonSendableViolations(const LangOptions &langOpts) {
+static bool shouldDiagnoseNonSendableViolations(
+    const LangOptions &langOpts) {
   return langOpts.WarnConcurrency;
 }
 
@@ -1426,17 +1434,11 @@ namespace {
         // was it an attempt to mutate an actor instance's isolated state?
       } else if (auto environment = kindOfUsage(decl, context)) {
 
-        if (isa<VarDecl>(decl) && cast<VarDecl>(decl)->isLet()) {
-          auto diag = decl->diagnose(diag::actor_isolated_let);
-          SourceLoc fixItLoc =
-              decl->getAttributeInsertionLoc(/*forModifier=*/true);
-          if (fixItLoc.isValid())
-            diag.fixItInsert(fixItLoc, "nonisolated ");
-        } else if (environment.getValue() == VarRefUseEnv::Read) {
+        if (environment.getValue() == VarRefUseEnv::Read)
           decl->diagnose(diag::kind_declared_here, decl->getDescriptiveKind());
-        } else {
+        else
           decl->diagnose(diag::actor_mutable_state, decl->getDescriptiveKind());
-        }
+
       } else {
         decl->diagnose(diag::kind_declared_here, decl->getDescriptiveKind());
       }
@@ -1609,6 +1611,11 @@ namespace {
 
       // is it an access to a property?
       if (isPropOrSubscript(decl)) {
+        // we assume let-bound properties are taken care of elsewhere,
+        // since they are never implicitly async.
+        assert(!isa<VarDecl>(decl) || cast<VarDecl>(decl)->isLet() == false
+               && "unexpected let-bound property; never implicitly async!");
+
         if (auto declRef = dyn_cast_or_null<DeclRefExpr>(context)) {
           if (usageEnv(declRef) == VarRefUseEnv::Read) {
 
@@ -1943,6 +1950,27 @@ namespace {
     bool checkKeyPathExpr(KeyPathExpr *keyPath) {
       bool diagnosed = false;
 
+      // returns None if it is not a 'let'-bound var decl. Otherwise,
+      // the bool indicates whether a diagnostic was emitted.
+      auto checkLetBoundVarDecl = [&](KeyPathExpr::Component const& component)
+                                                            -> Optional<bool> {
+        auto decl = component.getDeclRef().getDecl();
+        if (auto varDecl = dyn_cast<VarDecl>(decl)) {
+          if (varDecl->isLet()) {
+            auto type = component.getComponentType();
+            if (shouldDiagnoseNonSendableViolations(ctx.LangOpts)
+                && !isSendableType(getDeclContext(), type)) {
+              ctx.Diags.diagnose(
+                  component.getLoc(), diag::non_concurrent_keypath_access,
+                  type);
+              return true;
+            }
+            return false;
+          }
+        }
+        return None;
+      };
+
       // check the components of the keypath.
       for (const auto &component : keyPath->getComponents()) {
         // The decl referred to by the path component cannot be within an actor.
@@ -1970,6 +1998,13 @@ namespace {
             LLVM_FALLTHROUGH; // otherwise, it's invalid so diagnose it.
 
           case ActorIsolationRestriction::CrossActorSelf:
+            // 'let'-bound decls with this isolation are OK, just check them.
+            if (auto wasLetBound = checkLetBoundVarDecl(component)) {
+              diagnosed = wasLetBound.getValue();
+              break;
+            }
+            LLVM_FALLTHROUGH; // otherwise, it's invalid so diagnose it.
+
           case ActorIsolationRestriction::ActorSelf: {
             auto decl = concDecl.getDecl();
             ctx.Diags.diagnose(component.getLoc(),

lib/Sema/TypeCheckConcurrency.h

@@ -38,7 +38,6 @@ class EnumElementDecl;
 class Expr;
 class FuncDecl;
 class Initializer;
-class LangOptions;
 class PatternBindingDecl;
 class ProtocolConformance;
 class TopLevelCodeDecl;
@@ -214,10 +213,6 @@ bool diagnoseNonConcurrentTypesInReference(
     ConcurrentReferenceKind refKind,
     DiagnosticBehavior behavior = DiagnosticBehavior::Unspecified);
 
-/// Whether we should diagnose cases where Sendable conformances are
-/// missing.
-bool shouldDiagnoseNonSendableViolations(const LangOptions &langOpts);
-
 /// How the concurrent value check should be performed.
 enum class SendableCheck {
   /// Sendable conformance was explicitly stated and should be

test/Concurrency/Runtime/actor_keypaths.swift

@@ -7,7 +7,7 @@
 // UNSUPPORTED: back_deployment_runtime
 
 actor Page {
-    nonisolated let initialNumWords : Int
+    let initialNumWords : Int
 
     @actorIndependent(unsafe)
     var numWords : Int
@@ -19,7 +19,7 @@ actor Page {
 }
 
 actor Book {
-    nonisolated let pages : [Page]
+    let pages : [Page]
 
     init(_ numPages : Int) {
         var stack : [Page] = []

test/Concurrency/Runtime/custom_executors.swift

@@ -17,7 +17,7 @@ actor Simple {
 
 actor Custom {
   var count = 0
-  nonisolated let simple = Simple()
+  let simple = Simple()
 
   @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
   nonisolated var unownedExecutor: UnownedSerialExecutor {

test/Concurrency/actor_isolation.swift

@@ -49,7 +49,7 @@ class Point {
 
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
 actor MyActor: MySuperActor { // expected-error{{actor types do not support inheritance}}
-  nonisolated let immutable: Int = 17
+  let immutable: Int = 17
   // expected-note@+2 2{{property declared here}}
   // expected-note@+1 6{{mutation of this property is only permitted within the actor}}
   var mutable: Int = 71
@@ -58,8 +58,7 @@ actor MyActor: MySuperActor { // expected-error{{actor types do not support inhe
   // expected-note@+1 4{{property declared here}}
   var text: [String] = []
 
-  nonisolated let point : Point = Point() // expected-error{{non-isolated let property 'point' has non-Sendable type 'Point'}}
-  let otherPoint = Point()
+  let point : Point = Point()
 
   @MainActor
   var name : String = "koala" // expected-note{{property declared here}}
@@ -103,12 +102,7 @@ func checkAsyncPropertyAccess() async {
 
   act.text[0] += "hello" // expected-error{{actor-isolated property 'text' can only be mutated from inside the actor}}
 
-  _ = act.point
-  _ = act.otherPoint
-  // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  // expected-note@-2{{property access is 'async'}}
-  // expected-warning@-3{{cannot use property 'otherPoint' with a non-sendable type 'Point' across actors}}
-  _ = await act.otherPoint // expected-warning{{cannot use property 'otherPoint' with a non-sendable type 'Point' across actors}}
+  _ = act.point  // expected-warning{{cannot use property 'point' with a non-sendable type 'Point' across actors}}
 }
 
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
@@ -652,7 +646,7 @@ actor LazyActor {
     var v: Int = 0
     // expected-note@-1 6 {{property declared here}}
 
-    nonisolated let l: Int = 0
+    let l: Int = 0
 
     lazy var l11: Int = { v }()
     lazy var l12: Int = v

test/Concurrency/actor_isolation_objc.swift

@@ -28,7 +28,7 @@ actor A {
     _ = #keyPath(A.z)
   }
 
-  nonisolated let w: Int = 0 // expected-note{{add '@objc' to expose this property to Objective-C}}
+  let w: Int = 0 // expected-note{{add '@objc' to expose this property to Objective-C}}
 
   var x: Int = 0 // expected-note{{add '@objc' to expose this property to Objective-C}}
 

test/Concurrency/actor_keypath_isolation.swift

@@ -6,10 +6,10 @@ class Box {
 }
 
 actor Door {
-    nonisolated let immutable : Int = 0
+    let immutable : Int = 0
     let letBox : Box? = nil
     let letDict : [Int : Box] = [:]
-    nonisolated let immutableNeighbor : Door? = nil
+    let immutableNeighbor : Door? = nil
 
 
     var mutableNeighbor : Door? = nil
@@ -47,7 +47,7 @@ func tryKeyPathsMisc(d : Door) {
 
     // in combination with other key paths
 
-    _ = (\Door.letBox).appending(path:  // expected-error {{cannot form key path to actor-isolated property 'letBox'}}
+    _ = (\Door.letBox).appending(path:  // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
                                        \Box?.?.size)
 
     _ = (\Door.varBox).appending(path:  // expected-error {{cannot form key path to actor-isolated property 'varBox'}}
@@ -61,9 +61,9 @@ func tryKeyPathsFromAsync() async {
 }
 
 func tryNonSendable() {
-    _ = \Door.letDict[0] // expected-error {{cannot form key path to actor-isolated property 'letDict'}}
+    _ = \Door.letDict[0] // expected-warning {{cannot form key path that accesses non-sendable type '[Int : Box]'}}
     _ = \Door.varDict[0] // expected-error {{cannot form key path to actor-isolated property 'varDict'}}
-    _ = \Door.letBox!.size // expected-error {{cannot form key path to actor-isolated property 'letBox'}}
+    _ = \Door.letBox!.size // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
 }
 
 func tryKeypaths() {

test/Concurrency/concurrent_value_checking.swift

@@ -37,7 +37,7 @@ extension A1 {
     _ = await self.asynchronous(nil)
 
     // Across to a different actor, so Sendable restriction is enforced.
-    _ = await other.localLet // expected-warning{{cannot use property 'localLet' with a non-sendable type 'NotConcurrent' across actors}}
+    _ = other.localLet // expected-warning{{cannot use property 'localLet' with a non-sendable type 'NotConcurrent' across actors}}
     _ = await other.synchronous() // expected-warning{{cannot call function returning non-sendable type 'NotConcurrent?' across actors}}
     _ = await other.asynchronous(nil) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   }
@@ -67,7 +67,7 @@ func globalAsync(_: NotConcurrent?) async {
 }
 
 func globalTest() async {
-  let a = await globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
+  let a = globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
   await globalAsync(a) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   await globalSync(a)  // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
 }
@@ -88,7 +88,7 @@ class ClassWithGlobalActorInits {
 
 @MainActor
 func globalTestMain(nc: NotConcurrent) async {
-  let a = await globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
+  let a = globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
   await globalAsync(a) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   await globalSync(a)  // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   _ = await ClassWithGlobalActorInits(nc) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent' across actors}}

test/Concurrency/global_actor_from_ordinary_context.swift

@@ -31,8 +31,7 @@ actor Alex {
 func referenceGlobalActor() async {
   let a = Alex()
   _ = a.method
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  _ = a.const_memb // expected-note{{property access is 'async'}}
+  _ = a.const_memb
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
   _ = a.mut_memb // expected-note{{property access is 'async'}}
 
@@ -117,14 +116,12 @@ func fromAsync() async {
   y() // expected-note{{call is 'async'}}
 
   let a = Alex()
-
   let fn = a.method
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  fn() //expected-note{{calls to let 'fn' from outside of its actor context are implicitly asynchronous}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  _ = a.const_memb // expected-note{{property access is 'async'}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  _ = a.mut_memb  // expected-note{{property access is 'async'}}
+  fn() // expected-error{{expression is 'async' but is not marked with 'await'}}
+  // expected-note@-1{{calls to let 'fn' from outside of its actor context are implicitly asynchronous}}
+  _ = a.const_memb
+  _ = a.mut_memb  // expected-error{{expression is 'async' but is not marked with 'await'}}
+  // expected-note@-1{{property access is 'async'}}
 
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
   _ = a[1]  // expected-note{{subscript access is 'async'}}