Merge pull request swiftlang#40908 from kavon/property-initializer-isolation-warning

[SE-327] Reimplement: make initializing expressions for member stored properties nonisolated

Author: kavon

include/swift/AST/DeclContext.h

@@ -73,6 +73,7 @@ namespace swift {
   class PrefixOperatorDecl;
   class ProtocolConformance;
   class ValueDecl;
+  class VarDecl;
   class Initializer;
   class ClassDecl;
   class SerializedAbstractClosureExpr;
@@ -355,6 +356,11 @@ class alignas(1 << DeclContextAlignInBits) DeclContext
   LLVM_READONLY
   ProtocolDecl *getExtendedProtocolDecl() const;
 
+  /// If this DeclContext is the initializer expression of a global or instance
+  /// property, return the VarDecl, otherwise return null.
+  LLVM_READONLY
+  VarDecl *getNonLocalVarDecl() const;
+
   /// Retrieve the generic parameter 'Self' from a protocol or
   /// protocol extension.
   ///

include/swift/AST/DiagnosticsSema.def

@@ -4480,6 +4480,10 @@ ERROR(global_actor_from_nonactor_context,none,
       "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
       " from %select{this|a non-isolated}3%select{| synchronous}5 context",
       (DescriptiveDeclKind, DeclName, Type, bool, unsigned, bool))
+ERROR(global_actor_from_initializing_expr,none,
+      "expression requiring global actor %0 cannot appear in "
+        "default-value expression of %1 %2",
+      (Type, DescriptiveDeclKind, DeclName))
 ERROR(actor_isolated_call,none,
       "call to %0 function in a synchronous %1 context",
       (ActorIsolation, ActorIsolation))

lib/AST/Decl.cpp

@@ -8792,16 +8792,16 @@ ActorIsolation swift::getActorIsolationOfContext(DeclContext *dc) {
   // Without this distinction, a nominal can have non-async initializers
   // with various kinds of isolation, so an impossible constraint can be
   // created. See SE-0327 for details.
-  if (auto *init = dyn_cast<PatternBindingInitializer>(dc)) {
-    if (auto *var =
-          init->getBinding()->getAnchoringVarDecl(init->getBindingIndex())) {
+  if (auto *var = dc->getNonLocalVarDecl()) {
 
-      if (var->isInstanceMember() &&
-          !var->getAttrs().hasAttribute<LazyAttr>())
-        return ActorIsolation::forUnspecified();
-
-      return getActorIsolation(var);
+    // Isolation officially changes, as described above, in Swift 6+
+    if (dc->getASTContext().isSwiftVersionAtLeast(6) &&
+        var->isInstanceMember() &&
+        !var->getAttrs().hasAttribute<LazyAttr>()) {
+      return ActorIsolation::forUnspecified();
     }
+
+     return getActorIsolation(var);
   }
 
   if (auto *closure = dyn_cast<AbstractClosureExpr>(dc)) {

lib/AST/DeclContext.cpp

@@ -83,6 +83,16 @@ ProtocolDecl *DeclContext::getExtendedProtocolDecl() const {
   return nullptr;
 }
 
+VarDecl *DeclContext::getNonLocalVarDecl() const {
+  if (auto *init = dyn_cast<PatternBindingInitializer>(this)) {
+   if (auto *var =
+         init->getBinding()->getAnchoringVarDecl(init->getBindingIndex())) {
+      return var;
+     }
+  }
+  return nullptr;
+}
+
 GenericTypeParamType *DeclContext::getProtocolSelfType() const {
   assert(getSelfProtocolDecl() && "not a protocol");
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -1275,6 +1275,57 @@ namespace {
       return getDeclContext()->getParentModule();
     }
 
+    /// In Swift 6, global-actor isolation is not carried-over to the
+    /// initializing expressions of non-static instance properties.
+    /// The actual change happens in \c getActorIsolationOfContext ,
+    /// but this function exists to warn users of Swift 5 about this
+    /// isolation change, so that they can prepare ahead-of-time.
+    void warnAboutGlobalActorIsoChangeInSwift6(const ActorIsolation &reqIso,
+                                               const Expr *user) {
+      if (ctx.isSwiftVersionAtLeast(6))
+        return;
+
+      // Check our context stack for a PatternBindingInitializer environment.
+      DeclContext const* withinDC = nullptr;
+      for (auto dc = contextStack.rbegin(); dc != contextStack.rend(); dc++) {
+        if (isa<PatternBindingInitializer>(*dc)) {
+          withinDC = *dc;
+          break;
+        }
+      }
+
+      // Not within a relevant decl context.
+      if (!withinDC)
+        return;
+
+      // Check if this PatternBindingInitializer's isolation would change
+      // in Swift 6+
+      if (auto *var = withinDC->getNonLocalVarDecl()) {
+        if (var->isInstanceMember() &&
+            !var->getAttrs().hasAttribute<LazyAttr>()) {
+          // At this point, we know the isolation will change in Swift 6.
+          // So, let's check if that change will cause an error.
+
+          auto dcIso = getActorIsolationOfContext(
+                         const_cast<DeclContext*>(withinDC));
+
+          // If the isolation granted in Swift 5 is for a global actor, and
+          // the expression requires that global actor's isolation, then it will
+          // become an error in Swift 6.
+          if (dcIso.isGlobalActor() && dcIso == reqIso) {
+            ctx.Diags.diagnose(user->getLoc(),
+                               diag::global_actor_from_initializing_expr,
+                               reqIso.getGlobalActor(),
+                               var->getDescriptiveKind(), var->getName())
+            .highlight(user->getSourceRange())
+            // make it a warning and attach the "this will become an error..."
+            // to the message. The error in Swift 6 will not be this diagnostic.
+            .warnUntilSwiftVersion(6);
+          }
+        }
+      }
+    }
+
     /// Determine whether code in the given use context might execute
     /// concurrently with code in the definition context.
     bool mayExecuteConcurrentlyWith(
@@ -1919,7 +1970,7 @@ namespace {
       // Retrieve the actor isolation of the context.
       switch (auto isolation = getActorIsolationOfContext(dc)) {
       case ActorIsolation::ActorInstance:
-        case ActorIsolation::DistributedActorInstance:
+      case ActorIsolation::DistributedActorInstance:
       case ActorIsolation::Independent:
       case ActorIsolation::Unspecified:
         return isolation;
@@ -2097,8 +2148,12 @@ namespace {
       // we are within that global actor already.
       Optional<ActorIsolation> unsatisfiedIsolation;
       if (Type globalActor = fnType->getGlobalActor()) {
-        if (!getContextIsolation().isGlobalActor() ||
-            !getContextIsolation().getGlobalActor()->isEqual(globalActor)) {
+        if (getContextIsolation().isGlobalActor() &&
+            getContextIsolation().getGlobalActor()->isEqual(globalActor)) {
+          warnAboutGlobalActorIsoChangeInSwift6(
+              ActorIsolation::forGlobalActor(globalActor, false),
+              apply);
+        } else {
           unsatisfiedIsolation = ActorIsolation::forGlobalActor(
               globalActor, /*unsafe=*/false);
         }
@@ -2234,6 +2289,8 @@ namespace {
       auto contextIsolation = getInnermostIsolatedContext(declContext);
       if (contextIsolation.isGlobalActor() &&
           contextIsolation.getGlobalActor()->isEqual(globalActor)) {
+
+        warnAboutGlobalActorIsoChangeInSwift6(contextIsolation, context);
         return false;
       }
 

test/Concurrency/global_actor_inference.swift

@@ -563,10 +563,10 @@ func acceptAsyncSendableClosureInheriting<T>(@_inheritActorContext _: @Sendable
 
 // defer bodies inherit global actor-ness
 @MainActor
-var statefulThingy: Bool = false
+var statefulThingy: Bool = false // expected-note {{var declared here}}
 
 @MainActor
-func useFooInADefer() -> String {
+func useFooInADefer() -> String { // expected-note {{calls to global function 'useFooInADefer()' from outside of its actor context are implicitly asynchronous}}
   defer {
     statefulThingy = true
   }
@@ -581,3 +581,30 @@ func useFooInADefer() -> String {
 func replacesDynamicOnMainActor() {
   onlyOnMainActor()
 }
+
+// ----------------------------------------------------------------------
+// Global-actor isolation of stored property initializer expressions
+// ----------------------------------------------------------------------
+
+class Cutter {
+  @MainActor var x = useFooInADefer() // expected-warning {{expression requiring global actor 'MainActor' cannot appear in default-value expression of property 'x'; this is an error in Swift 6}}
+  @MainActor var y = { () -> Bool in
+      var z = statefulThingy // expected-warning {{expression requiring global actor 'MainActor' cannot appear in default-value expression of property 'y'; this is an error in Swift 6}}
+      return z
+    }()
+}
+
+@SomeGlobalActor
+class Butter {
+  var a = useFooInADefer() // expected-error {{call to main actor-isolated global function 'useFooInADefer()' in a synchronous global actor 'SomeGlobalActor'-isolated context}}
+
+  nonisolated let b = statefulThingy // expected-error {{var 'statefulThingy' isolated to global actor 'MainActor' can not be referenced from a non-isolated synchronous context}}
+
+  var c: Int = {
+    return getGlobal7() // expected-warning {{expression requiring global actor 'SomeGlobalActor' cannot appear in default-value expression of property 'c'; this is an error in Swift 6}}
+  }()
+
+  lazy var d: Int = getGlobal7()
+
+  static var e: Int = getGlobal7()
+}

test/Concurrency/property_initializers.swift renamed to test/Concurrency/property_initializers_swift6.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -disable-availability-checking -warn-concurrency
+// RUN: %target-typecheck-verify-swift -swift-version 6 -disable-availability-checking -warn-concurrency
 // REQUIRES: concurrency