Merge pull request #38043 from kavon/5.5-restricted-self-actorinit

[5.5] Restrict uses of 'self' in actor initializers

Author: DougGregor

include/swift/AST/DiagnosticsSIL.def

@@ -168,6 +168,22 @@ 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))
+NOTE(actor_convenience_init,none,
+     "convenience initializers allow non-isolated use of 'self' once "
+     "initialized",
+     ())
+
+
+
 
 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())
+        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,21 @@ 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,
+                                      bool suggestConvenienceInit) const;
+
+    void handleLoadUseFailureForActorInit(const DIMemoryUse &Use,
+                                          ActorInitKind ActorKind) const;
+
     void handleLoadUseFailure(const DIMemoryUse &Use,
                               bool SuperInitDone,
                               bool FailedSelfUse);
@@ -866,9 +888,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 +1203,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 +1222,11 @@ 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'",
+                                 /*suggestConvenienceInit=*/false);
+
   // 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 +1389,20 @@ 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",
+                                   /*suggestConvenienceInit=*/true);
+
     return;
   }
 
@@ -1746,6 +1786,100 @@ 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,
+                                           bool suggestConvenienceInit) 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;
+  }
+
+  if (suggestConvenienceInit)
+    diagnose(Module, Use.Inst->getLoc(), diag::actor_convenience_init);
+}
+
+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;
+  }
+
+  // We cannot easily determine which argument in the call the use of 'self'
+  // appears in. If we could, then we could determine whether the callee
+  // is 'isolated' to that parameter, in order to avoid suggesting a convenience
+  // init in those cases. Thus, the phrasing of the note should be informative.
+  if (isa<ApplyInst>(Inst))
+    diagnose(Module, Use.Inst->getLoc(), diag::actor_convenience_init);
+}
+
 /// Check and diagnose various failures when a load use is not fully
 /// initialized.
 ///