[Concurrency] Diagnose default values whose isolation differs from the corresponding

stored property.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -5346,7 +5346,7 @@ ERROR(distributed_actor_isolated_non_self_reference,none,
       "non-isolated context",
       (const ValueDecl *))
 ERROR(isolated_default_argument_context,none,
-      "%0 default argument in a %1 context", 
+      "%0 default value in a %1 context", 
       (ActorIsolation, ActorIsolation))
 ERROR(conflicting_default_argument_isolation,none,
       "default argument cannot be both %0 and %1",

lib/AST/Decl.cpp

@@ -10613,6 +10613,7 @@ ActorIsolation swift::getActorIsolationOfContext(
     DeclContext *dc,
     llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
         getClosureActorIsolation) {
+  auto &ctx = dc->getASTContext();
   auto dcToUse = dc;
   // Defer bodies share actor isolation of their enclosing context.
   if (auto FD = dyn_cast<FuncDecl>(dcToUse)) {
@@ -10633,6 +10634,13 @@ ActorIsolation swift::getActorIsolationOfContext(
   //     actor isolation as the field itself. That default expression may only
   //     be used from inits that meet the required isolation.
   if (auto *var = dcToUse->getNonLocalVarDecl()) {
+    // If IsolatedDefaultValues are enabled, treat this context as having
+    // unspecified isolation. We'll compute the required isolation for
+    // the initializer and validate that it matches the isolation of the
+    // var itself in the DefaultInitializerIsolation request.
+    if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues))
+      return ActorIsolation::forUnspecified();
+
     return getActorIsolation(var);
   }
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -3143,7 +3143,10 @@ namespace {
       if (!unsatisfiedIsolation)
         return false;
 
-      if (refineRequiredIsolation(*unsatisfiedIsolation))
+      bool onlyArgsCrossIsolation = callOptions.contains(
+          ActorReferenceResult::Flags::OnlyArgsCrossIsolation);
+      if (!onlyArgsCrossIsolation &&
+          refineRequiredIsolation(*unsatisfiedIsolation))
         return false;
 
       // At this point, we know a jump is made to the callee that yields
@@ -4757,6 +4760,7 @@ DefaultInitializerIsolation::evaluate(Evaluator &evaluator,
 
   Initializer *dc = nullptr;
   Expr *initExpr = nullptr;
+  ActorIsolation enclosingIsolation;
 
   if (auto *pbd = var->getParentPatternBinding()) {
     if (!var->isParentInitialized())
@@ -4765,6 +4769,7 @@ DefaultInitializerIsolation::evaluate(Evaluator &evaluator,
     auto i = pbd->getPatternEntryIndexForVarDecl(var);
     dc = cast<Initializer>(pbd->getInitContext(i));
     initExpr = var->getParentInitializer();
+    enclosingIsolation = getActorIsolation(var);
   } else if (auto *param = dyn_cast<ParamDecl>(var)) {
     // If this parameter corresponds to a stored property for a
     // memberwise initializer, the default argument is the default
@@ -4782,13 +4787,27 @@ DefaultInitializerIsolation::evaluate(Evaluator &evaluator,
 
     dc = param->getDefaultArgumentInitContext();
     initExpr = param->getTypeCheckedDefaultExpr();
+    enclosingIsolation =
+        getActorIsolationOfContext(param->getDeclContext());
   }
 
   if (!dc || !initExpr)
     return ActorIsolation::forUnspecified();
 
+  // If the default argument has isolation, it must match the
+  // isolation of the decl context.
   ActorIsolationChecker checker(dc);
-  return checker.computeRequiredIsolation(initExpr);
+  auto requiredIsolation = checker.computeRequiredIsolation(initExpr);
+  if (requiredIsolation.isActorIsolated()) {
+    if (enclosingIsolation != requiredIsolation) {
+      var->diagnose(
+          diag::isolated_default_argument_context,
+          requiredIsolation, enclosingIsolation);
+      return ActorIsolation::forUnspecified();
+    }
+  }
+
+  return requiredIsolation;
 }
 
 void swift::checkOverrideActorIsolation(ValueDecl *value) {
@@ -5859,8 +5878,8 @@ bool swift::isAccessibleAcrossActors(
 }
 
 ActorReferenceResult ActorReferenceResult::forSameConcurrencyDomain(
-    ActorIsolation isolation) {
-  return ActorReferenceResult{SameConcurrencyDomain, llvm::None, isolation};
+    ActorIsolation isolation, Options options) {
+  return ActorReferenceResult{SameConcurrencyDomain, options, isolation};
 }
 
 ActorReferenceResult ActorReferenceResult::forEntersActor(
@@ -5869,8 +5888,8 @@ ActorReferenceResult ActorReferenceResult::forEntersActor(
 }
 
 ActorReferenceResult ActorReferenceResult::forExitsActorToNonisolated(
-    ActorIsolation isolation) {
-  return ActorReferenceResult{ExitsActorToNonisolated, llvm::None, isolation};
+    ActorIsolation isolation, Options options) {
+  return ActorReferenceResult{ExitsActorToNonisolated, options, isolation};
 }
 
 // Determine if two actor isolation contexts are considered to be equivalent.
@@ -5906,10 +5925,24 @@ ActorReferenceResult ActorReferenceResult::forReference(
       declIsolation = declIsolation.subst(declRef.getSubstitutions());
   }
 
+  // Determine what adjustments we need to perform for cross-actor
+  // references.
+  Options options = llvm::None;
+
+  // FIXME: Actor constructors are modeled as isolated to the actor
+  // so that Sendable checking is applied to their arguments, but the
+  // call itself does not hop to another executor.
+  if (auto ctor = dyn_cast<ConstructorDecl>(declRef.getDecl())) {
+    if (auto nominal = ctor->getDeclContext()->getSelfNominalTypeDecl()) {
+      if (nominal->isAnyActor())
+        options |= Flags::OnlyArgsCrossIsolation;
+    }
+  }
+
   // If the entity we are referencing is not a value, we're in the same
   // concurrency domain.
   if (isNonValueReference(declRef.getDecl()))
-    return forSameConcurrencyDomain(declIsolation);
+    return forSameConcurrencyDomain(declIsolation, options);
 
   // Compute the isolation of the context, if not provided.
   ActorIsolation contextIsolation = ActorIsolation::forUnspecified();
@@ -5928,11 +5961,11 @@ ActorReferenceResult ActorReferenceResult::forReference(
     if (isAsyncDecl(declRef) && contextIsolation.isActorIsolated() &&
         !declRef.getDecl()->getAttrs()
             .hasAttribute<UnsafeInheritExecutorAttr>())
-      return forExitsActorToNonisolated(contextIsolation);
+      return forExitsActorToNonisolated(contextIsolation, options);
 
     // Otherwise, we stay in the same concurrency domain, whether on an actor
     // or in a task.
-    return forSameConcurrencyDomain(declIsolation);
+    return forSameConcurrencyDomain(declIsolation, options);
   }
 
   // The declaration we are accessing is actor-isolated. First, check whether
@@ -5941,11 +5974,11 @@ ActorReferenceResult ActorReferenceResult::forReference(
       declIsolation.getActorInstanceParameter() == 0) {
     // If this instance is isolated, we're in the same concurrency domain.
     if (actorInstance->isIsolated())
-      return forSameConcurrencyDomain(declIsolation);
+      return forSameConcurrencyDomain(declIsolation, options);
   } else if (equivalentIsolationContexts(declIsolation, contextIsolation)) {
     // The context isolation matches, so we are in the same concurrency
     // domain.
-    return forSameConcurrencyDomain(declIsolation);
+    return forSameConcurrencyDomain(declIsolation, options);
   }
 
   // Initializing an actor isolated stored property with a value effectively
@@ -5965,7 +5998,8 @@ 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.
-        return forEntersActor(declIsolation, Flags::Preconcurrency);
+        options |= Flags::Preconcurrency;
+        return forEntersActor(declIsolation, options);
       }
     }
   }
@@ -5975,7 +6009,7 @@ ActorReferenceResult ActorReferenceResult::forReference(
   if (actorInstance &&
       checkedByFlowIsolation(
           fromDC, *actorInstance, declRef.getDecl(), declRefLoc, useKind))
-    return forSameConcurrencyDomain(declIsolation);
+    return forSameConcurrencyDomain(declIsolation, options);
 
   // If we are delegating to another initializer, treat them as being in the
   // same concurrency domain.
@@ -5984,7 +6018,7 @@ ActorReferenceResult ActorReferenceResult::forReference(
   if (actorInstance && actorInstance->isSelf() &&
       isa<ConstructorDecl>(declRef.getDecl()) &&
       isa<ConstructorDecl>(fromDC))
-    return forSameConcurrencyDomain(declIsolation);
+    return forSameConcurrencyDomain(declIsolation, options);
 
   // If there is an instance that corresponds to 'self',
   // we are in a constructor or destructor, and we have a stored property of
@@ -5994,18 +6028,14 @@ ActorReferenceResult ActorReferenceResult::forReference(
       isNonInheritedStorage(declRef.getDecl(), fromDC) &&
       declIsolation.isGlobalActor() &&
       (isa<ConstructorDecl>(fromDC) || isa<DestructorDecl>(fromDC)))
-    return forSameConcurrencyDomain(declIsolation);
-
-  // Determine what adjustments we need to perform for cross-actor
-  // references.
-  Options options = llvm::None;
+    return forSameConcurrencyDomain(declIsolation, options);
 
   // At this point, we are accessing the target from outside the actor.
   // First, check whether it is something that can be accessed directly,
   // without any kind of promotion.
   if (isAccessibleAcrossActors(
           declRef.getDecl(), declIsolation, fromDC, options, actorInstance))
-    return forEntersActor(declIsolation, llvm::None);
+    return forEntersActor(declIsolation, options);
 
   // This is a cross-actor reference.
 

lib/Sema/TypeCheckConcurrency.h

@@ -202,6 +202,10 @@ struct ActorReferenceResult {
 
     /// The declaration is 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,
   };
 
   using Options = OptionSet<Flags>;
@@ -212,13 +216,13 @@ struct ActorReferenceResult {
 
 private:
   static ActorReferenceResult forSameConcurrencyDomain(
-      ActorIsolation isolation);
+      ActorIsolation isolation, Options options = llvm::None);
 
   static ActorReferenceResult forEntersActor(
       ActorIsolation isolation, Options options);
 
   static ActorReferenceResult forExitsActorToNonisolated(
-      ActorIsolation isolation);
+      ActorIsolation isolation, Options options = llvm::None);
 
 public:
   /// Determine what happens when referencing the given declaration from the

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -1055,15 +1055,7 @@ static void checkDefaultArguments(ParameterList *params) {
 
     // If the default argument has isolation, it must match the
     // isolation of the decl context.
-    auto defaultArgIsolation = param->getInitializerIsolation();
-    if (defaultArgIsolation.isActorIsolated()) {
-      auto *dc = param->getDeclContext();
-      auto enclosingIsolation = getActorIsolationOfContext(dc);
-      if (enclosingIsolation != defaultArgIsolation) {
-        param->diagnose(diag::isolated_default_argument_context,
-            defaultArgIsolation, enclosingIsolation);
-      }
-    }
+    (void)param->getInitializerIsolation();
 
     if (!ifacety->hasPlaceholder()) {
       continue;

test/Concurrency/isolated_default_arguments.swift

@@ -20,11 +20,11 @@ func requiresMainActor() -> Int { 0 }
 @SomeGlobalActor
 func requiresSomeGlobalActor() -> Int { 0 }
 
-// expected-error@+1 {{main actor-isolated default argument in a nonisolated context}}
+// expected-error@+1 {{main actor-isolated default value in a nonisolated context}}
 func mainActorDefaultArgInvalid(value: Int = requiresMainActor()) {}
 
 func mainActorClosureInvalid(
-  closure: () -> Int = { // expected-error {{main actor-isolated default argument in a nonisolated context}}
+  closure: () -> Int = { // expected-error {{main actor-isolated default value in a nonisolated context}}
     requiresMainActor()
   }
 ) {}
@@ -41,7 +41,7 @@ func mainActorDefaultArg(value: Int = requiresMainActor()) {}
 ) {}
 
 func mainActorClosureCall(
-  closure: Int = { // expected-error {{main actor-isolated default argument in a nonisolated context}}
+  closure: Int = { // expected-error {{main actor-isolated default value in a nonisolated context}}
     requiresMainActor()
   }()
 ) {}
@@ -155,6 +155,11 @@ struct S3 {
   var (x, y, z) = (requiresMainActor(), requiresSomeGlobalActor(), 10)
 }
 
+struct S4 {
+  // expected-error@+1 {{main actor-isolated default value in a nonisolated context}}
+  var x: Int = requiresMainActor()
+}
+
 @MainActor
 func initializeFromMainActor() {
   _ = S1(required: 10)

test/Concurrency/isolated_default_property_inits.swift

@@ -19,11 +19,11 @@ func requiresMainActor() -> Int { 0 }
 @SomeGlobalActor
 func requiresSomeGlobalActor() -> Int { 0 }
 
-struct S1 {
+class C1 {
   // expected-note@+1 2 {{'self.x' not initialized}}
-  var x = requiresMainActor()
+  @MainActor var x = requiresMainActor()
   // expected-note@+1 2 {{'self.y' not initialized}}
-  var y = requiresSomeGlobalActor()
+  @SomeGlobalActor var y = requiresSomeGlobalActor()
   var z = 10
 
   // expected-error@+1 {{return from initializer without initializing all stored properties}}
@@ -36,9 +36,9 @@ struct S1 {
   @SomeGlobalActor init(c: Int) {}
 }
 
-struct S2 {
-  var x = requiresMainActor()
-  var y = requiresSomeGlobalActor()
+class C2 {
+  @MainActor var x = requiresMainActor()
+  @SomeGlobalActor var y = requiresSomeGlobalActor()
   var z = 10
 
   nonisolated init(x: Int, y: Int) {