Skip to content

[6.2] Local function isolation fixes #82613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 84 additions & 29 deletions include/swift/AST/ActorIsolation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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; }
Expand Down Expand Up @@ -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;
Expand Down
35 changes: 23 additions & 12 deletions lib/AST/ActorIsolation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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())) {
Expand Down
13 changes: 9 additions & 4 deletions lib/AST/ConformanceLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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);
}

Expand Down
12 changes: 7 additions & 5 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6123,13 +6123,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;

Expand Down
27 changes: 16 additions & 11 deletions lib/SILGen/SILGenConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
}
Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading