Merge pull request #40028 from kavon/flowsensitive-isolation

Author: swift-ci

include/swift/AST/ActorIsolation.h

@@ -26,8 +26,10 @@ class raw_ostream;
 namespace swift {
 class DeclContext;
 class ModuleDecl;
+class VarDecl;
 class NominalTypeDecl;
 class SubstitutionMap;
+class AbstractFunctionDecl;
 
 /// Determine whether the given types are (canonically) equal, declared here
 /// to avoid having to include Types.h.
@@ -36,6 +38,11 @@ bool areTypesEqual(Type type1, Type type2);
 /// Determine whether the given type is suitable as a concurrent value type.
 bool isSendableType(ModuleDecl *module, Type type);
 
+/// Determines if the 'let' can be read from anywhere within the given module,
+/// regardless of the isolation or async-ness of the context in which
+/// the var is read.
+bool isLetAccessibleAnywhere(const ModuleDecl *fromModule, VarDecl *let);
+
 /// Describes the actor isolation of a given declaration, which determines
 /// the actors with which it can interact.
 class ActorIsolation {
@@ -169,6 +176,9 @@ ActorIsolation getActorIsolation(ValueDecl *value);
 /// Determine how the given declaration context is isolated.
 ActorIsolation getActorIsolationOfContext(DeclContext *dc);
 
+/// Determines whether this function's body uses flow-sensitive isolation.
+bool usesFlowSensitiveIsolation(AbstractFunctionDecl const *fn);
+
 void simple_display(llvm::raw_ostream &out, const ActorIsolation &state);
 
 } // end namespace swift

include/swift/AST/DiagnosticsSIL.def

@@ -168,19 +168,6 @@ ERROR(variable_defer_use_uninit,none,
 ERROR(self_closure_use_uninit,none,
       "'self' captured by a closure before all members were initialized", ())
 
-ERROR(self_disallowed_nonisolated_actor_init,none,
-      "%select{|this use of }0actor 'self' cannot %2 %select{from|in}0 "
-      "%select{a non-isolated, designated|a global-actor isolated}1 initializer",
-      (bool, bool, StringRef))
-ERROR(self_disallowed_plain_actor_init,none,
-      "%select{|this use of }0actor 'self' can only %1 %select{from|in}0 an async initializer",
-      (bool, StringRef))
-NOTE(actor_convenience_init,none,
-     "convenience initializers allow non-isolated use of 'self' once "
-     "initialized",
-     ())
-
-
 
 
 ERROR(variable_addrtaken_before_initialized,none,
@@ -625,6 +612,16 @@ WARNING(warning_int_to_fp_inexact, none,
   "'%1' is not exactly representable as %0; it becomes '%2'",
   (Type, StringRef, StringRef))
 
+// Flow-isolation diagnostics
+ERROR(isolated_after_nonisolated, none,
+      "cannot access %1 %2 here in %select{non-isolated initializer|deinitializer}0",
+      (bool, DescriptiveDeclKind, DeclName))
+NOTE(nonisolated_blame, none, "after %1%2 %3, "
+     "only non-isolated properties of 'self' can be accessed from "
+     "%select{this init|a deinit}0", (bool, StringRef, StringRef, DeclName))
+ERROR(non_sendable_from_deinit,none,
+        "cannot access %1 %2 with a non-sendable type %0 from non-isolated deinit",
+        (Type, DescriptiveDeclKind, DeclName))
 
 // Yield usage errors
 ERROR(return_before_yield, none, "accessor must yield before returning",())

include/swift/AST/DiagnosticsSema.def

@@ -4708,9 +4708,6 @@ ERROR(nonisolated_local_var,none,
 ERROR(nonisolated_actor_sync_init,none,
       "'nonisolated' on an actor's synchronous initializer is invalid",
       ())
-ERROR(nonisolated_actor_convenience_init,none,
-      "'nonisolated' on an actor's convenience initializer is redundant",
-      ())
 
 ERROR(actor_instance_property_wrapper,none,
       "%0 property in property wrapper type %1 cannot be isolated to "

include/swift/SILOptimizer/PassManager/Passes.def

@@ -224,6 +224,8 @@ PASS(EmitDFDiagnostics, "dataflow-diagnostics",
      "Emit SIL Diagnostics")
 PASS(EscapeAnalysisDumper, "escapes-dump",
      "Dump Escape Analysis Results")
+PASS(FlowIsolation, "flow-isolation",
+     "Enforces flow-sensitive actor isolation rules")
 PASS(FunctionOrderPrinter, "function-order-printer",
      "Print Function Order for Testing")
 PASS(FunctionSignatureOpts, "function-signature-opts",

lib/AST/Decl.cpp

@@ -1985,6 +1985,45 @@ static bool isPolymorphic(const AbstractStorageDecl *storage) {
   return false;
 }
 
+/// Returns true iff a defer's storage access kind should always
+/// match the access kind of its immediately enclosing function.
+///
+/// In Swift 5 and earlier, this was not true, meaning that property observers,
+/// etc, would be invoked in initializers or deinitializers if a property access
+/// happens within a defer, but not when outside the defer.
+static bool deferMatchesEnclosingAccess(const FuncDecl *defer) {
+  assert(defer->isDeferBody());
+
+  // In Swift 6+, then yes.
+  if (defer->getASTContext().isSwiftVersionAtLeast(6))
+    return true;
+
+  // If the defer is part of a function that is a member of an actor or
+  // concurrency-aware type, then yes.
+  if (auto *deferContext = defer->getParent()) {
+    if (auto *funcContext = deferContext->getParent()) {
+      if (auto *type = funcContext->getSelfNominalTypeDecl()) {
+        if (type->isAnyActor())
+          return true;
+
+        switch (getActorIsolation(type)) {
+          case swift::ActorIsolation::Unspecified:
+          case swift::ActorIsolation::GlobalActorUnsafe:
+            break;
+
+          case swift::ActorIsolation::ActorInstance:
+          case swift::ActorIsolation::DistributedActorInstance:
+          case swift::ActorIsolation::Independent:
+          case swift::ActorIsolation::GlobalActor:
+            return true;
+        }
+      }
+    }
+  }
+
+  return false;
+}
+
 static bool isDirectToStorageAccess(const DeclContext *UseDC,
                                     const VarDecl *var, bool isAccessOnSelf) {
   if (!var->hasStorage())
@@ -1994,6 +2033,11 @@ static bool isDirectToStorageAccess(const DeclContext *UseDC,
   if (AFD == nullptr)
     return false;
 
+  // Check if this is a function representing a defer.
+  if (auto *func = dyn_cast<FuncDecl>(AFD))
+    if (func->isDeferBody() && deferMatchesEnclosingAccess(func))
+      return isDirectToStorageAccess(func->getParent(), var, isAccessOnSelf);
+
   // The property reference is for immediate class, not a derived class.
   if (AFD->getParent()->getSelfNominalTypeDecl() !=
       var->getDeclContext()->getSelfNominalTypeDecl())

lib/SILGen/SILGenConstructor.cpp

@@ -315,6 +315,45 @@ 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,
+                                           ActorIsolation const& isolation) {
+  // must be async, but we can assume that.
+  assert(ctor->hasAsync());
+
+  auto *dc = ctor->getDeclContext();
+  auto selfClassDecl = dc->getSelfClassDecl();
+
+  // must be an actor
+  if (!selfClassDecl || !selfClassDecl->isAnyActor())
+    return false;
+
+  // must be instance isolated
+  switch (isolation) {
+    case ActorIsolation::ActorInstance:
+    case ActorIsolation::DistributedActorInstance:
+      return true;
+
+    case ActorIsolation::Unspecified:
+    case ActorIsolation::Independent:
+    case ActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActorUnsafe:
+      return false;
+  }
+}
+
 void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
   MagicFunctionName = SILGenModule::getMagicFunctionName(ctor);
 
@@ -360,9 +399,13 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
 
   // Make sure we've hopped to the right global actor, if any.
   if (ctor->hasAsync()) {
-    SILLocation prologueLoc(selfDecl);
-    prologueLoc.markAsPrologue();
-    emitConstructorPrologActorHop(prologueLoc, getActorIsolation(ctor));
+    auto isolation = getActorIsolation(ctor);
+    // if it's not injected by definite init, we do it in the prologue now.
+    if (!ctorHopsInjectedByDefiniteInit(ctor, isolation)) {
+      SILLocation prologueLoc(selfDecl);
+      prologueLoc.markAsPrologue();
+      emitConstructorPrologActorHop(prologueLoc, isolation);
+    }
   }
 
   // Create a basic block to jump to for the implicit 'self' return.
@@ -754,10 +797,14 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
     selfClassDecl->isDistributedActor() && !isDelegating;
 
   // Make sure we've hopped to the right global actor, if any.
-  if (ctor->hasAsync() && !selfClassDecl->isActor()) {
-    SILLocation prologueLoc(selfDecl);
-    prologueLoc.markAsPrologue();
-    emitConstructorPrologActorHop(prologueLoc, getActorIsolation(ctor));
+  if (ctor->hasAsync()) {
+    auto isolation = getActorIsolation(ctor);
+    // if it's not injected by definite init, we do it in the prologue now.
+    if (!ctorHopsInjectedByDefiniteInit(ctor, isolation)) {
+      SILLocation prologueLoc(selfDecl);
+      prologueLoc.markAsPrologue();
+      emitConstructorPrologActorHop(prologueLoc, isolation);
+    }
   }
 
   if (!NeedsBoxForSelf) {

lib/SILOptimizer/Mandatory/CMakeLists.txt

@@ -14,6 +14,7 @@ target_sources(swiftSILOptimizer PRIVATE
   DiagnoseStaticExclusivity.cpp
   DiagnoseUnreachable.cpp
   Differentiation.cpp
+  FlowIsolation.cpp
   IRGenPrepare.cpp
   LexicalLifetimeEliminator.cpp
   LowerHopToActor.cpp

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

@@ -450,11 +450,10 @@ ConstructorDecl *DIMemoryObjectInfo::getActorInitSelf() const {
       // is it for an actor?
       if (decl->isAnyActor())
         if (auto *silFn = MemoryInst->getFunction())
-          // is it a designated initializer?
+          // are we in a constructor?
           if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(
                             silFn->getDeclContext()->getAsDecl()))
-            if (ctor->isDesignatedInit())
-              return ctor;
+            return ctor;
 
   return nullptr;
 }

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h

@@ -169,7 +169,7 @@ class DIMemoryObjectInfo {
   }
 
   /// Returns the initializer if the memory use is 'self' and appears in an
-  /// actor's designated initializer. Otherwise, returns nullptr.
+  /// actor's initializer. Otherwise, returns nullptr.
   ConstructorDecl *getActorInitSelf() const;
 
   /// True if this memory object is the 'self' of a derived class initializer.