Merge pull request swiftlang#82592 from rjmccall/local-function-isolation-fixes

Local function isolation fixes

Author: rjmccall

include/swift/AST/ActorIsolation.h

@@ -88,21 +88,75 @@ class ActorIsolation {
   /// Set to true if this was parsed from SIL.
   unsigned silParsed : 1;
 
-  unsigned parameterIndex : 27;
+  /// The opaque value of an EncodedParameterIndex.
+  /// Only meaningful for ActorInstance.
+  unsigned encodedParameterIndex : 27;
+
+  class EncodedParameterIndex {
+    enum : unsigned {
+      SpecialIndex_Capture,
+      SpecialIndex_Self,
+      NumSpecialIndexes
+    };
+
+    /// Either a special index or (parameter index + NumSpecialIndexes).
+    unsigned value;
+
+    constexpr EncodedParameterIndex(unsigned value) : value(value) {}
+
+  public:
+    static constexpr EncodedParameterIndex parameter(unsigned index) {
+      return EncodedParameterIndex(NumSpecialIndexes + index);
+    }
+    static constexpr EncodedParameterIndex self() {
+      return EncodedParameterIndex(SpecialIndex_Self);
+    }
+    static constexpr EncodedParameterIndex capture() {
+      return EncodedParameterIndex(SpecialIndex_Capture);
+    }
+
+    unsigned getParameterIndex() const {
+      assert(value >= NumSpecialIndexes);
+      return value - NumSpecialIndexes;
+    }
+    bool isSelf() const {
+      return value == SpecialIndex_Self;
+    }
+    bool isCapture() const {
+      return value == SpecialIndex_Capture;
+    }
 
-  ActorIsolation(Kind kind, NominalTypeDecl *actor, unsigned parameterIndex);
+    static EncodedParameterIndex fromOpaqueValue(unsigned value) {
+      return EncodedParameterIndex(value);
+    }
+    unsigned getOpaqueValue() const {
+      return value;
+    }
+  };
 
-  ActorIsolation(Kind kind, VarDecl *actor, unsigned parameterIndex);
+  ActorIsolation(Kind kind, NominalTypeDecl *actor,
+                 EncodedParameterIndex parameterIndex);
 
-  ActorIsolation(Kind kind, Expr *actor, unsigned parameterIndex);
+  ActorIsolation(Kind kind, VarDecl *actor,
+                 EncodedParameterIndex parameterIndex);
+
+  ActorIsolation(Kind kind, Expr *actor,
+                 EncodedParameterIndex parameterIndex);
 
   ActorIsolation(Kind kind, Type globalActor);
 
+  EncodedParameterIndex getEncodedParameterIndex() const {
+    return EncodedParameterIndex::fromOpaqueValue(encodedParameterIndex);
+  }
+
 public:
   // No-argument constructor needed for DenseMap use in PostfixCompletion.cpp
   explicit ActorIsolation(Kind kind = Unspecified, bool isSILParsed = false)
       : pointer(nullptr), kind(kind), isolatedByPreconcurrency(false),
-        silParsed(isSILParsed), parameterIndex(0) {}
+        silParsed(isSILParsed), encodedParameterIndex(0) {
+    // SIL's use of this has weaker invariants for now.
+    assert(kind != ActorInstance || isSILParsed);
+  }
 
   static ActorIsolation forUnspecified() {
     return ActorIsolation(Unspecified);
@@ -125,19 +179,22 @@ class ActorIsolation {
 
   static ActorIsolation forActorInstanceParameter(NominalTypeDecl *actor,
                                                   unsigned parameterIndex) {
-    return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
+    return ActorIsolation(ActorInstance, actor,
+                          EncodedParameterIndex::parameter(parameterIndex));
   }
 
   static ActorIsolation forActorInstanceParameter(VarDecl *actor,
                                                   unsigned parameterIndex) {
-    return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
+    return ActorIsolation(ActorInstance, actor,
+                          EncodedParameterIndex::parameter(parameterIndex));
   }
 
   static ActorIsolation forActorInstanceParameter(Expr *actor,
                                                   unsigned parameterIndex);
 
   static ActorIsolation forActorInstanceCapture(VarDecl *capturedActor) {
-    return ActorIsolation(ActorInstance, capturedActor, 0);
+    return ActorIsolation(ActorInstance, capturedActor,
+                          EncodedParameterIndex::capture());
   }
 
   static ActorIsolation forGlobalActor(Type globalActor) {
@@ -153,21 +210,14 @@ class ActorIsolation {
   static std::optional<ActorIsolation> forSILString(StringRef string) {
     auto kind =
         llvm::StringSwitch<std::optional<ActorIsolation::Kind>>(string)
-            .Case("unspecified",
-                  std::optional<ActorIsolation>(ActorIsolation::Unspecified))
-            .Case("actor_instance",
-                  std::optional<ActorIsolation>(ActorIsolation::ActorInstance))
-            .Case("nonisolated",
-                  std::optional<ActorIsolation>(ActorIsolation::Nonisolated))
-            .Case("nonisolated_unsafe", std::optional<ActorIsolation>(
-                                            ActorIsolation::NonisolatedUnsafe))
-            .Case("global_actor",
-                  std::optional<ActorIsolation>(ActorIsolation::GlobalActor))
-            .Case("global_actor_unsafe",
-                  std::optional<ActorIsolation>(ActorIsolation::GlobalActor))
+            .Case("unspecified", ActorIsolation::Unspecified)
+            .Case("actor_instance", ActorIsolation::ActorInstance)
+            .Case("nonisolated", ActorIsolation::Nonisolated)
+            .Case("nonisolated_unsafe", ActorIsolation::NonisolatedUnsafe)
+            .Case("global_actor", ActorIsolation::GlobalActor)
+            .Case("global_actor_unsafe", ActorIsolation::GlobalActor)
             .Case("caller_isolation_inheriting",
-                  std::optional<ActorIsolation>(
-                      ActorIsolation::CallerIsolationInheriting))
+                  ActorIsolation::CallerIsolationInheriting)
             .Default(std::nullopt);
     if (kind == std::nullopt)
       return std::nullopt;
@@ -187,17 +237,22 @@ class ActorIsolation {
   bool isNonisolatedUnsafe() const { return kind == NonisolatedUnsafe; }
 
   /// Retrieve the parameter to which actor-instance isolation applies.
-  ///
-  /// Parameter 0 is `self`.
-  unsigned getActorInstanceParameter() const {
+  unsigned getActorInstanceParameterIndex() const {
     assert(getKind() == ActorInstance);
-    return parameterIndex;
+    return getEncodedParameterIndex().getParameterIndex();
+  }
+
+  /// Given that this is actor instance isolation, is it a capture?
+  bool isActorInstanceForCapture() const {
+    assert(getKind() == ActorInstance);
+    return getEncodedParameterIndex().isCapture();
   }
 
   /// Returns true if this is an actor-instance isolation that additionally
   /// applies to the self parameter of a method.
   bool isActorInstanceForSelfParameter() const {
-    return getActorInstanceParameter() == 0;
+    assert(getKind() == ActorInstance);
+    return getEncodedParameterIndex().isSelf();
   }
 
   bool isSILParsed() const { return silParsed; }
@@ -285,13 +340,13 @@ class ActorIsolation {
     id.AddPointer(pointer);
     id.AddBoolean(isolatedByPreconcurrency);
     id.AddBoolean(silParsed);
-    id.AddInteger(parameterIndex);
+    id.AddInteger(encodedParameterIndex);
   }
 
   friend llvm::hash_code hash_value(const ActorIsolation &state) {
     return llvm::hash_combine(state.kind, state.pointer,
                               state.isolatedByPreconcurrency, state.silParsed,
-                              state.parameterIndex);
+                              state.encodedParameterIndex);
   }
 
   void print(llvm::raw_ostream &os) const;

lib/AST/ActorIsolation.cpp

@@ -23,23 +23,27 @@ ActorIsolation ActorIsolation::forMainActor(ASTContext &ctx) {
       ctx.getMainActorType()->mapTypeOutOfContext());
 }
 
+// These constructors are defined out-of-line so that including ActorIsolation.h
+// doesn't require a bunch of other headers to be included.
+
 ActorIsolation::ActorIsolation(Kind kind, NominalTypeDecl *actor,
-                               unsigned parameterIndex)
+                               EncodedParameterIndex parameterIndex)
     : actorInstance(actor), kind(kind), isolatedByPreconcurrency(false),
-      silParsed(false), parameterIndex(parameterIndex) {}
+      silParsed(false), encodedParameterIndex(parameterIndex.getOpaqueValue()) {}
 
 ActorIsolation::ActorIsolation(Kind kind, VarDecl *actor,
-                               unsigned parameterIndex)
+                               EncodedParameterIndex parameterIndex)
     : actorInstance(actor), kind(kind), isolatedByPreconcurrency(false),
-      silParsed(false), parameterIndex(parameterIndex) {}
+      silParsed(false), encodedParameterIndex(parameterIndex.getOpaqueValue()) {}
 
-ActorIsolation::ActorIsolation(Kind kind, Expr *actor, unsigned parameterIndex)
+ActorIsolation::ActorIsolation(Kind kind, Expr *actor,
+                               EncodedParameterIndex parameterIndex)
     : actorInstance(actor), kind(kind), isolatedByPreconcurrency(false),
-      silParsed(false), parameterIndex(parameterIndex) {}
+      silParsed(false), encodedParameterIndex(parameterIndex.getOpaqueValue()) {}
 
 ActorIsolation::ActorIsolation(Kind kind, Type globalActor)
     : globalActor(globalActor), kind(kind), isolatedByPreconcurrency(false),
-      silParsed(false), parameterIndex(0) {}
+      silParsed(false), encodedParameterIndex(0) {}
 
 ActorIsolation
 ActorIsolation::forActorInstanceParameter(Expr *actor,
@@ -67,25 +71,29 @@ ActorIsolation::forActorInstanceParameter(Expr *actor,
     }
   }
 
-  return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
+  return ActorIsolation(ActorInstance, actor,
+                        EncodedParameterIndex::parameter(parameterIndex));
 }
 
 ActorIsolation ActorIsolation::forActorInstanceSelf(ValueDecl *decl) {
   if (auto *fn = dyn_cast<AbstractFunctionDecl>(decl))
-    return ActorIsolation(ActorInstance, fn->getImplicitSelfDecl(), 0);
+    return ActorIsolation(ActorInstance, fn->getImplicitSelfDecl(),
+                          EncodedParameterIndex::self());
 
   if (auto *storage = dyn_cast<AbstractStorageDecl>(decl)) {
     if (auto *fn = storage->getAccessor(AccessorKind::Get)) {
-      return ActorIsolation(ActorInstance, fn->getImplicitSelfDecl(), 0);
+      return ActorIsolation(ActorInstance, fn->getImplicitSelfDecl(),
+                            EncodedParameterIndex::self());
     }
   }
 
   auto *dc = decl->getDeclContext();
-  return ActorIsolation(ActorInstance, dc->getSelfNominalTypeDecl(), 0);
+  return ActorIsolation(ActorInstance, dc->getSelfNominalTypeDecl(),
+                        EncodedParameterIndex::self());
 }
 
 ActorIsolation ActorIsolation::forActorInstanceSelf(NominalTypeDecl *selfDecl) {
-  return ActorIsolation(ActorInstance, selfDecl, 0);
+  return ActorIsolation(ActorInstance, selfDecl, EncodedParameterIndex::self());
 }
 
 NominalTypeDecl *ActorIsolation::getActor() const {
@@ -183,6 +191,9 @@ bool ActorIsolation::isEqual(const ActorIsolation &lhs,
     auto *lhsActor = lhs.getActorInstance();
     auto *rhsActor = rhs.getActorInstance();
     if (lhsActor && rhsActor) {
+      if (lhsActor == rhsActor)
+        return true;
+
       // FIXME: This won't work for arbitrary isolated parameter captures.
       if ((lhsActor->isSelfParameter() && rhsActor->isSelfParamCapture()) ||
           (lhsActor->isSelfParamCapture() && rhsActor->isSelfParameter())) {

lib/AST/ConformanceLookup.cpp

@@ -269,8 +269,13 @@ static ProtocolConformanceRef getBuiltinTupleTypeConformance(
   return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
 }
 
+// We can end up checking builtin conformances for generic function types
+// when e.g. we're checking whether a captured local func declaration is
+// sendable. That's fine, we can answer that question in the abstract
+// without needing to generally support conformances on generic function
+// types.
 using EitherFunctionType =
-    llvm::PointerUnion<const SILFunctionType *, const FunctionType *>;
+    llvm::PointerUnion<const SILFunctionType *, const AnyFunctionType *>;
 
 /// Whether the given function type conforms to Sendable.
 static bool isSendableFunctionType(EitherFunctionType eitherFnTy) {
@@ -288,7 +293,7 @@ static bool isSendableFunctionType(EitherFunctionType eitherFnTy) {
     representation = *converted;
 
   } else {
-    auto functionType = eitherFnTy.get<const FunctionType *>();
+    auto functionType = eitherFnTy.get<const AnyFunctionType *>();
 
     if (functionType->isSendable())
       return true;
@@ -332,7 +337,7 @@ static bool isBitwiseCopyableFunctionType(EitherFunctionType eitherFnTy) {
   if (auto silFnTy = eitherFnTy.dyn_cast<const SILFunctionType *>()) {
     representation = silFnTy->getRepresentation();
   } else {
-    auto fnTy = eitherFnTy.get<const FunctionType *>();
+    auto fnTy = eitherFnTy.get<const AnyFunctionType *>();
     representation = convertRepresentation(fnTy->getRepresentation());
   }
 
@@ -621,7 +626,7 @@ LookupConformanceInModuleRequest::evaluate(
   }
 
   // Function types can conform to protocols.
-  if (auto functionType = type->getAs<FunctionType>()) {
+  if (auto functionType = type->getAs<AnyFunctionType>()) {
     return getBuiltinFunctionTypeConformance(type, functionType, protocol);
   }
 

lib/SILGen/SILGenApply.cpp

@@ -6122,13 +6122,15 @@ RValue SILGenFunction::emitApply(
     SILValue executor;
     switch (*implicitActorHopTarget) {
     case ActorIsolation::ActorInstance:
-      if (unsigned paramIndex =
-              implicitActorHopTarget->getActorInstanceParameter()) {
+      assert(!implicitActorHopTarget->isActorInstanceForCapture());
+      if (implicitActorHopTarget->isActorInstanceForSelfParameter()) {
+        executor = emitLoadActorExecutor(loc, args.back());
+      } else {
+        unsigned paramIndex =
+          implicitActorHopTarget->getActorInstanceParameterIndex();
         auto isolatedIndex = calleeTypeInfo.origFormalType
-          ->getLoweredParamIndex(paramIndex - 1);
+          ->getLoweredParamIndex(paramIndex);
         executor = emitLoadActorExecutor(loc, args[isolatedIndex]);
-      } else {
-        executor = emitLoadActorExecutor(loc, args.back());
       }
       break;
 

lib/SILGen/SILGenConcurrency.cpp

@@ -195,7 +195,8 @@ void SILGenFunction::emitExpectedExecutorProlog() {
 
     case ActorIsolation::GlobalActor:
       if (F.isAsync() || wantDataRaceChecks) {
-        setExpectedExecutorForGlobalActor(*this, actorIsolation.getGlobalActor());
+        auto globalActorType = F.mapTypeIntoContext(actorIsolation.getGlobalActor());
+        setExpectedExecutorForGlobalActor(*this, globalActorType);
       }
       break;
     }
@@ -226,7 +227,8 @@ void SILGenFunction::emitExpectedExecutorProlog() {
 
     case ActorIsolation::GlobalActor:
       if (wantExecutor) {
-        setExpectedExecutorForGlobalActor(*this, actorIsolation.getGlobalActor());
+        auto globalActorType = F.mapTypeIntoContext(actorIsolation.getGlobalActor());
+        setExpectedExecutorForGlobalActor(*this, globalActorType);
         break;
       }
     }
@@ -573,9 +575,10 @@ SILGenFunction::emitFunctionTypeIsolation(SILLocation loc,
 
   // Emit global actor isolation by loading .shared from the global actor,
   // erasing it into `any Actor`, and injecting that into Optional.
-  case FunctionTypeIsolation::Kind::GlobalActor:
+  case FunctionTypeIsolation::Kind::GlobalActor: {
     return emitGlobalActorIsolation(loc,
              isolation.getGlobalActorType()->getCanonicalType());
+  }
 
   // Emit @isolated(any) isolation by loading the actor reference from the
   // function.
@@ -646,14 +649,14 @@ SILGenFunction::emitClosureIsolation(SILLocation loc, SILDeclRef constant,
   case ActorIsolation::Erased:
     llvm_unreachable("closures cannot directly have erased isolation");
 
-  case ActorIsolation::GlobalActor:
-    return emitGlobalActorIsolation(loc,
-             isolation.getGlobalActor()->getCanonicalType());
+  case ActorIsolation::GlobalActor: {
+    auto globalActorType = F.mapTypeIntoContext(isolation.getGlobalActor())
+                               ->getCanonicalType();
+    return emitGlobalActorIsolation(loc, globalActorType);
+  }
 
   case ActorIsolation::ActorInstance: {
-    // This should always be a capture.  That's not expressed super-cleanly
-    // in ActorIsolation, unfortunately.
-    assert(isolation.getActorInstanceParameter() == 0);
+    assert(isolation.isActorInstanceForCapture());
     auto capture = isolation.getActorInstance();
     assert(capture);
     return emitLoadOfCaptureIsolation(*this, loc, capture, constant, captures);
@@ -705,8 +708,10 @@ SILGenFunction::emitExecutor(SILLocation loc, ActorIsolation isolation,
     return emitLoadActorExecutor(loc, self);
   }
 
-  case ActorIsolation::GlobalActor:
-    return emitLoadGlobalActorExecutor(isolation.getGlobalActor());
+  case ActorIsolation::GlobalActor: {
+    auto globalActorType = F.mapTypeIntoContext(isolation.getGlobalActor());
+    return emitLoadGlobalActorExecutor(globalActorType);
+  }
   }
   llvm_unreachable("covered switch");
 }

lib/SILGen/SILGenConstructor.cpp

@@ -595,7 +595,7 @@ static bool ctorHopsInjectedByDefiniteInit(ConstructorDecl *ctor,
   // must be self-isolated
   switch (isolation) {
   case ActorIsolation::ActorInstance:
-    return isolation.getActorInstanceParameter() == 0;
+    return isolation.isActorInstanceForSelfParameter();
 
   case ActorIsolation::Erased:
     llvm_unreachable("constructor cannot have erased isolation");