restrict uses of 'self' in certain actor initializers

A uniqueness rule for 'self' is enforced throughout
the following types of actor initializers:

1. synchronous
2. global-actor isolated

This means that 'self' cannot be used for anything,
even after 'self' is fully initialized, other than
for access to stored properties. Method calls are
considered escaping uses of 'self', since it is passed
implicitly to the method of computed property when invoked.

Also, an actor's convenience init now treats self as
nonisolated.

This provides a way to perform follow-up work after
self is fully initialized, when creating the
actor from a synchronous context.

Resolves rdar://78790369

Author: kavon

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
     }