[Concurrency] Allow default initializer expressions for stored instance

hborla · Holly Borla · commit 7f119474acb1 · 2023-10-04T13:12:11.000-07:00
properties to require actor isolation.

Member initializer expressions are only used in a constructor with
matching actor isolation. If the isolation prohibits the member
initializer from being evaluated synchronously (or propagating required
isolation through closure bodies), then the default value cannot be used
and the member must be explicitly initialized in the constructor.

Member initializer expressions are also used as default arguments for the
memberwise initializer, and the same rules for default argument isolation
apply.
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -1973,6 +1973,15 @@ class PatternBindingEntry {
   SourceRange getOriginalInitRange() const;
   void setInit(Expr *E);
 
+  /// Get the required actor isolation for evaluating the initializer
+  /// expression synchronously (if there is one). 
+  ///
+  /// If this pattern binding entry is for a stored instance property, the
+  /// initializer can only be used in an `init` that meets the required
+  /// isolation; otherwise, the property must be explicitly initialized in
+  /// the `init`.
+  ActorIsolation getInitializerIsolation() const;
+
   /// Gets the text of the initializer expression, stripping out inactive
   /// branches of any #ifs inside the expression.
   StringRef getInitStringRepresentation(SmallVectorImpl<char> &scratch) const;
@@ -2226,6 +2235,10 @@ class PatternBindingDecl final : public Decl,
     getMutablePatternList()[i].setOriginalInit(E);
   }
 
+  ActorIsolation getInitializerIsolation(unsigned i) const {
+    return getPatternList()[i].getInitializerIsolation();
+  }
+
   Pattern *getPattern(unsigned i) const {
     return getPatternList()[i].getPattern();
   }
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -2935,23 +2935,27 @@ class DefaultArgumentTypeRequest
 };
 
 /// Compute the actor isolation needed to synchronously evaluate the
-/// default argument for the given parameter.
-class DefaultArgumentIsolation
-    : public SimpleRequest<DefaultArgumentIsolation,
-                           ActorIsolation(ParamDecl *),
+/// default initializer expression.
+class DefaultInitializerIsolation
+    : public SimpleRequest<DefaultInitializerIsolation,
+                           ActorIsolation(Initializer *, Expr *),
                            RequestFlags::Cached> {
 public:
   using SimpleRequest::SimpleRequest;
 
 private:
   friend SimpleRequest;
 
-  ActorIsolation evaluate(Evaluator &evaluator, ParamDecl *param) const;
+  ActorIsolation evaluate(Evaluator &evaluator, 
+                          Initializer *init,
+                          Expr *initExpr) const;
 
 public:
   bool isCached() const { return true; }
 };
 
+void simple_display(llvm::raw_ostream &out, Initializer *init);
+
 /// Computes the fully type-checked caller-side default argument within the
 /// context of the call site that it will be inserted into.
 class CallerSideDefaultArgExprRequest
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -62,8 +62,9 @@ SWIFT_REQUEST(TypeChecker, DefaultArgumentExprRequest,
               Expr *(ParamDecl *), SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, DefaultArgumentTypeRequest,
               Type(ParamDecl *), SeparatelyCached, NoLocationInfo)
-SWIFT_REQUEST(TypeChecker, DefaultArgumentIsolation,
-              ActorIsolation(ParamDecl *), Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, DefaultInitializerIsolation,
+              ActorIsolation(Initializer *, Expr *),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, DefaultArgumentInitContextRequest,
               Initializer *(ParamDecl *), SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, DefaultDefinitionTypeRequest,
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -1976,6 +1976,20 @@ void PatternBindingEntry::setInit(Expr *E) {
                              PatternFlags::IsText);
 }
 
