[Concurrency] Handle nested decl contexts while computing the required isolation

for a default argument, and improve diagnostics for conflicting isolation.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -5194,6 +5194,9 @@ ERROR(isolated_default_argument,none,
       "%0 default argument cannot be synchronously evaluated from a "
       "%1 context",
       (ActorIsolation, ActorIsolation))
+ERROR(conflicting_default_argument_isolation,none,
+      "default argument cannot be both %0 and %1",
+      (ActorIsolation, ActorIsolation))
 ERROR(distributed_actor_needs_explicit_distributed_import,none,
       "'Distributed' module not imported, required for 'distributed actor'",
       ())

lib/Sema/TypeCheckConcurrency.cpp

@@ -1939,11 +1939,11 @@ namespace {
     llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
         getClosureActorIsolation;
 
-    bool shouldRefineIsolation = false;
+    SourceLoc requiredIsolationLoc;
 
     /// Used under the mode to compute required actor isolation for
     /// an expression or function.
-    ActorIsolation requiredIsolation = ActorIsolation::forNonisolated();
+    llvm::SmallDenseMap<const DeclContext *, ActorIsolation> requiredIsolation;
 
     /// Keeps track of the capture context of variables that have been
     /// explicitly captured in closures.
@@ -2130,31 +2130,60 @@ namespace {
     }
 
     bool refineRequiredIsolation(ActorIsolation refinedIsolation) {
-      if (!shouldRefineIsolation)
+      if (requiredIsolationLoc.isInvalid())
         return false;
 
-      if (auto *closure = dyn_cast<AbstractClosureExpr>(getDeclContext())) {
-        // We cannot infer a more specific actor isolation for a @Sendable
-        // closure. It is an error to cast away actor isolation from a function
-        // type, but this is okay for non-Sendable closures because they cannot
-        // leave the isolation domain they're created in anyway.
-        if (closure->isSendable())
-          return false;
+      auto infersIsolationFromContext =
+          [](const DeclContext *dc) -> bool {
+            // Isolation for declarations is based solely on explicit 
+            // annotations; only infer isolation for initializer expressions
+            // and closures.
+            if (dc->getAsDecl())
+              return false;
+
+            if (auto *closure = dyn_cast<AbstractClosureExpr>(dc)) {
+              // We cannot infer a more specific actor isolation for a Sendable
+              // closure. It is an error to cast away actor isolation from a
+              // function type, but this is okay for non-Sendable closures
+              // because they cannot leave the isolation domain they're created
+              // in anyway.
+              if (closure->isSendable())
+                return false;
 
-        if (closure->getActorIsolation().isActorIsolated())
+              if (closure->getActorIsolation().isActorIsolated())
+                return false;
+            }
+
+            return true;
+          };
+
+      // For the call to require the given actor isolation, every DeclContext
+      // in the current context stack must require the same isolation. If
+      // along the way to the innermost context, we find a DeclContext that
+      // has a different isolation (e.g. it's a local function that does not
+      // recieve isolation from its decl context), then the expression cannot
+      // require a different isolation.
+      for (auto *dc : contextStack) {
+        if (!infersIsolationFromContext(dc))
           return false;
-      }
 
-      // To refine the required isolation, the existing requirement
-      // must either be 'nonisolated' or exactly the same as the
-      // new refinement.
-      if (requiredIsolation == ActorIsolation::Nonisolated ||
-          requiredIsolation == refinedIsolation) {
-        requiredIsolation = refinedIsolation;
-        return true;
+        // To refine the required isolation, the existing requirement
+        // must either be 'nonisolated' or exactly the same as the
+        // new refinement.
+        auto isolation = requiredIsolation.find(dc);
+        if (isolation == requiredIsolation.end() ||
+            isolation->second == ActorIsolation::Nonisolated) {
+          requiredIsolation[dc] = refinedIsolation;
+        } else if (isolation->second != refinedIsolation) {
+          dc->getASTContext().Diags.diagnose(
+              requiredIsolationLoc,
+              diag::conflicting_default_argument_isolation,
+              isolation->second, refinedIsolation);
+          return true;
+        }
       }
 
-      return false;
+      return true;
     }
 
     void checkDefaultArgument(DefaultArgumentExpr *expr) {
@@ -2248,11 +2277,12 @@ namespace {
     ActorIsolation computeRequiredIsolation(Expr *expr) {
       auto &ctx = getDeclContext()->getASTContext();
 
-      shouldRefineIsolation =
-          ctx.LangOpts.hasFeature(Feature::IsolatedDefaultArguments);
+      if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultArguments))
+        requiredIsolationLoc = expr->getLoc();
+
       expr->walk(*this);
-      shouldRefineIsolation = false;
-      return requiredIsolation;
+      requiredIsolationLoc = SourceLoc();
+      return requiredIsolation[getDeclContext()];
     }
 
     /// Searches the applyStack from back to front for the inner-most CallExpr

test/Concurrency/isolated_default_arguments.swift

@@ -13,10 +13,10 @@ actor SomeGlobalActor {
   static let shared = SomeGlobalActor()
 }
 
+// expected-note@+2 2 {{calls to global function 'requiresMainActor()' from outside of its actor context are implicitly asynchronous}}
 @MainActor
 func requiresMainActor() -> Int { 0 }
 
-// expected-note@+2 2 {{calls to global function 'requiresSomeGlobalActor()' from outside of its actor context are implicitly asynchronous}}
 @SomeGlobalActor
 func requiresSomeGlobalActor() -> Int { 0 }
 
@@ -77,7 +77,7 @@ func nonisolatedAsyncCaller() async {
 }
 
 func conflictingIsolationDefaultArg(
-  // expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated global function 'requiresSomeGlobalActor()' in a synchronous nonisolated context}}
+  // expected-error@+1 {{default argument cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}}
   value: (Int, Int) = (requiresMainActor(), requiresSomeGlobalActor())
 ) {}
 
@@ -100,9 +100,27 @@ func closureIsolationMismatch(
 ) {}
 
 func conflictingClosureIsolation(
+  // expected-error@+1 {{default argument cannot be both main actor-isolated and global actor 'SomeGlobalActor'-isolated}}
   closure: () -> Void = {
     _ = requiresMainActor()
-    // expected-error@+1 {{call to global actor 'SomeGlobalActor'-isolated global function 'requiresSomeGlobalActor()' in a synchronous nonisolated context}}
     _ = requiresSomeGlobalActor()
   }
 ) {}
+
+func isolationInLocalFunction(
+  closure: () -> Void = {
+    nonisolated func local() -> Int {
+      // expected-error@+1 {{call to main actor-isolated global function 'requiresMainActor()' in a synchronous nonisolated context}}
+      requiresMainActor()
+    }
+  }
+) {}
+
+actor A {}
+
+func closureWithIsolatedParam(
+  closure: (isolated A) -> Void = { _ in
+    // expected-error@+1 {{call to main actor-isolated global function 'requiresMainActor()' in a synchronous actor-isolated context}}
+    _ = requiresMainActor()
+  }
+) {}