fix bugs with actor inits and flow-isolation

This is a combination of fixes:

- inject hops in self-isolated delegating actor initializers
  after self becomes initialized.

- fix sendable and isolation for convenience inits

- fix bug in distributed actor inits that I introduced when
  implementing flow-isolation.

- fix / add test coverage.

Author: kavon

lib/SILGen/SILGenConstructor.cpp

@@ -315,6 +315,17 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
   return;
 }
 
+// FIXME: the callers of ctorHopsInjectedByDefiniteInit is not correct (rdar://87485045)
+// we must still set the SGF.ExpectedExecutor field to say that we must
+// hop to the executor after every apply in the constructor. This seems to
+// happen for the main actor isolated async inits, but not for the plain ones,
+// where 'self' is not going to directly be the instance. We have to extend the
+// ExecutorBreadcrumb class to detect whether it needs to do a load or not
+// in it's emit method.
+//
+// So, the big problem right now is that for a delegating async actor init,
+// after calling an async function, no hop-back is being emitted.
+
 /// Returns true if the given async constructor will have its
 /// required actor hops injected later by definite initialization.
 static bool ctorHopsInjectedByDefiniteInit(ConstructorDecl *ctor,

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -790,7 +790,7 @@ static bool isDistributedActorCtor(SILFunction &F) {
 /// Injects a hop_to_executor instruction after the specified insertion point.
 static void injectHopToExecutorAfter(SILLocation loc,
                                      SILBasicBlock::iterator insertPt,
-                                     SILValue actor, bool needsBorrow = true) {
+                                     DIMemoryObjectInfo &TheMemory) {
 
   LLVM_DEBUG(llvm::dbgs() << "hop-injector: requested insertion after "
                           << *insertPt);
@@ -805,20 +805,30 @@ static void injectHopToExecutorAfter(SILLocation loc,
   auto injectAfter = [&](SILInstruction *insertPt) -> void {
     LLVM_DEBUG(llvm::dbgs() << "hop-injector: injecting after " << *insertPt);
     SILBuilderWithScope::insertAfter(insertPt, [&](SILBuilder &b) {
-      if (needsBorrow)
-        actor = b.createBeginBorrow(loc, actor);
+
+      const bool delegating = !TheMemory.isNonDelegatingInit();
+      SILValue val = TheMemory.getUninitializedValue();
+
+      // delegating inits always have an alloc we need to load it from.
+      if (delegating)
+        val = b.createLoad(loc.asAutoGenerated(), val,
+                           LoadOwnershipQualifier::Copy);
+
+      SILValue actor = b.createBeginBorrow(loc.asAutoGenerated(), val);
 
       b.createHopToExecutor(loc.asAutoGenerated(), actor, /*mandatory=*/false);
 
       // Distributed actors also need to notify their transport immediately
       // after performing the hop.
-      if (isDistributedActorCtor(b.getFunction())) {
+      if (!delegating && isDistributedActorCtor(b.getFunction())) {
         auto transport = findFirstDistributedActorSystemArg(b.getFunction());
         emitActorReadyCall(b, loc.asAutoGenerated(), actor, transport);
       }
 
-      if (needsBorrow)
-        b.createEndBorrow(loc, actor);
+      b.createEndBorrow(loc.asAutoGenerated(), actor);
+
+      if (delegating)
+        b.createDestroyValue(loc.asAutoGenerated(), val);
     });
   };
 
@@ -892,7 +902,7 @@ void LifetimeChecker::injectActorHopForBlock(
   // Make sure we found the initializing use of TheMemory.
   assert(bbi != bbe && "this block is not initializing?");
 
-  injectHopToExecutorAfter(loc, bbi, TheMemory.getUninitializedValue());
+  injectHopToExecutorAfter(loc, bbi, TheMemory);
 }
 
 static bool isFailableInitReturnUseOfEnum(EnumInst *EI);
@@ -929,8 +939,8 @@ void LifetimeChecker::injectActorHops() {
   // We insert this directly after the mark_uninitialized instruction, so
   // that it happens as early as `self` is available.`
   if (TheMemory.getNumElements() == 0) {
-    auto *selfDef = TheMemory.getUninitializedValue();
-    return injectHopToExecutorAfter(ctor, selfDef->getIterator(), selfDef);
+    auto *selfDef = TheMemory.getUninitializedValue(); // FIXME: this might be wrong for convenience inits (rdar://87485045)
+    return injectHopToExecutorAfter(ctor, selfDef->getIterator(), TheMemory);
   }
 
   // Returns true iff a block returns normally from the initializer,

lib/Sema/TypeCheckConcurrency.cpp

@@ -136,8 +136,13 @@ bool swift::usesFlowSensitiveIsolation(AbstractFunctionDecl const *fn) {
     if (isa<DestructorDecl>(fn))
       return true;
 
-    return requiresFlowIsolation(ActorIsolation::forActorInstance(nominal),
-                                 cast<ConstructorDecl>(fn));
+    // construct an isolation corresponding to the type.
+    auto actorTypeIso =
+      nominal->isDistributedActor()
+        ? ActorIsolation::forDistributedActorInstance(nominal)
+        : ActorIsolation::forActorInstance(nominal);
+
+    return requiresFlowIsolation(actorTypeIso, cast<ConstructorDecl>(fn));
   }
 
   // Otherwise, the type must be isolated to a global actor.
@@ -514,13 +519,12 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
     }
 
     case ActorIsolation::Independent:
-      // While some synchronous, non-delegating actor inits are
-      // nonisolated, they need cross-actor restrictions (e.g., for Sendable).
+      // While some synchronous actor inits are not isolated they still need
+      // cross-actor restrictions (e.g., for Sendable) for safety.
       if (auto *ctor = dyn_cast<ConstructorDecl>(decl))
-        if (!ctor->isConvenienceInit())
-          if (auto *parent = ctor->getParent()->getSelfClassDecl())
-            if (parent->isAnyActor())
-              return forActorSelf(parent, /*isCrossActor=*/true);
+        if (auto *parent = ctor->getParent()->getSelfClassDecl())
+          if (parent->isAnyActor())
+            return forActorSelf(parent, /*isCrossActor=*/true);
 
       // `nonisolated let` members are cross-actor as well.
       if (auto var = dyn_cast<VarDecl>(decl)) {
@@ -3570,12 +3574,11 @@ ActorIsolation ActorIsolationRequest::evaluate(
     }
   }
 
-  // Every actor's convenience or synchronous init is
-  // assumed to be actor-independent.
+  // When no other isolation applies, an actor's non-async init is independent
   if (auto nominal = value->getDeclContext()->getSelfNominalTypeDecl())
     if (nominal->isAnyActor())
       if (auto ctor = dyn_cast<ConstructorDecl>(value))
-        if (ctor->isConvenienceInit() || !ctor->hasAsync())
+        if (!ctor->hasAsync())
           defaultIsolation = ActorIsolation::forIndependent();
 
   // Function used when returning an inferred isolation.