Skip to content

[concurrency] For now undo ActorIsolation::Concurrent refactoring. #79204

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 5 commits into from
Feb 7, 2025
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
58 changes: 16 additions & 42 deletions include/swift/AST/ActorIsolation.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,8 @@ class ActorIsolation {
/// the actor is isolated to the instance of that actor.
ActorInstance,
/// The declaration is explicitly specified to be not isolated to any actor,
/// meaning it cannot itself access actor isolated state but /does/ allow
/// for indirect access to actor isolated state if the declaration can
/// guarantee that all code generated to work with the declaration will run
/// on said actor.
///
/// E.x.: a nonisolated function can accept actor-isolated values as
/// arguments since the caller knows that it will not escape the values to
/// another isolation domain and that the function will remain on the
/// caller's actor.
///
/// NOTE: This used to have the meaning of Concurrent which is a stricter
/// definition of nonisolated that prevents the code generated to work with
/// the declaration to never touch actor isolated state.
/// meaning that it can be used from any actor but is also unable to
/// refer to the isolated state of any given actor.
Nonisolated,
/// The declaration is explicitly specified to be not isolated and with the
/// "unsafe" annotation, which means that we do not enforce isolation.
Expand All @@ -80,20 +69,11 @@ class ActorIsolation {
/// The actor isolation iss statically erased, as for a call to
/// an isolated(any) function. This is not possible for declarations.
Erased,
/// 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.
/// Inherits isolation from the caller of the given function.
///
/// NOTE: This used to be nonisolated, but we changed nonisolated to have a
/// weaker definition of nonisolated that allows for actor isolated state to
/// be manipulated by code generated to work with the actor as long as all
/// such code generation is guaranteed to always run on whatever actor
/// isolation it is invoked in consistently and not escape the value to any
/// other isolation domain.
Concurrent,
/// The declaration is explicitly specified to be concurrent and with the
/// "unsafe" annotation, which means that we do not enforce isolation.
ConcurrentUnsafe,
/// DISCUSSION: This is used for nonisolated asynchronous functions that we
/// want to inherit from their context the context's actor isolation.
CallerIsolationInheriting,
};

private:
Expand All @@ -108,7 +88,7 @@ class ActorIsolation {
/// Set to true if this was parsed from SIL.
unsigned silParsed : 1;

unsigned parameterIndex : 26;
unsigned parameterIndex : 27;

ActorIsolation(Kind kind, NominalTypeDecl *actor, unsigned parameterIndex);

Expand All @@ -132,8 +112,10 @@ class ActorIsolation {
return ActorIsolation(unsafe ? NonisolatedUnsafe : Nonisolated);
}

static ActorIsolation forConcurrent(bool unsafe) {
return ActorIsolation(unsafe ? ConcurrentUnsafe : Concurrent);
static ActorIsolation forCallerIsolationInheriting() {
// NOTE: We do not use parameter indices since the parameter is implicit
// from the perspective of the AST.
return ActorIsolation(CallerIsolationInheriting);
}

static ActorIsolation forActorInstanceSelf(ValueDecl *decl);
Expand Down Expand Up @@ -183,10 +165,9 @@ class ActorIsolation {
std::optional<ActorIsolation>(ActorIsolation::GlobalActor))
.Case("global_actor_unsafe",
std::optional<ActorIsolation>(ActorIsolation::GlobalActor))
.Case("concurrent",
std::optional<ActorIsolation>(ActorIsolation::Concurrent))
.Case("concurrent_unsafe", std::optional<ActorIsolation>(
ActorIsolation::ConcurrentUnsafe))
.Case("caller_isolation_inheriting",
std::optional<ActorIsolation>(
ActorIsolation::CallerIsolationInheriting))
.Default(std::nullopt);
if (kind == std::nullopt)
return std::nullopt;
Expand All @@ -199,17 +180,11 @@ class ActorIsolation {

bool isUnspecified() const { return kind == Unspecified; }

bool isConcurrent() const {
return (kind == Concurrent) || (kind == ConcurrentUnsafe);
}

bool isNonisolated() const {
return (kind == Nonisolated) || (kind == NonisolatedUnsafe);
}

bool isUnsafe() const {
return kind == NonisolatedUnsafe || kind == ConcurrentUnsafe;
}
bool isNonisolatedUnsafe() const { return kind == NonisolatedUnsafe; }

/// Retrieve the parameter to which actor-instance isolation applies.
///
Expand Down Expand Up @@ -237,8 +212,7 @@ class ActorIsolation {
case Unspecified:
case Nonisolated:
case NonisolatedUnsafe:
case Concurrent:
case ConcurrentUnsafe:
case CallerIsolationInheriting:
return false;
}
}
Expand Down
9 changes: 4 additions & 5 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3905,21 +3905,20 @@ class PrintExpr : public ExprVisitor<PrintExpr, void, Label>,
switch (auto isolation = E->getActorIsolation()) {
case ActorIsolation::Unspecified:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::ConcurrentUnsafe:
break;

case ActorIsolation::Nonisolated:
printFlag(true, "nonisolated", CapturesColor);
break;

case ActorIsolation::Concurrent:
printFlag(true, "concurrent", CapturesColor);
break;

case ActorIsolation::Erased:
printFlag(true, "dynamically_isolated", CapturesColor);
break;

case ActorIsolation::CallerIsolationInheriting:
printFlag(true, "isolated_to_caller_isolation", CapturesColor);
break;

case ActorIsolation::ActorInstance:
printReferencedDeclWithContextField(isolation.getActorInstance(),
Label::always("actor_isolated"),
Expand Down
14 changes: 5 additions & 9 deletions lib/AST/ActorIsolation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@ ActorIsolation::ActorIsolation(Kind kind, Expr *actor, unsigned parameterIndex)

ActorIsolation::ActorIsolation(Kind kind, Type globalActor)
: globalActor(globalActor), kind(kind), isolatedByPreconcurrency(false),
silParsed(false), parameterIndex(0) {
assert((silParsed || globalActor) &&
"If we are not sil parsed, global actor must be a real type");
}
silParsed(false), parameterIndex(0) {}

ActorIsolation
ActorIsolation::forActorInstanceParameter(Expr *actor,
Expand All @@ -52,7 +49,7 @@ ActorIsolation::forActorInstanceParameter(Expr *actor,
// An isolated value of `nil` is statically nonisolated.
// FIXME: Also allow 'Optional.none'
if (isa<NilLiteralExpr>(actor))
return ActorIsolation::forConcurrent(/*unsafe*/ false);
return ActorIsolation::forNonisolated(/*unsafe*/ false);

// An isolated value of `<global actor type>.shared` is statically
// global actor isolated.
Expand Down Expand Up @@ -165,8 +162,8 @@ bool ActorIsolation::isEqual(const ActorIsolation &lhs,
return false;

switch (lhs.getKind()) {
case Concurrent:
case ConcurrentUnsafe:
case Nonisolated:
case NonisolatedUnsafe:
case Unspecified:
return true;

Expand All @@ -177,8 +174,7 @@ bool ActorIsolation::isEqual(const ActorIsolation &lhs,
// to answer.
return false;

case Nonisolated:
case NonisolatedUnsafe:
case CallerIsolationInheriting:
// This returns false for the same reason as erased. The caller has to check
// against the actual caller isolation.
return false;
Expand Down
6 changes: 2 additions & 4 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2757,7 +2757,6 @@ static bool deferMatchesEnclosingAccess(const FuncDecl *defer) {
switch (isolation) {
case ActorIsolation::Unspecified:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::ConcurrentUnsafe:
break;

case ActorIsolation::GlobalActor:
Expand All @@ -2766,9 +2765,9 @@ static bool deferMatchesEnclosingAccess(const FuncDecl *defer) {

return true;

case ActorIsolation::CallerIsolationInheriting:
case ActorIsolation::ActorInstance:
case ActorIsolation::Nonisolated:
case ActorIsolation::Concurrent:
case ActorIsolation::Erased: // really can't happen
return true;
}
Expand Down Expand Up @@ -11462,10 +11461,9 @@ bool VarDecl::isSelfParamCaptureIsolated() const {
case ActorIsolation::Unspecified:
case ActorIsolation::Nonisolated:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::Concurrent:
case ActorIsolation::ConcurrentUnsafe:
case ActorIsolation::GlobalActor:
case ActorIsolation::Erased:
case ActorIsolation::CallerIsolationInheriting:
return false;

case ActorIsolation::ActorInstance:
Expand Down
41 changes: 19 additions & 22 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1856,11 +1856,10 @@ SourceLoc MacroDefinitionRequest::getNearestLoc() const {

bool ActorIsolation::requiresSubstitution() const {
switch (kind) {
case CallerIsolationInheriting:
case ActorInstance:
case Nonisolated:
case NonisolatedUnsafe:
case Concurrent:
case ConcurrentUnsafe:
case Unspecified:
return false;

Expand All @@ -1873,10 +1872,9 @@ bool ActorIsolation::requiresSubstitution() const {
ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const {
switch (kind) {
case ActorInstance:
case CallerIsolationInheriting:
case Nonisolated:
case NonisolatedUnsafe:
case Concurrent:
case ConcurrentUnsafe:
case Unspecified:
return *this;

Expand All @@ -1895,6 +1893,11 @@ void ActorIsolation::printForDiagnostics(llvm::raw_ostream &os,
os << "actor" << (asNoun ? " isolation" : "-isolated");
break;

case ActorIsolation::CallerIsolationInheriting:
os << "caller isolation inheriting"
<< (asNoun ? " isolation" : "-isolated");
break;

case ActorIsolation::GlobalActor: {
if (isMainActor()) {
os << "main actor" << (asNoun ? " isolation" : "-isolated");
Expand All @@ -1909,13 +1912,11 @@ void ActorIsolation::printForDiagnostics(llvm::raw_ostream &os,
os << "@isolated(any)";
break;

case ActorIsolation::Concurrent:
case ActorIsolation::ConcurrentUnsafe:
case ActorIsolation::Nonisolated:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::Unspecified:
os << "nonisolated";
if (isUnsafe()) {
if (*this == ActorIsolation::NonisolatedUnsafe) {
os << "(unsafe)";
}
break;
Expand All @@ -1933,18 +1934,15 @@ void ActorIsolation::print(llvm::raw_ostream &os) const {
os << ". name: '" << vd->getBaseIdentifier() << "'";
}
return;
case CallerIsolationInheriting:
os << "caller_isolation_inheriting";
return;
case Nonisolated:
os << "nonisolated";
return;
case NonisolatedUnsafe:
os << "nonisolated_unsafe";
return;
case Concurrent:
os << "concurrent";
return;
case ConcurrentUnsafe:
os << "concurrent_unsafe";
return;
case GlobalActor:
os << "global_actor. type: " << getGlobalActor();
return;
Expand All @@ -1963,18 +1961,15 @@ void ActorIsolation::printForSIL(llvm::raw_ostream &os) const {
case ActorInstance:
os << "actor_instance";
return;
case CallerIsolationInheriting:
os << "caller_isolation_inheriting";
return;
case Nonisolated:
os << "nonisolated";
return;
case NonisolatedUnsafe:
os << "nonisolated_unsafe";
return;
case Concurrent:
os << "concurrent";
return;
case ConcurrentUnsafe:
os << "concurrent_unsafe";
return;
case GlobalActor:
os << "global_actor";
return;
Expand Down Expand Up @@ -2014,12 +2009,14 @@ void swift::simple_display(
}
break;

case ActorIsolation::CallerIsolationInheriting:
out << "isolated to isolation of caller";
break;

case ActorIsolation::Nonisolated:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::Concurrent:
case ActorIsolation::ConcurrentUnsafe:
out << "nonisolated";
if (state.isUnsafe()) {
if (state == ActorIsolation::NonisolatedUnsafe) {
out << "(unsafe)";
}
break;
Expand Down
3 changes: 1 addition & 2 deletions lib/IDE/CompletionLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -827,9 +827,8 @@ void CompletionLookup::analyzeActorIsolation(
break;
case ActorIsolation::Unspecified:
case ActorIsolation::Nonisolated:
case ActorIsolation::CallerIsolationInheriting:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::Concurrent:
case ActorIsolation::ConcurrentUnsafe:
return;
}
}
Expand Down
9 changes: 5 additions & 4 deletions lib/SIL/IR/SILFunctionType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1651,7 +1651,8 @@ class DestructureInputs {
// enabled.
if (TC.Context.LangOpts.hasFeature(
Feature::NonIsolatedAsyncInheritsIsolationFromContext) &&
IsolationInfo && IsolationInfo->isNonisolated() &&
IsolationInfo &&
IsolationInfo->getKind() == ActorIsolation::CallerIsolationInheriting &&
extInfoBuilder.isAsync()) {
auto actorProtocol = TC.Context.getProtocol(KnownProtocolKind::Actor);
auto actorType =
Expand Down Expand Up @@ -2529,15 +2530,15 @@ static CanSILFunctionType getSILFunctionType(
std::optional<ActorIsolation> actorIsolation;
if (constant) {
if (constant->kind == SILDeclRef::Kind::Deallocator) {
actorIsolation = ActorIsolation::forConcurrent(false);
actorIsolation = ActorIsolation::forNonisolated(false);
} else if (auto *decl = constant->getAbstractFunctionDecl();
decl && decl->getExecutionBehavior().has_value()) {
switch (*decl->getExecutionBehavior()) {
case ExecutionKind::Concurrent:
actorIsolation = ActorIsolation::forConcurrent(false /*unsafe*/);
actorIsolation = ActorIsolation::forNonisolated(false /*unsafe*/);
break;
case ExecutionKind::Caller:
actorIsolation = ActorIsolation::forNonisolated(false /*unsafe*/);
actorIsolation = ActorIsolation::forCallerIsolationInheriting();
break;
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ static ActorIsolation getActorIsolationForFunction(SILFunction &fn) {
if (constant.kind == SILDeclRef::Kind::Deallocator) {
// Deallocating destructor is always nonisolated. Isolation of the deinit
// applies only to isolated deallocator and destroyer.
return ActorIsolation::forConcurrent(false);
return ActorIsolation::forNonisolated(false);
}

// If we have actor isolation for our constant, put the isolation onto the
Expand Down
6 changes: 2 additions & 4 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3104,9 +3104,8 @@ static void emitDelayedArguments(SILGenFunction &SGF,

case ActorIsolation::Unspecified:
case ActorIsolation::Nonisolated:
case ActorIsolation::CallerIsolationInheriting:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::Concurrent:
case ActorIsolation::ConcurrentUnsafe:
llvm_unreachable("Not isolated");
}

Expand Down Expand Up @@ -5833,9 +5832,8 @@ RValue SILGenFunction::emitApply(

case ActorIsolation::Unspecified:
case ActorIsolation::Nonisolated:
case ActorIsolation::CallerIsolationInheriting:
case ActorIsolation::NonisolatedUnsafe:
case ActorIsolation::Concurrent:
case ActorIsolation::ConcurrentUnsafe:
llvm_unreachable("Not isolated");
break;
}
Expand Down
Loading