[Concurrency] Allow isolated default arguments to be used from across isolation

hborla · hborla · commit 49d6399a88d4 · 2023-10-24T22:54:22.000-07:00
boundaries.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -5263,9 +5263,8 @@ ERROR(distributed_actor_isolated_non_self_reference,none,
       "distributed actor-isolated %kind0 can not be accessed from a "
       "non-isolated context",
       (const ValueDecl *))
-ERROR(isolated_default_argument,none,
-      "%0 default argument cannot be synchronously evaluated from a "
-      "%1 context",
+ERROR(isolated_default_argument_context,none,
+      "%0 default argument in a %1 context", 
       (ActorIsolation, ActorIsolation))
 ERROR(conflicting_default_argument_isolation,none,
       "default argument cannot be both %0 and %1",
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -4540,6 +4540,10 @@ class DefaultArgumentExpr final : public Expr {
   /// default expression.
   PointerUnion<DeclContext *, Expr *> ContextOrCallerSideExpr;
 
+  /// Whether this default argument is evaluated asynchronously because
+  /// it's isolated to the callee's isolation domain.
+  bool implicitlyAsync = false;
+
 public:
   explicit DefaultArgumentExpr(ConcreteDeclRef defaultArgsOwner,
                                unsigned paramIndex, SourceLoc loc, Type Ty,
@@ -4574,6 +4578,16 @@ class DefaultArgumentExpr final : public Expr {
   /// argument must be written explicitly at the call-site.
   ActorIsolation getRequiredIsolation() const;
 
+  /// Whether this default argument is evaluated asynchronously because
+  /// it's isolated to the callee's isolation domain.
+  bool isImplicitlyAsync() const {
+    return implicitlyAsync;
+  }
+
+  void setImplicitlyAsync() {
+    implicitlyAsync = true;
+  }
+
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::DefaultArgument;
   }
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -10536,22 +10536,15 @@ ActorIsolation swift::getActorIsolationOfContext(
     return getActorIsolation(vd);
 
   // In the context of the initializing or default-value expression of a
-  // stored property, the isolation varies between instance and type members:
+  // stored property:
   //   - For a static stored property, the isolation matches the VarDecl.
   //     Static properties are initialized upon first use, so the isolation
   //     of the initializer must match the isolation required to access the
   //     property.
-  //   - For a field of a nominal type, the expression can require a specific
-  //     actor isolation. That default expression may only be used from inits
-  //     that meet the required isolation.
+  //   - For a field of a nominal type, the expression can require the same
+  //     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()) {
-    auto &ctx = dc->getASTContext();
-    if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues) &&
-        var->isInstanceMember() &&
-        !var->getAttrs().hasAttribute<LazyAttr>()) {
-      return ActorIsolation::forNonisolated();
-    }
-
     return getActorIsolation(var);
   }
 
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -2518,18 +2518,21 @@ class DelayedArgument {
     AbstractionPattern origResultType;
     ClaimedParamsRef paramsToEmit;
     SILFunctionTypeRepresentation functionRepresentation;
-    
+    bool implicitlyAsync;
+
     DefaultArgumentStorage(SILLocation loc,
                            ConcreteDeclRef defaultArgsOwner,
                            unsigned destIndex,
                            CanType resultType,
                            AbstractionPattern origResultType,
                            ClaimedParamsRef paramsToEmit,
-                           SILFunctionTypeRepresentation functionRepresentation)
+                           SILFunctionTypeRepresentation functionRepresentation,
+                           bool implicitlyAsync)
       : loc(loc), defaultArgsOwner(defaultArgsOwner), destIndex(destIndex),
         resultType(resultType), origResultType(origResultType),
         paramsToEmit(paramsToEmit),
-        functionRepresentation(functionRepresentation)
+        functionRepresentation(functionRepresentation),
+        implicitlyAsync(implicitlyAsync)
     {}
   };
   struct BorrowedLValueStorage {
@@ -2656,13 +2659,15 @@ class DelayedArgument {
                   CanType resultType,
                   AbstractionPattern origResultType,
                   ClaimedParamsRef params,
-                  SILFunctionTypeRepresentation functionTypeRepresentation)
+                  SILFunctionTypeRepresentation functionTypeRepresentation,
+                  bool implicitlyAsync)
     : Kind(DefaultArgument) {
     Value.emplace<DefaultArgumentStorage>(Kind, loc, defaultArgsOwner,
                                           destIndex,
                                           resultType,
                                           origResultType, params,
-                                          functionTypeRepresentation);
+                                          functionTypeRepresentation,
+                                          implicitlyAsync);
   }
 
   DelayedArgument(DelayedArgument &&other)
@@ -3261,7 +3266,7 @@ class ArgEmitter {
                                     defArg->getParamIndex(),
                                     substParamType, origParamType,
                                     claimNextParameters(numParams),
-                                    Rep);
+                                    Rep, defArg->isImplicitlyAsync());
       Args.push_back(ManagedValue());
 
       maybeEmitForeignArgument();
@@ -4255,7 +4260,8 @@ void DelayedArgument::emitDefaultArgument(SILGenFunction &SGF,
   auto value = SGF.emitApplyOfDefaultArgGenerator(info.loc,
                                                   info.defaultArgsOwner,
                                                   info.destIndex,
-                                                  info.resultType);
+                                                  info.resultType,
+                                                  info.implicitlyAsync);
 
   SmallVector<ManagedValue, 4> loweredArgs;
   SmallVector<DelayedArgument, 4> delayedArgs;
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -2548,6 +2548,7 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
                                                ConcreteDeclRef defaultArgsOwner,
                                                unsigned destIndex,
                                                CanType resultType,
+                                               bool implicitlyAsync,
                                                SGFContext C) {
   SILDeclRef generator 
     = SILDeclRef::getDefaultArgGenerator(defaultArgsOwner.getDecl(),
@@ -2578,8 +2579,24 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
   emitCaptures(loc, generator, CaptureEmission::ImmediateApplication,
                captures);
 
+  // The default argument might require the callee's isolation. If so,
+  // make sure to emit an actor hop.
+  //
+  // FIXME: Instead of hopping back and forth for each individual isolated
+  // default argument, we should emit one hop for all default arguments if
+  // any of them are isolated, and immediately enter the function after.
+  llvm::Optional<ActorIsolation> implicitActorHopTarget = llvm::None;
+  if (implicitlyAsync) {
+    auto *param = getParameterAt(defaultArgsOwner.getDecl(), destIndex);
+    auto isolation = param->getInitializerIsolation();
+    if (isolation.isActorIsolated()) {
+      implicitActorHopTarget = isolation;
+    }
+  }
+
   return emitApply(std::move(resultPtr), std::move(argScope), loc, fnRef, subs,
-                   captures, calleeTypeInfo, ApplyOptions(), C, llvm::None);
+                   captures, calleeTypeInfo, ApplyOptions(), C,
+                   implicitActorHopTarget);
 }
 
 RValue SILGenFunction::emitApplyOfStoredPropertyInitializer(
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
@@ -1907,6 +1907,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                                         ConcreteDeclRef defaultArgsOwner,
                                         unsigned destIndex,
                                         CanType resultType,
+                                        bool implicitlyAsync,
                                         SGFContext C = SGFContext());
 
   RValue emitApplyOfStoredPropertyInitializer(
diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp
@@ -339,7 +339,14 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl,
 
   if (ICK == ImplicitConstructorKind::Memberwise) {
     ctor->setIsMemberwiseInitializer();
-    addNonIsolatedToSynthesized(decl, ctor);
+
+    // FIXME: If 'IsolatedDefaultValues' is enabled, the memberwise init
+    // should be 'nonisolated' if none of the memberwise-initialized properties
+    // are global actor isolated and have non-Sendable type, and none of the
+    // initial values require global actor isolation.
+    if (!ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues)) {
+      addNonIsolatedToSynthesized(decl, ctor);
+    }
   }
 
   // If we are defining a default initializer for a class that has a superclass,
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -1921,7 +1921,17 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
       // Determine the type of the argument, ignoring any implicit
       // conversions that could have stripped sendability.
       if (Expr *argExpr = arg.getExpr()) {
-          argType = argExpr->findOriginalType();
+        argType = argExpr->findOriginalType();
+
+        // If this is a default argument expression, don't check Sendability
+        // if the argument is evaluated in the callee's isolation domain.
+        if (auto *defaultExpr = dyn_cast<DefaultArgumentExpr>(argExpr)) {
+          auto argIsolation = defaultExpr->getRequiredIsolation();
+          auto calleeIsolation = isolationCrossing->getCalleeIsolation();
+          if (argIsolation == calleeIsolation) {
+            continue;
+          }
+        }
       }
     }
 
@@ -2180,8 +2190,10 @@ namespace {
       // recieve isolation from its decl context), then the expression cannot
       // require a different isolation.
       for (auto *dc : contextStack) {
-        if (!infersIsolationFromContext(dc))
+        if (!infersIsolationFromContext(dc)) {
+          requiredIsolation.clear();
           return false;
+        }
 
         // To refine the required isolation, the existing requirement
         // must either be 'nonisolated' or exactly the same as the
@@ -2195,6 +2207,7 @@ namespace {
               requiredIsolationLoc,
               diag::conflicting_default_argument_isolation,
               isolation->second, refinedIsolation);
+          requiredIsolation.clear();
           return true;
         }
       }
@@ -2205,8 +2218,8 @@ namespace {
     void checkDefaultArgument(DefaultArgumentExpr *expr) {
       // Check the context isolation against the required isolation for
       // evaluating the default argument synchronously. If the default
-      // argument must be evaluated asynchronously, it must be written
-      // explicitly in the argument list with 'await'.
+      // argument must be evaluated asynchronously, record that in the
+      // expression node.
       auto requiredIsolation = expr->getRequiredIsolation();
       auto contextIsolation = getInnermostIsolatedContext(
           getDeclContext(), getClosureActorIsolation);
@@ -2227,10 +2240,7 @@ namespace {
         break;
       }
 
-      auto &ctx = getDeclContext()->getASTContext();
-      ctx.Diags.diagnose(
-          expr->getLoc(), diag::isolated_default_argument,
-          requiredIsolation, contextIsolation);
+      expr->setImplicitlyAsync();
     }
 
     /// Check closure captures for Sendable violations.
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -1031,7 +1031,18 @@ static void checkDefaultArguments(ParameterList *params) {
   for (auto *param : *params) {
     auto ifacety = param->getInterfaceType();
     auto *expr = param->getTypeCheckedDefaultExpr();
-    (void)param->getInitializerIsolation();
+
+    // 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);
+      }
+    }
 
     if (!ifacety->hasPlaceholder()) {
       continue;
diff --git a/test/Concurrency/isolated_default_arguments.swift b/test/Concurrency/isolated_default_arguments.swift
diff --git a/test/Concurrency/isolated_default_arguments_serialized.swift b/test/Concurrency/isolated_default_arguments_serialized.swift