+ActorIsolation PatternBindingEntry::getInitializerIsolation() const {
+  if (!isInitialized())
+    return ActorIsolation::forUnspecified();
+
+  auto *dc = cast<Initializer>(getInitContext());
+  auto &ctx = dc->getASTContext();
+  auto *initExpr = getExecutableInit();
+
+  return evaluateOrDefault(
+      ctx.evaluator,
+      DefaultInitializerIsolation{dc, initExpr},
+      ActorIsolation::forNonisolated());
+}
+
 VarDecl *PatternBindingEntry::getAnchoringVarDecl() const {
   SmallVector<VarDecl *, 8> variables;
   getPattern()->collectVariables(variables);
@@ -8264,10 +8278,30 @@ Type ParamDecl::getTypeOfDefaultExpr() const {
 }
 
 ActorIsolation ParamDecl::getDefaultArgumentIsolation() const {
+  // If this parameter corresponds to a stored property for a
+  // memberwise initializer, the default argument is the default
+  // initializer expression.
+  auto *var = getStoredProperty();
+  if (var && !var->isInvalid()) {
+    // FIXME: Force computation of property wrapper initializers.
+    if (auto *wrapped = var->getOriginalWrappedProperty())
+      (void)wrapped->getPropertyWrapperInitializerInfo();
+
+    auto *pbd = var->getParentPatternBinding();
+    auto i = pbd->getPatternEntryIndexForVarDecl(var);
+    return var->getParentPatternBinding()->getInitializerIsolation(i);
+  }
+
+  if (!hasDefaultExpr())
+    return ActorIsolation::forUnspecified();
+
   auto &ctx = getASTContext();
+  auto *dc = getDefaultArgumentInitContext();
+  auto *initExpr = getTypeCheckedDefaultExpr();
+
   return evaluateOrDefault(
       ctx.evaluator,
-      DefaultArgumentIsolation{const_cast<ParamDecl *>(this)},
+      DefaultInitializerIsolation{dc, initExpr},
       ActorIsolation::forNonisolated());
 }
 
@@ -10453,8 +10487,25 @@ ActorIsolation swift::getActorIsolationOfContext(
   if (auto *vd = dyn_cast_or_null<ValueDecl>(dcToUse->getAsDecl()))
     return getActorIsolation(vd);
 
-  if (auto *var = dcToUse->getNonLocalVarDecl())
+  // In the context of the initializing or default-value expression of a
+  // stored property, the isolation varies between instance and type members:
+  //   - 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.
+  if (auto *var = dcToUse->getNonLocalVarDecl()) {
+    auto &ctx = dc->getASTContext();
+    if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultArguments) &&
+        var->isInstanceMember() &&
+        !var->getAttrs().hasAttribute<LazyAttr>()) {
+      return ActorIsolation::forNonisolated();
+    }
+
     return getActorIsolation(var);
+  }
 
   if (auto *closure = dyn_cast<AbstractClosureExpr>(dcToUse)) {
     return getClosureActorIsolation(closure);
diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp
@@ -1302,6 +1302,24 @@ void DefaultArgumentTypeRequest::cacheResult(Type type) const {
   param->setDefaultExprType(type);
 }
 
+//----------------------------------------------------------------------------//
+// DefaultInitializerIsolation computation.
+//----------------------------------------------------------------------------//
+
+void swift::simple_display(llvm::raw_ostream &out, Initializer *init) {
+  switch (init->getInitializerKind()) {
+  case InitializerKind::PatternBinding:
+    out << "pattern binding initializer";
+    break;
+  case InitializerKind::DefaultArgument:
+    out << "default argument initializer";
+    break;
+  case InitializerKind::PropertyWrapper:
+    out << "property wrapper initializer";
+    break;
+  }
+}
+
 //----------------------------------------------------------------------------//
 // CallerSideDefaultArgExprRequest computation.
 //----------------------------------------------------------------------------//
diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp
@@ -1542,6 +1542,26 @@ void SILGenFunction::emitMemberInitializer(DeclContext *dc, VarDecl *selfDecl,
     if (!init)
       continue;
 
+    // Member initializer expressions are only used in a constructor with
+    // matching actor isolation. If the isolation prohibits the member
+    // initializer from being evaluated synchronously (or propagating required
+    // isolation through closure bodies), then the default value cannot be used
+    // and the member must be explicitly initialized in the constructor.
+    auto requiredIsolation = field->getInitializerIsolation(i);
+    auto contextIsolation = getActorIsolationOfContext(dc);
+    switch (requiredIsolation) {
+    // 'nonisolated' expressions can be evaluated from anywhere
+    case ActorIsolation::Unspecified:
+    case ActorIsolation::Nonisolated:
+      break;
+
+    case ActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActorUnsafe:
+    case ActorIsolation::ActorInstance:
+      if (requiredIsolation != contextIsolation)
+        continue;
+    }
+
     auto *varPattern = field->getPattern(i);
 
     // Cleanup after this initialization.
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -3582,11 +3582,6 @@ void swift::checkFunctionActorIsolation(AbstractFunctionDecl *decl) {
   }
 }
 
-void swift::checkInitializerActorIsolation(Initializer *init, Expr *expr) {
-  ActorIsolationChecker checker(init);
-  expr->walk(checker);
-}
-
 ActorIsolation 
 swift::computeRequiredIsolation(Initializer *init, Expr *expr) {
   ActorIsolationChecker checker(init);
diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h
@@ -56,7 +56,6 @@ void addAsyncNotes(AbstractFunctionDecl const* func);
 /// Check actor isolation rules.
 void checkTopLevelActorIsolation(TopLevelCodeDecl *decl);
 void checkFunctionActorIsolation(AbstractFunctionDecl *decl);
-void checkInitializerActorIsolation(Initializer *init, Expr *expr);
 void checkEnumElementActorIsolation(EnumElementDecl *element, Expr *expr);
 void checkPropertyWrapperActorIsolation(VarDecl *wrappedVar, Expr *expr);
 
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -1157,14 +1157,13 @@ Expr *DefaultArgumentExprRequest::evaluate(Evaluator &evaluator,
 }
 
 ActorIsolation 
-DefaultArgumentIsolation::evaluate(Evaluator &evaluator,
-                                   ParamDecl *param) const {
-  if (!param->hasDefaultExpr())
+DefaultInitializerIsolation::evaluate(Evaluator &evaluator,
+                                      Initializer *init,
+                                      Expr *initExpr) const {
+  if (!init || !initExpr)
     return ActorIsolation::forUnspecified();
 
-  auto *dc = param->getDefaultArgumentInitContext();
-  auto *initExpr = param->getTypeCheckedDefaultExpr();
-  return computeRequiredIsolation(dc, initExpr);
+  return computeRequiredIsolation(init, initExpr);
 }
 
 Type DefaultArgumentTypeRequest::evaluate(Evaluator &evaluator,
@@ -2477,7 +2476,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
               PBD->getInitContext(i));
           if (initContext) {
             TypeChecker::contextualizeInitializer(initContext, init);
-            checkInitializerActorIsolation(initContext, init);
+            (void)PBD->getInitializerIsolation(i);
             TypeChecker::checkInitializerEffects(initContext, init);
           }
         }
diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp
@@ -3226,7 +3226,7 @@ PropertyWrapperInitializerInfoRequest::evaluate(Evaluator &evaluator,
   // synthesize a computed property for '$foo'.
   Expr *projectedValueInit = nullptr;
   if (auto *projection = var->getPropertyWrapperProjectionVar()) {
-    createPBD(projection);
+    auto *pbd = createPBD(projection);
 
     if (var->hasExternalPropertyWrapper()) {
       // Projected-value initialization is currently only supported for parameters.
@@ -3240,7 +3240,7 @@ PropertyWrapperInitializerInfoRequest::evaluate(Evaluator &evaluator,
       // Check initializer effects.
       auto *initContext = new (ctx) PropertyWrapperInitializer(
           dc, param, PropertyWrapperInitializer::Kind::ProjectedValue);
-      checkInitializerActorIsolation(initContext, projectedValueInit);
+      (void)pbd->getInitializerIsolation(0);
       TypeChecker::checkInitializerEffects(initContext, projectedValueInit);
     }
   }
diff --git a/test/Concurrency/isolated_default_arguments.swift b/test/Concurrency/isolated_default_arguments.swift
@@ -124,3 +124,59 @@ func closureWithIsolatedParam(
     _ = requiresMainActor()
   }
 ) {}
+
+@MainActor
+struct S1 {
+  var required: Int
+
+  var x: Int = requiresMainActor()
+
+  lazy var y: Int = requiresMainActor()
+
+  static var z: Int = requiresMainActor()
+}
+
+@SomeGlobalActor
+struct S2 {
+  var required: Int
+
+  var x: Int = requiresSomeGlobalActor()
+
+  lazy var y: Int = requiresSomeGlobalActor()
+
+  static var z: Int = requiresSomeGlobalActor()
+}
+
+@MainActor
+func initializeFromMainActor() {
+  _ = S1(required: 10)
+
+  // expected-error@+1 {{global actor 'SomeGlobalActor'-isolated default argument cannot be synchronously evaluated from a main actor-isolated context}}
+  _ = S2(required: 10)
+}
+
+@SomeGlobalActor
+func initializeFromSomeGlobalActor() {
+  // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a global actor 'SomeGlobalActor'-isolated context}}
+  _ = S1(required: 10)
+
+  _ = S2(required: 10)
+}
+
+func initializeFromNonisolated() {
+  // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a nonisolated context}}
+  _ = S1(required: 10)
+
+  // expected-error@+1 {{global actor 'SomeGlobalActor'-isolated default argument cannot be synchronously evaluated from a nonisolated context}}
+  _ = S2(required: 10)
+}
+
+extension A {
+  func initializeFromActorInstance() {
+    // expected-error@+1 {{main actor-isolated default argument cannot be synchronously evaluated from a actor-isolated context}}
+    _ = S1(required: 10)
+
+    // expected-error@+1 {{global actor 'SomeGlobalActor'-isolated default argument cannot be synchronously evaluated from a actor-isolated context}}
+    _ = S2(required: 10)
+  }
+}
diff --git a/test/Concurrency/isolated_default_property_inits.swift b/test/Concurrency/isolated_default_property_inits.swift
@@ -0,0 +1,57 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
+
+// RUN: %target-swift-frontend -I %t  -disable-availability-checking -strict-concurrency=complete -enable-experimental-feature IsolatedDefaultArguments -parse-as-library -emit-sil -o /dev/null -verify %s
+// RUN: %target-swift-frontend -I %t  -disable-availability-checking -strict-concurrency=complete -parse-as-library -emit-sil -o /dev/null -verify -enable-experimental-feature IsolatedDefaultArguments -enable-experimental-feature SendNonSendable %s
+
+// REQUIRES: concurrency
+// REQUIRES: asserts
+
+@globalActor
+actor SomeGlobalActor {
+  static let shared = SomeGlobalActor()
+}
+
+@MainActor
+func requiresMainActor() -> Int { 0 }
+
+@SomeGlobalActor
+func requiresSomeGlobalActor() -> Int { 0 }
+
+struct S1 {
+  // expected-note@+1 2 {{'self.x' not initialized}}
+  var x = requiresMainActor()
+  // expected-note@+1 2 {{'self.y' not initialized}}
+  var y = requiresSomeGlobalActor()
+  var z = 10
+
+  // expected-error@+1 {{return from initializer without initializing all stored properties}}
+  nonisolated init(a: Int) {}
+
+  // expected-error@+1 {{return from initializer without initializing all stored properties}}
+  @MainActor init(b: Int) {}
+
+  // expected-error@+1 {{return from initializer without initializing all stored properties}}
+  @SomeGlobalActor init(c: Int) {}
+}
+
+struct S2 {
+  var x = requiresMainActor()
+  var y = requiresSomeGlobalActor()
+  var z = 10
+
+  nonisolated init(x: Int, y: Int) {
+    self.x = x
+    self.y = y
+  }
+
+  @MainActor init(y: Int) {
+    self.y = y
+  }
+
+  @SomeGlobalActor init(x: Int) {
+    self.x = x
+  }
+}
+

Original file line number	Diff line number	Diff line change
`@@ -3582,11 +3582,6 @@ void swift::checkFunctionActorIsolation(AbstractFunctionDecl *decl) {`
`3582`	`3582`	`}`
`3583`	`3583`	`}`
`3584`	`3584`
`3585`		`-void swift::checkInitializerActorIsolation(Initializer init, Expr expr) {`
`3586`		`- ActorIsolationChecker checker(init);`
`3587`		`- expr->walk(checker);`
`3588`		`-}`
`3589`		`-`
`3590`	`3585`	`ActorIsolation`
`3591`	`3586`	`swift::computeRequiredIsolation(Initializer init, Expr expr) {`
`3592`	`3587`	`ActorIsolationChecker checker(init);`