Restrict uses of 'self' in actor initializers

include/swift/AST/DiagnosticsSIL.def

@@ -168,6 +168,18 @@ ERROR(variable_defer_use_uninit,none,
 ERROR(self_closure_use_uninit,none,
       "'self' captured by a closure before all members were initialized", ())
 
+/// false == sync; true == global-actor isolated
+ERROR(self_use_actor_init,none,
+      "this use of actor 'self' %select{can only|cannot}0 appear in "
+      "%select{an async|a global-actor isolated}0 initializer",
+      (bool))
+ERROR(self_disallowed_actor_init,none,
+      "actor 'self' %select{can only|cannot}0 %1 from "
+      "%select{an async|a global-actor isolated}0 initializer",
+      (bool, StringRef))
+
+
+
 
 ERROR(variable_addrtaken_before_initialized,none,
       "address of %select{variable|constant}1 '%0' taken before it is"

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

@@ -432,6 +432,23 @@ bool DIMemoryObjectInfo::isElementLetProperty(unsigned Element) const {
   return false;
 }
 
+ConstructorDecl *DIMemoryObjectInfo::isActorInitSelf() const {
+  // is it 'self'?
+  if (!MemoryInst->isVar())
+    if (auto decl =
+        dyn_cast_or_null<ClassDecl>(getASTType()->getAnyNominal()))
+      // is it for an actor?
+      if (decl->isActor() && !decl->isDistributedActor()) // FIXME(78484431) skip distributed actors for now, until their initializers are fixed!
+        if (auto *silFn = MemoryInst->getFunction())
+          // is it a designated initializer?
+          if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(
+                            silFn->getDeclContext()->getAsDecl()))
+            if (ctor->isDesignatedInit())
+              return ctor;
+
+  return nullptr;
+}
+
 //===----------------------------------------------------------------------===//
 //                        DIMemoryUse Implementation
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h

@@ -164,6 +164,10 @@ class DIMemoryObjectInfo {
     return false;
   }
 
+  /// Returns the initializer if the memory use is 'self' and appears in an
+  /// actor's designated initializer. Otherwise, returns nullptr.
+  ConstructorDecl *isActorInitSelf() const;
+
   /// True if this memory object is the 'self' of a derived class initializer.
   bool isDerivedClassSelf() const { return MemoryInst->isDerivedClassSelf(); }
 

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -384,6 +384,13 @@ namespace {
 
   using BlockStates = BasicBlockData<LiveOutBlockState>;
 
+  enum class ActorInitKind {
+    None,           // not an actor init
+    Plain,          // synchronous, not isolated to global-actor
+    PlainAsync,     // asynchronous, not isolated to global-actor
+    GlobalActorIsolated  // isolated to global-actor (sync or async).
+  };
+
   /// LifetimeChecker - This is the main heavy lifting for definite
   /// initialization checking of a memory object.
   class LifetimeChecker {
@@ -480,6 +487,20 @@ namespace {
     bool diagnoseReturnWithoutInitializingStoredProperties(
         const SILInstruction *Inst, SILLocation loc, const DIMemoryUse &Use);
 
+    /// Returns true iff the use involves 'self' in a restricted kind of
+    /// actor initializer. If a non-null Kind pointer was passed in,
+    /// then the specific kind of restricted actor initializer will be
+    /// written out. Otherwise, the None initializer kind will be written out.
+    bool isRestrictedActorInitSelf(const DIMemoryUse& Use,
+                               ActorInitKind *Kind = nullptr) const;
+
+    void reportIllegalUseForActorInit(const DIMemoryUse &Use,
+                                      ActorInitKind ActorKind,
+                                      StringRef ProblemDesc) const;
+
+    void handleLoadUseFailureForActorInit(const DIMemoryUse &Use,
+                                          ActorInitKind ActorKind) const;
+
     void handleLoadUseFailure(const DIMemoryUse &Use,
                               bool SuperInitDone,
                               bool FailedSelfUse);
@@ -866,9 +887,13 @@ void LifetimeChecker::doIt() {
 
 void LifetimeChecker::handleLoadUse(const DIMemoryUse &Use) {
   bool IsSuperInitComplete, FailedSelfUse;
+  ActorInitKind ActorKind = ActorInitKind::None;
   // If the value is not definitively initialized, emit an error.
   if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse))
     return handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse);
+  // Check if it involves 'self' in a restricted actor init.
+  if (isRestrictedActorInitSelf(Use, &ActorKind))
+    return handleLoadUseFailureForActorInit(Use, ActorKind);
 }
 
 static void replaceValueMetatypeInstWithMetatypeArgument(
@@ -1177,6 +1202,7 @@ static FullApplySite findApply(SILInstruction *I, bool &isSelfParameter) {
 
 void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) {
   bool IsSuperInitDone, FailedSelfUse;
+  ActorInitKind ActorKind = ActorInitKind::None;
 
   // inout uses are generally straight-forward: the memory must be initialized
   // before the "address" is passed as an l-value.
@@ -1195,6 +1221,10 @@ void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) {
     return;
   }
 
+  // 'self' cannot be passed 'inout' from some kinds of actor initializers.
+  if (isRestrictedActorInitSelf(Use, &ActorKind))
+    reportIllegalUseForActorInit(Use, ActorKind, "be passed 'inout'");
+
   // One additional check: 'let' properties may never be passed inout, because
   // they are only allowed to have their initial value set, not a subsequent
   // overwrite.
@@ -1357,12 +1387,19 @@ static bool isLoadForReturn(SingleValueInstruction *loadInst) {
 }
 
 void LifetimeChecker::handleEscapeUse(const DIMemoryUse &Use) {
+
   // The value must be fully initialized at all escape points.  If not, diagnose
   // the error.
   bool SuperInitDone, FailedSelfUse, FullyUninitialized;
+  ActorInitKind ActorKind = ActorInitKind::None;
 
   if (isInitializedAtUse(Use, &SuperInitDone, &FailedSelfUse,
                          &FullyUninitialized)) {
+
+    // no escaping uses of 'self' are allowed in restricted actor inits.
+    if (isRestrictedActorInitSelf(Use, &ActorKind))
+      reportIllegalUseForActorInit(Use, ActorKind, "be captured by a closure");
+
     return;
   }
 
@@ -1746,6 +1783,89 @@ bool LifetimeChecker::diagnoseReturnWithoutInitializingStoredProperties(
   return true;
 }
 
+bool LifetimeChecker::isRestrictedActorInitSelf(const DIMemoryUse& Use,
+                                            ActorInitKind *Kind) const {
+
+  auto result = [&](ActorInitKind k, bool isRestricted) -> bool {
+    if (Kind)
+      *Kind = k;
+    return isRestricted;
+  };
+
+  // Currently: being synchronous, or global-actor isolated, means the actor's
+  // self is restricted within the init.
+  if (auto *ctor = TheMemory.isActorInitSelf()) {
+    if (getActorIsolation(ctor).isGlobalActor())  // global-actor isolated?
+      return result(ActorInitKind::GlobalActorIsolated, true);
+    else if (!ctor->hasAsync()) // synchronous?
+      return result(ActorInitKind::Plain, true);
+    else
+      return result(ActorInitKind::PlainAsync, false);
+  }
+
+  return result(ActorInitKind::None, false);
+}
+
+void LifetimeChecker::reportIllegalUseForActorInit(
+                                           const DIMemoryUse &Use,
+                                           ActorInitKind ActorKind,
+                                           StringRef ProblemDesc) const {
+  switch(ActorKind) {
+  case ActorInitKind::None:
+  case ActorInitKind::PlainAsync:
+    llvm::report_fatal_error("this actor init is never problematic!");
+
+  case ActorInitKind::Plain:
+    diagnose(Module, Use.Inst->getLoc(), diag::self_disallowed_actor_init,
+             false, ProblemDesc);
+    break;
+
+  case ActorInitKind::GlobalActorIsolated:
+    diagnose(Module, Use.Inst->getLoc(), diag::self_disallowed_actor_init,
+             true, ProblemDesc);
+    break;
+  }
+}
+
+void LifetimeChecker::handleLoadUseFailureForActorInit(
+                                           const DIMemoryUse &Use,
+                                           ActorInitKind ActorKind) const {
+  assert(TheMemory.isAnyInitSelf());
+  SILInstruction *Inst = Use.Inst;
+
+  // While enum instructions have no side-effects, they do wrap-up `self`
+  // such that it can escape our analysis. So, enum instruction uses are only
+  // acceptable if the enum is part of a specific pattern of usage that is
+  // generated for a failable initializer.
+  if (auto *enumInst = dyn_cast<EnumInst>(Inst)) {
+    if (isFailableInitReturnUseOfEnum(enumInst))
+      return;
+
+    // Otherwise, if the use has no possibility of side-effects, then its OK.
+  } else if (!Inst->mayHaveSideEffects()) {
+    return;
+
+  // While loads can have side-effects, we are not concerned by them here.
+  } else if (isa<LoadInst>(Inst)) {
+    return;
+  }
+
+  // Everything else is disallowed!
+  switch(ActorKind) {
+  case ActorInitKind::None:
+  case ActorInitKind::PlainAsync:
+    llvm::report_fatal_error("this actor init is never problematic!");
+
+  case ActorInitKind::Plain:
+    diagnose(Module, Use.Inst->getLoc(), diag::self_use_actor_init, false);
+    break;
+
+  case ActorInitKind::GlobalActorIsolated:
+    diagnose(Module, Use.Inst->getLoc(), diag::self_use_actor_init, true);
+    break;
+  }
+}
+
 /// Check and diagnose various failures when a load use is not fully
 /// initialized.
 ///

lib/Sema/TypeCheckConcurrency.cpp

@@ -2143,12 +2143,14 @@ namespace {
     }
 
     /// Check whether we are in an actor's initializer or deinitializer.
-    static bool isActorInitOrDeInitContext(const DeclContext *dc) {
+    /// \returns nullptr iff we are not in such a declaration. Otherwise,
+    ///          returns a pointer to the declaration.
+    static AbstractFunctionDecl const* isActorInitOrDeInitContext(const DeclContext *dc) {
       while (true) {
         // Non-Sendable closures are considered part of the enclosing context.
         if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
           if (isSendableClosure(closure, /*forActorIsolation=*/false))
-            return false;
+            return nullptr;
 
           dc = dc->getParent();
           continue;
@@ -2158,26 +2160,33 @@ namespace {
           // If this is an initializer or deinitializer of an actor, we're done.
           if ((isa<ConstructorDecl>(func) || isa<DestructorDecl>(func)) &&
               getSelfActorDecl(dc->getParent()))
-            return true;
+            return func;
 
           // Non-Sendable local functions are considered part of the enclosing
           // context.
           if (func->getDeclContext()->isLocalContext()) {
             if (auto fnType =
                     func->getInterfaceType()->getAs<AnyFunctionType>()) {
               if (fnType->isSendable())
-                return false;
+                return nullptr;
 
               dc = dc->getParent();
               continue;
             }
           }
         }
 
-        return false;
+        return nullptr;
       }
     }
 
+    static bool isConvenienceInit(AbstractFunctionDecl const* fn) {
+      if (auto ctor = dyn_cast_or_null<ConstructorDecl>(fn))
+        return ctor->isConvenienceInit();
+
+      return false;
+    }
+
     /// Check a reference to a local or global.
     bool checkNonMemberReference(
         ConcreteDeclRef valueRef, SourceLoc loc, DeclRefExpr *declRefExpr) {
@@ -2251,9 +2260,10 @@ namespace {
         bool continueToCheckingLocalIsolation = false;
         // Must reference distributed actor-isolated state on 'self'.
         //
-        // FIXME: For now, be loose about access to "self" in actor
-        // initializers/deinitializers. We'll want to tighten this up once
-        // we decide exactly how the model should go.
+        // FIXME(78484431): For now, be loose about access to "self" in actor
+        // initializers/deinitializers for distributed actors.
+        // We'll want to tighten this up once we decide exactly
+        // how the model should go.
         auto isolatedActor = getIsolatedActor(base);
         if (!isolatedActor &&
             !(isolatedActor.isActorSelf() &&
@@ -2311,14 +2321,12 @@ namespace {
         if (isolatedActor)
           return false;
 
-        // An instance member of an actor can be referenced from an initializer
-        // or deinitializer.
-        // FIXME: We likely want to narrow this down to only stored properties,
-        // but this matches existing behavior.
-        if (isolatedActor.isActorSelf() &&
-            member->isInstanceMember() &&
-            isActorInitOrDeInitContext(getDeclContext()))
-          return false;
+        // An instance member of an actor can be referenced from an actor's
+        // designated initializer or deinitializer.
+        if (isolatedActor.isActorSelf() && member->isInstanceMember())
+          if (auto fn = isActorInitOrDeInitContext(getDeclContext()))
+            if (!isConvenienceInit(fn))
+              return false;
 
         // An escaping partial application of something that is part of
         // the actor's isolated state is never permitted.
@@ -2936,9 +2944,11 @@ static Optional<MemberIsolationPropagation> getMemberIsolationPropagation(
   case DeclKind::PatternBinding:
   case DeclKind::EnumCase:
   case DeclKind::EnumElement:
-  case DeclKind::Constructor:
     return MemberIsolationPropagation::GlobalActor;
 
+  case DeclKind::Constructor:
+    return MemberIsolationPropagation::AnyIsolation;
+
   case DeclKind::Func:
   case DeclKind::Accessor:
   case DeclKind::Subscript:
@@ -3021,6 +3031,13 @@ ActorIsolation ActorIsolationRequest::evaluate(
     }
   }
 
+  // An actor's convenience init is assumed to be actor-independent.
+  if (auto nominal = value->getDeclContext()->getSelfNominalTypeDecl())
+    if (nominal->isActor())
+      if (auto ctor = dyn_cast<ConstructorDecl>(value))
+        if (ctor->isConvenienceInit())
+          defaultIsolation = ActorIsolation::forIndependent();
+
   // Function used when returning an inferred isolation.
   auto inferredIsolation = [&](
       ActorIsolation inferred, bool onlyGlobal = false) {
@@ -3180,10 +3197,6 @@ bool HasIsolatedSelfRequest::evaluate(
 
   switch (*memberIsolation) {
   case MemberIsolationPropagation::GlobalActor:
-    // Treat constructors as being actor-isolated... for now.
-    if (isa<ConstructorDecl>(value))
-      break;
-
     return false;
 
   case MemberIsolationPropagation::AnyIsolation:
@@ -3221,6 +3234,13 @@ bool HasIsolatedSelfRequest::evaluate(
     }
   }
 
+  // In an actor's convenience init, self is not isolated.
+  if (auto ctor = dyn_cast<ConstructorDecl>(value)) {
+    if (ctor->isConvenienceInit()) {
+      return false;
+    }
+  }
+
   return true;
 }
 

test/Concurrency/Runtime/actor_keypaths.swift

@@ -17,10 +17,14 @@ actor Page {
       set { numWordsMem.pointee = newValue }
     }
 
-    init(_ words : Int) {
+    private init(withWords words : Int) {
         initialNumWords = words
         numWordsMem = .allocate(capacity: 1)
         numWordsMem.initialize(to: words)
+    }
+
+    convenience init(_ words: Int) {
+        self.init(withWords: words)
         numWords = words
     }