Merge pull request #68852 from hborla/5.10-sendable-fixes

[5.10][Concurrency] An assortment of bug fixes in `Sendable` and actor isolation checking.

Author: hborla

include/swift/AST/ActorIsolation.h

@@ -31,18 +31,6 @@ class NominalTypeDecl;
 class SubstitutionMap;
 class AbstractFunctionDecl;
 class AbstractClosureExpr;
-class ClosureActorIsolation;
-
-/// Trampoline for AbstractClosureExpr::getActorIsolation.
-ClosureActorIsolation
-__AbstractClosureExpr_getActorIsolation(AbstractClosureExpr *CE);
-
-/// Returns a function reference to \c __AbstractClosureExpr_getActorIsolation.
-/// This is needed so we can use it as a default argument for
-/// \c getActorIsolationOfContext without knowing the layout of
-/// \c ClosureActorIsolation.
-llvm::function_ref<ClosureActorIsolation(AbstractClosureExpr *)>
-_getRef__AbstractClosureExpr_getActorIsolation();
 
 /// Determine whether the given types are (canonically) equal, declared here
 /// to avoid having to include Types.h.
@@ -68,10 +56,10 @@ class ActorIsolation {
     /// For example, a mutable stored property or synchronous function within
     /// the actor is isolated to the instance of that actor.
     ActorInstance,
-    /// The declaration is explicitly specified to be independent of any actor,
+    /// The declaration is explicitly specified to be not isolated to any actor,
     /// meaning that it can be used from any actor but is also unable to
     /// refer to the isolated state of any given actor.
-    Independent,
+    Nonisolated,
     /// The declaration is isolated to a global actor. It can refer to other
     /// entities with the same global actor.
     GlobalActor,
@@ -83,29 +71,34 @@ class ActorIsolation {
 
 private:
   union {
-    NominalTypeDecl *actor;
+    llvm::PointerUnion<NominalTypeDecl *, VarDecl *> actorInstance;
     Type globalActor;
     void *pointer;
   };
   unsigned kind : 3;
   unsigned isolatedByPreconcurrency : 1;
   unsigned parameterIndex : 28;
 
-  ActorIsolation(Kind kind, NominalTypeDecl *actor, unsigned parameterIndex)
-      : actor(actor), kind(kind), isolatedByPreconcurrency(false),
-        parameterIndex(parameterIndex) { }
+  ActorIsolation(Kind kind, NominalTypeDecl *actor, unsigned parameterIndex);
+
+  ActorIsolation(Kind kind, VarDecl *capturedActor);
 
   ActorIsolation(Kind kind, Type globalActor)
       : globalActor(globalActor), kind(kind), isolatedByPreconcurrency(false),
         parameterIndex(0) { }
 
 public:
+  // No-argument constructor needed for DenseMap use in PostfixCompletion.cpp
+  explicit ActorIsolation(Kind kind = Unspecified)
+      : pointer(nullptr), kind(kind), isolatedByPreconcurrency(false),
+        parameterIndex(0) { }
+
   static ActorIsolation forUnspecified() {
     return ActorIsolation(Unspecified, nullptr);
   }
 
-  static ActorIsolation forIndependent() {
-    return ActorIsolation(Independent, nullptr);
+  static ActorIsolation forNonisolated() {
+    return ActorIsolation(Nonisolated, nullptr);
   }
 
   static ActorIsolation forActorInstanceSelf(NominalTypeDecl *actor) {
@@ -117,6 +110,10 @@ class ActorIsolation {
     return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
   }
 
+  static ActorIsolation forActorInstanceCapture(VarDecl *capturedActor) {
+    return ActorIsolation(ActorInstance, capturedActor);
+  }
+
   static ActorIsolation forGlobalActor(Type globalActor, bool unsafe) {
     return ActorIsolation(
         unsafe ? GlobalActorUnsafe : GlobalActor, globalActor);
@@ -128,7 +125,7 @@ class ActorIsolation {
 
   bool isUnspecified() const { return kind == Unspecified; }
 
-  bool isIndependent() const { return kind == Independent; }
+  bool isNonisolated() const { return kind == Nonisolated; }
 
   /// Retrieve the parameter to which actor-instance isolation applies.
   ///
@@ -146,15 +143,14 @@ class ActorIsolation {
       return true;
 
     case Unspecified:
-    case Independent:
+    case Nonisolated:
       return false;
     }
   }
 
-  NominalTypeDecl *getActor() const {
-    assert(getKind() == ActorInstance);
-    return actor;
-  }
+  NominalTypeDecl *getActor() const;
+
+  VarDecl *getActorInstance() const;
 
   bool isGlobalActor() const {
     return getKind() == GlobalActor || getKind() == GlobalActorUnsafe;
@@ -195,12 +191,13 @@ class ActorIsolation {
       return false;
 
     switch (lhs.getKind()) {
-    case Independent:
+    case Nonisolated:
     case Unspecified:
       return true;
 
     case ActorInstance:
-      return lhs.actor == rhs.actor && lhs.parameterIndex == rhs.parameterIndex;
+      return (lhs.getActor() == rhs.getActor() &&
+              lhs.parameterIndex == rhs.parameterIndex);
 
     case GlobalActor:
     case GlobalActorUnsafe:
@@ -223,43 +220,26 @@ class ActorIsolation {
 /// Determine how the given value declaration is isolated.
 ActorIsolation getActorIsolation(ValueDecl *value);
 
+/// Trampoline for AbstractClosureExpr::getActorIsolation.
+ActorIsolation
+__AbstractClosureExpr_getActorIsolation(AbstractClosureExpr *CE);
+
 /// Determine how the given declaration context is isolated.
 /// \p getClosureActorIsolation allows the specification of actor isolation for
 /// closures that haven't been saved been saved to the AST yet. This is useful
 /// for solver-based code completion which doesn't modify the AST but stores the
 /// actor isolation of closures in the constraint system solution.
 ActorIsolation getActorIsolationOfContext(
     DeclContext *dc,
-    llvm::function_ref<ClosureActorIsolation(AbstractClosureExpr *)>
-        getClosureActorIsolation =
-            _getRef__AbstractClosureExpr_getActorIsolation());
+    llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
+        getClosureActorIsolation = __AbstractClosureExpr_getActorIsolation);
 
 /// Check if both the value, and context are isolated to the same actor.
 bool isSameActorIsolated(ValueDecl *value, DeclContext *dc);
 
 /// Determines whether this function's body uses flow-sensitive isolation.
 bool usesFlowSensitiveIsolation(AbstractFunctionDecl const *fn);
 
-/// Check if it is safe for the \c globalActor qualifier to be removed from
-/// \c ty, when the function value of that type is isolated to that actor.
-///
-/// In general this is safe in a narrow but common case: a global actor
-/// qualifier can be dropped from a function type while in a DeclContext
-/// isolated to that same actor, as long as the value is not Sendable.
-///
-/// \param dc the innermost context in which the cast to remove the global actor
-///           is happening.
-/// \param globalActor global actor that was dropped from \c ty.
-/// \param ty a function type where \c globalActor was removed from it.
-/// \param getClosureActorIsolation function that knows how to produce accurate
-///        information about the isolation of a closure.
-/// \return true if it is safe to drop the global-actor qualifier.
-bool safeToDropGlobalActor(
-                DeclContext *dc, Type globalActor, Type ty,
-                llvm::function_ref<ClosureActorIsolation(AbstractClosureExpr *)>
-                    getClosureActorIsolation =
-                        _getRef__AbstractClosureExpr_getActorIsolation());
-
 void simple_display(llvm::raw_ostream &out, const ActorIsolation &state);
 
 } // end namespace swift

include/swift/AST/DiagnosticsSema.def

@@ -5236,8 +5236,9 @@ ERROR(concurrent_access_of_inout_param,none,
       "concurrently-executing code",
       (DeclName))
 ERROR(non_sendable_capture,none,
-      "capture of %1 with non-sendable type %0 in a `@Sendable` closure",
-      (Type, DeclName))
+      "capture of %1 with non-sendable type %0 in a `@Sendable` "
+      "%select{local function|closure}2",
+      (Type, DeclName, bool))
 ERROR(implicit_async_let_non_sendable_capture,none,
       "capture of %1 with non-sendable type %0 in 'async let' binding",
       (Type, DeclName))

include/swift/AST/Expr.h

@@ -3774,92 +3774,6 @@ class SequenceExpr final : public Expr,
   }
 };
 
-/// Actor isolation for a closure.
-class ClosureActorIsolation {
-public:
-  enum Kind {
-    /// The closure is independent of any actor.
-    Independent,
-
-    /// The closure is tied to the actor instance described by the given
-    /// \c VarDecl*, which is the (captured) `self` of an actor.
-    ActorInstance,
-
-    /// The closure is tied to the global actor described by the given type.
-    GlobalActor,
-  };
-
-private:
-    /// The actor to which this closure is isolated, plus a bit indicating
-    /// whether the isolation was imposed by a preconcurrency declaration.
-    ///
-    /// There are three possible states for the pointer:
-    ///   - NULL: The closure is independent of any actor.
-    ///   - VarDecl*: The 'self' variable for the actor instance to which
-    ///     this closure is isolated. It will always have a type that conforms
-    ///     to the \c Actor protocol.
-    ///   - Type: The type of the global actor on which
-  llvm::PointerIntPair<llvm::PointerUnion<VarDecl *, Type>, 1, bool> storage;
-
-  ClosureActorIsolation(VarDecl *selfDecl, bool preconcurrency)
-      : storage(selfDecl, preconcurrency) { }
-  ClosureActorIsolation(Type globalActorType, bool preconcurrency)
-      : storage(globalActorType, preconcurrency) { }
-
-public:
-  ClosureActorIsolation(bool preconcurrency = false)
-      : storage(nullptr, preconcurrency) { }
-
-  static ClosureActorIsolation forIndependent(bool preconcurrency) {
-    return ClosureActorIsolation(preconcurrency);
-  }
-
-  static ClosureActorIsolation forActorInstance(VarDecl *selfDecl,
-                                                bool preconcurrency) {
-    return ClosureActorIsolation(selfDecl, preconcurrency);
-  }
-
-  static ClosureActorIsolation forGlobalActor(Type globalActorType,
-                                              bool preconcurrency) {
-    return ClosureActorIsolation(globalActorType, preconcurrency);
-  }
-
-  /// Determine the kind of isolation.
-  Kind getKind() const {
-    if (storage.getPointer().isNull())
-      return Kind::Independent;
-
-    if (storage.getPointer().is<VarDecl *>())
-      return Kind::ActorInstance;
-
-    return Kind::GlobalActor;
-  }
-
-  /// Whether the closure is isolated at all.
-  explicit operator bool() const {
-    return getKind() != Kind::Independent;
-  }
-
-  /// Whether the closure is isolated at all.
-  operator Kind() const {
-    return getKind();
-  }
-
-  VarDecl *getActorInstance() const {
-    return storage.getPointer().dyn_cast<VarDecl *>();
-  }
-
-  Type getGlobalActor() const {
-    return storage.getPointer().dyn_cast<Type>();
-  }
-
-  bool preconcurrency() const {
-    return storage.getInt();
-  }
-
-  ActorIsolation getActorIsolation() const;
-};
-
 /// A base class for closure expressions.
 class AbstractClosureExpr : public DeclContext, public Expr {
   CaptureInfo Captures;
@@ -3868,14 +3782,15 @@ class AbstractClosureExpr : public DeclContext, public Expr {
   ParameterList *parameterList;
 
   /// Actor isolation of the closure.
-  ClosureActorIsolation actorIsolation;
+  ActorIsolation actorIsolation;
 
 public:
   AbstractClosureExpr(ExprKind Kind, Type FnType, bool Implicit,
                       DeclContext *Parent)
       : DeclContext(DeclContextKind::AbstractClosureExpr, Parent),
         Expr(Kind, Implicit, FnType),
-        parameterList(nullptr) {
+        parameterList(nullptr),
+        actorIsolation(ActorIsolation::forUnspecified()) {
     Bits.AbstractClosureExpr.Discriminator = InvalidDiscriminator;
   }
 
@@ -3948,9 +3863,11 @@ class AbstractClosureExpr : public DeclContext, public Expr {
   /// returns nullptr if the closure doesn't have a body
   BraceStmt *getBody() const;
 
-  ClosureActorIsolation getActorIsolation() const { return actorIsolation; }
+  ActorIsolation getActorIsolation() const {
+    return actorIsolation;
+  }
 
-  void setActorIsolation(ClosureActorIsolation actorIsolation) {
+  void setActorIsolation(ActorIsolation actorIsolation) {
     this->actorIsolation = actorIsolation;
   }
 

include/swift/IDE/CompletionLookup.h

@@ -131,7 +131,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
   bool CanCurrDeclContextHandleAsync = false;
   /// Actor isolations that were determined during constraint solving but that
   /// haven't been saved to the AST.
-  llvm::DenseMap<AbstractClosureExpr *, ClosureActorIsolation>
+  llvm::DenseMap<AbstractClosureExpr *, ActorIsolation>
       ClosureActorIsolations;
   bool HaveDot = false;
   bool IsUnwrappedOptional = false;
@@ -253,7 +253,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
   }
 
   void setClosureActorIsolations(
-      llvm::DenseMap<AbstractClosureExpr *, ClosureActorIsolation>
+      llvm::DenseMap<AbstractClosureExpr *, ActorIsolation>
           ClosureActorIsolations) {
     this->ClosureActorIsolations = ClosureActorIsolations;
   }

include/swift/IDE/PostfixCompletion.h

@@ -63,7 +63,7 @@ class PostfixCompletionCallback : public TypeCheckCompletionCallback {
 
     /// Actor isolations that were determined during constraint solving but that
     /// haven't been saved to the AST.
-    llvm::DenseMap<AbstractClosureExpr *, ClosureActorIsolation>
+    llvm::DenseMap<AbstractClosureExpr *, ActorIsolation>
         ClosureActorIsolations;
 
     /// Checks whether this result has the same \c BaseTy and \c BaseDecl as

include/swift/Sema/IDETypeChecking.h

@@ -318,9 +318,9 @@ namespace swift {
   bool completionContextUsesConcurrencyFeatures(const DeclContext *dc);
 
   /// Determine the isolation of a particular closure.
-  ClosureActorIsolation determineClosureActorIsolation(
+  ActorIsolation determineClosureActorIsolation(
       AbstractClosureExpr *closure, llvm::function_ref<Type(Expr *)> getType,
-      llvm::function_ref<ClosureActorIsolation(AbstractClosureExpr *)>
+      llvm::function_ref<ActorIsolation(AbstractClosureExpr *)>
           getClosureActorIsolation);
 
   /// If the capture list shadows any declarations using shorthand syntax, i.e.

lib/AST/ASTDumper.cpp

@@ -2593,15 +2593,17 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
       << " discriminator=" << E->getRawDiscriminator();
 
     switch (auto isolation = E->getActorIsolation()) {
-    case ClosureActorIsolation::Independent:
+    case ActorIsolation::Unspecified:
+    case ActorIsolation::Nonisolated:
       break;
 
-    case ClosureActorIsolation::ActorInstance:
+    case ActorIsolation::ActorInstance:
       PrintWithColorRAII(OS, CapturesColor) << " actor-isolated="
         << isolation.getActorInstance()->printRef();
       break;
 
-    case ClosureActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActorUnsafe:
       PrintWithColorRAII(OS, CapturesColor) << " global-actor-isolated="
         << isolation.getGlobalActor().getString();
       break;