Eliminate ImplicitActorHopTarget in favor of ActorIsolation.

DougGregor · DougGregor · commit 5191c315e100 · 2022-08-23T23:19:46.000-07:00
The generalized ActorIsolation is enough to represent everything that
ImplicitActorHopTarget can do, and we were mapping between the two way
too often, so collapse them.
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -1163,80 +1163,19 @@ class DiscardAssignmentExpr : public Expr {
   }
 };
 
-/// Describes the actor to which an implicit-async expression will hop.
-struct ImplicitActorHopTarget {
-  enum Kind {
-    /// The "self" instance.
-    InstanceSelf,
-    /// A global actor with the given type.
-    GlobalActor,
-    /// An isolated parameter in a call.
-    IsolatedParameter,
-  };
-
-private:
-  /// The lower two bits are the Kind, and the remaining bits are used for
-  /// the payload, which might by a TypeBase * (for a global actor) or a
-  /// integer value (for an isolated parameter).
-  uintptr_t bits;
-
-  constexpr ImplicitActorHopTarget(uintptr_t bits) : bits(bits) { }
-
-public:
-  /// Default-initialized to instance "self".
-  constexpr ImplicitActorHopTarget() : bits(0) { }
-
-  static ImplicitActorHopTarget forInstanceSelf() {
-    return ImplicitActorHopTarget(InstanceSelf);
-  }
-
-  static ImplicitActorHopTarget forGlobalActor(Type globalActor) {
-    uintptr_t bits =
-      reinterpret_cast<uintptr_t>(globalActor.getPointer()) | GlobalActor;
-    return ImplicitActorHopTarget(bits);
-  }
-
-  static ImplicitActorHopTarget forIsolatedParameter(unsigned index) {
-    uintptr_t bits = static_cast<uintptr_t>(index) << 2 | IsolatedParameter;
-    return ImplicitActorHopTarget(bits);
-  }
-
-  /// Determine the kind of implicit actor hop being performed.
-  Kind getKind() const {
-    return static_cast<Kind>(bits & 0x03);
-  }
-
-  operator Kind() const {
-    return getKind();
-  }
-
-  /// Retrieve the global actor type for an implicit hop to a global actor.
-  Type getGlobalActor() const {
-    assert(getKind() == GlobalActor);
-    return Type(reinterpret_cast<TypeBase *>(bits & ~0x03));
-  }
-
-  /// Retrieve the (zero-based) parameter index for the isolated parameter
-  /// in a call.
-  unsigned getIsolatedParameterIndex() const {
-    assert(getKind() == IsolatedParameter);
-    return bits >> 2;
-  }
-};
-
-
 /// DeclRefExpr - A reference to a value, "x".
 class DeclRefExpr : public Expr {
   /// The declaration pointer.
   ConcreteDeclRef D;
   DeclNameLoc Loc;
-  ImplicitActorHopTarget implicitActorHopTarget;
+  ActorIsolation implicitActorHopTarget;
 
 public:
   DeclRefExpr(ConcreteDeclRef D, DeclNameLoc Loc, bool Implicit,
               AccessSemantics semantics = AccessSemantics::Ordinary,
               Type Ty = Type())
-    : Expr(ExprKind::DeclRef, Implicit, Ty), D(D), Loc(Loc) {
+    : Expr(ExprKind::DeclRef, Implicit, Ty), D(D), Loc(Loc),
+      implicitActorHopTarget(ActorIsolation::forUnspecified()) {
     Bits.DeclRefExpr.Semantics = (unsigned) semantics;
     Bits.DeclRefExpr.FunctionRefKind =
       static_cast<unsigned>(Loc.isCompound() ? FunctionRefKind::Compound
@@ -1258,15 +1197,15 @@ class DeclRefExpr : public Expr {
 
   /// Determine whether this reference needs to happen asynchronously, i.e.,
   /// guarded by hop_to_executor, and if so describe the target.
-  Optional<ImplicitActorHopTarget> isImplicitlyAsync() const {
+  Optional<ActorIsolation> isImplicitlyAsync() const {
     if (!Bits.DeclRefExpr.IsImplicitlyAsync)
       return None;
 
     return implicitActorHopTarget;
   }
 
   /// Note that this reference is implicitly async and set the target.
-  void setImplicitlyAsync(ImplicitActorHopTarget target) {
+  void setImplicitlyAsync(ActorIsolation target) {
     Bits.DeclRefExpr.IsImplicitlyAsync = true;
     implicitActorHopTarget = target;
   }
@@ -1600,12 +1539,13 @@ class UnresolvedDeclRefExpr : public Expr {
 class LookupExpr : public Expr {
   Expr *Base;
   ConcreteDeclRef Member;
-  ImplicitActorHopTarget implicitActorHopTarget;
+  ActorIsolation implicitActorHopTarget;
 
 protected:
   explicit LookupExpr(ExprKind Kind, Expr *base, ConcreteDeclRef member,
                           bool Implicit)
-      : Expr(Kind, Implicit), Base(base), Member(member) {
+      : Expr(Kind, Implicit), Base(base), Member(member),
+        implicitActorHopTarget(ActorIsolation::forUnspecified()) {
     Bits.LookupExpr.IsSuper = false;
     Bits.LookupExpr.IsImplicitlyAsync = false;
     Bits.LookupExpr.IsImplicitlyThrows = false;
@@ -1640,15 +1580,15 @@ class LookupExpr : public Expr {
 
   /// Determine whether this reference needs to happen asynchronously, i.e.,
   /// guarded by hop_to_executor, and if so describe the target.
-  Optional<ImplicitActorHopTarget> isImplicitlyAsync() const {
+  Optional<ActorIsolation> isImplicitlyAsync() const {
     if (!Bits.LookupExpr.IsImplicitlyAsync)
       return None;
 
     return implicitActorHopTarget;
   }
 
   /// Note that this reference is implicitly async and set the target.
-  void setImplicitlyAsync(ImplicitActorHopTarget target) {
+  void setImplicitlyAsync(ActorIsolation target) {
     Bits.LookupExpr.IsImplicitlyAsync = true;
     implicitActorHopTarget = target;
   }
@@ -4482,12 +4422,13 @@ class ApplyExpr : public Expr {
   /// The list of arguments to call the function with.
   ArgumentList *ArgList;
 
-  ImplicitActorHopTarget implicitActorHopTarget;
+  ActorIsolation implicitActorHopTarget;
 
 protected:
   ApplyExpr(ExprKind kind, Expr *fn, ArgumentList *argList, bool implicit,
             Type ty = Type())
-      : Expr(kind, implicit, ty), Fn(fn), ArgList(argList) {
+      : Expr(kind, implicit, ty), Fn(fn), ArgList(argList),
+        implicitActorHopTarget(ActorIsolation::forUnspecified()) {
     assert(ArgList);
     assert(classof((Expr*)this) && "ApplyExpr::classof out of date");
     Bits.ApplyExpr.ThrowsIsSet = false;
@@ -4553,15 +4494,15 @@ class ApplyExpr : public Expr {
   ///
   /// When the application is implicitly async, the result describes
   /// the actor to which we need to need to hop.
-  Optional<ImplicitActorHopTarget> isImplicitlyAsync() const {
+  Optional<ActorIsolation> isImplicitlyAsync() const {
     if (!Bits.ApplyExpr.ImplicitlyAsync)
       return None;
 
     return implicitActorHopTarget;
   }
 
   /// Note that this application is implicitly async and set the target.
-  void setImplicitlyAsync(ImplicitActorHopTarget target) {
+  void setImplicitlyAsync(ActorIsolation target) {
     Bits.ApplyExpr.ImplicitlyAsync = true;
     implicitActorHopTarget = target;
   }
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -3886,7 +3886,7 @@ class CallEmission {
 
   Callee callee;
   FormalEvaluationScope initialWritebackScope;
-  Optional<ImplicitActorHopTarget> implicitActorHopTarget;
+  Optional<ActorIsolation> implicitActorHopTarget;
   bool implicitlyThrows;
 
 public:
@@ -3937,7 +3937,7 @@ class CallEmission {
   /// implicitly async, i.e., it requires a hop_to_executor prior to 
   /// invoking the sync callee, etc.
   void setImplicitlyAsync(
-      Optional<ImplicitActorHopTarget> implicitActorHopTarget) {
+      Optional<ActorIsolation> implicitActorHopTarget) {
     this->implicitActorHopTarget = implicitActorHopTarget;
   }
 
@@ -4573,7 +4573,7 @@ RValue SILGenFunction::emitApply(
     ArrayRef<ManagedValue> args,
     const CalleeTypeInfo &calleeTypeInfo,
     ApplyOptions options, SGFContext evalContext,
-    Optional<ImplicitActorHopTarget> implicitActorHopTarget) {
+    Optional<ActorIsolation> implicitActorHopTarget) {
   auto substFnType = calleeTypeInfo.substFnType;
   auto substResultType = calleeTypeInfo.substResultType;
 
@@ -4672,18 +4672,24 @@ RValue SILGenFunction::emitApply(
 
     SILValue executor;
     switch (*implicitActorHopTarget) {
-    case ImplicitActorHopTarget::InstanceSelf:
-      executor = emitLoadActorExecutor(loc, args.back());
+    case ActorIsolation::ActorInstance:
+      if (unsigned paramIndex =
+              implicitActorHopTarget->getActorInstanceParameter()) {
+        executor = emitLoadActorExecutor(loc, args[paramIndex-1]);
+      } else {
+        executor = emitLoadActorExecutor(loc, args.back());
+      }
       break;
 
-    case ImplicitActorHopTarget::GlobalActor:
+    case ActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActorUnsafe:
       executor = emitLoadGlobalActorExecutor(
           implicitActorHopTarget->getGlobalActor());
       break;
 
-    case ImplicitActorHopTarget::IsolatedParameter:
-      executor = emitLoadActorExecutor(
-          loc, args[implicitActorHopTarget->getIsolatedParameterIndex()]);
+    case ActorIsolation::Unspecified:
+    case ActorIsolation::Independent:
+      llvm_unreachable("Not isolated");
       break;
     }
 
@@ -5897,7 +5903,7 @@ RValue SILGenFunction::emitGetAccessor(
     PreparedArguments &&subscriptIndices,
     SGFContext c,
     bool isOnSelfParameter,
-    Optional<ImplicitActorHopTarget> implicitActorHopTarget) {
+    Optional<ActorIsolation> implicitActorHopTarget) {
   // Scope any further writeback just within this operation.
   FormalEvaluationScope writebackScope(*this);
 
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
@@ -1451,7 +1451,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
       bool isDirectAccessorUse,
       PreparedArguments &&optionalSubscripts, SGFContext C,
       bool isOnSelfParameter,
-      Optional<ImplicitActorHopTarget> implicitActorHopTarget = None);
+      Optional<ActorIsolation> implicitActorHopTarget = None);
 
   void emitSetAccessor(SILLocation loc, SILDeclRef setter,
                        SubstitutionMap substitutions,
@@ -1684,7 +1684,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                    ArrayRef<ManagedValue> args,
                    const CalleeTypeInfo &calleeTypeInfo, ApplyOptions options,
                    SGFContext evalContext, 
-                   Optional<ImplicitActorHopTarget> implicitActorHopTarget);
+                   Optional<ActorIsolation> implicitActorHopTarget);
 
   RValue emitApplyOfDefaultArgGenerator(SILLocation loc,
                                         ConcreteDeclRef defaultArgsOwner,
diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp
@@ -1648,23 +1648,13 @@ namespace {
       RValue rvalue;
       FormalEvaluationScope scope(SGF);
 
-      // FIXME: This somewhat silly, because the original expression should
-      // already have one of these.
-      Optional<ImplicitActorHopTarget> implicitActorHopTarget;
-      if (ActorIso) {
-        implicitActorHopTarget = ActorIso->isGlobalActor()
-            ? ImplicitActorHopTarget::forGlobalActor(
-              ActorIso->getGlobalActor())
-            : ImplicitActorHopTarget::forInstanceSelf();
-      }
-
       auto args =
           std::move(*this).prepareAccessorArgs(SGF, loc, base, getter);
 
       rvalue = SGF.emitGetAccessor(
           loc, getter, Substitutions, std::move(args.base), IsSuper,
           IsDirectAccessorUse, std::move(args.Indices), c,
-          IsOnSelfParameter, implicitActorHopTarget);
+          IsOnSelfParameter, ActorIso);
 
       return rvalue;
     }
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -1783,7 +1783,7 @@ namespace {
     ///
     /// and we reach up to mark the CallExpr.
     void markNearestCallAsImplicitly(
-        Optional<ImplicitActorHopTarget> setAsync,
+        Optional<ActorIsolation> setAsync,
         bool setThrows = false,
         bool setDistributedThunk = false) {
       assert(applyStack.size() > 0 && "not contained within an Apply?");
@@ -2369,7 +2369,7 @@ namespace {
     AsyncMarkingResult tryMarkImplicitlyAsync(SourceLoc declLoc,
                                               ConcreteDeclRef concDeclRef,
                                               Expr* context,
-                                              ImplicitActorHopTarget target,
+                                              ActorIsolation target,
                                               bool isDistributed) {
       ValueDecl *decl = concDeclRef.getDecl();
       AsyncMarkingResult result = AsyncMarkingResult::NotFound;
@@ -2583,25 +2583,8 @@ namespace {
       }
 
       // Mark as implicitly async.
-      if (!fnType->getExtInfo().isAsync()) {
-        switch (*unsatisfiedIsolation) {
-        case ActorIsolation::GlobalActor:
-        case ActorIsolation::GlobalActorUnsafe:
-          apply->setImplicitlyAsync(
-              ImplicitActorHopTarget::forGlobalActor(
-                unsatisfiedIsolation->getGlobalActor()));
-          break;
-
-        case ActorIsolation::ActorInstance:
-          apply->setImplicitlyAsync(
-            ImplicitActorHopTarget::forIsolatedParameter(
-                unsatisfiedIsolation->getActorInstanceParameter() - 1));
-          break;
-
-        case ActorIsolation::Unspecified:
-        case ActorIsolation::Independent:
-          llvm_unreachable("Not actor-isolated");
-        }
+      if (!fnType->getExtInfo().isAsync() && unsatisfiedIsolation) {
+        apply->setImplicitlyAsync(*unsatisfiedIsolation);
       }
 
       // Check for sendability of the parameter types.
@@ -2879,12 +2862,8 @@ namespace {
           ActorReferenceResult::Flags::Distributed);
 
       // Determine the actor hop.
-      ImplicitActorHopTarget target = result.isolation.isGlobalActor()
-          ? ImplicitActorHopTarget::forGlobalActor(
-              result.isolation.getGlobalActor())
-          : ImplicitActorHopTarget::forInstanceSelf();
       auto implicitAsyncResult = tryMarkImplicitlyAsync(
-          loc, declRef, context, target, isDistributed);
+          loc, declRef, context, result.isolation, isDistributed);
       switch (implicitAsyncResult) {
       case AsyncMarkingResult::FoundAsync:
         // Success! We're done.
