From 4b235647115108c42940590d9b4621a30ed24273 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 14 Sep 2023 21:46:49 -0700 Subject: [PATCH 1/4] [Concurrency] Rename AbstractClosureExpr::getActorIsolation to getClosureActorIsolation. This is preparation for changing AbstractClosureExpr to store ActorIsolation instead of ClosureActorIsolation, and convert to ClosureActorIsolation when needed to allow incrementally updating callers. This change is NFC. --- include/swift/AST/Expr.h | 4 +++- lib/AST/ASTDumper.cpp | 2 +- lib/AST/Decl.cpp | 2 +- lib/AST/Expr.cpp | 2 +- lib/IDE/CompletionLookup.cpp | 2 +- lib/SILGen/SILGenProlog.cpp | 4 ++-- lib/Sema/CSSimplify.cpp | 2 +- 7 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 59b407b339730..fa7e83b5aadd6 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -3951,7 +3951,9 @@ class AbstractClosureExpr : public DeclContext, public Expr { /// returns nullptr if the closure doesn't have a body BraceStmt *getBody() const; - ClosureActorIsolation getActorIsolation() const { return actorIsolation; } + ClosureActorIsolation getClosureActorIsolation() const { + return actorIsolation; + } void setActorIsolation(ClosureActorIsolation actorIsolation) { this->actorIsolation = actorIsolation; diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index c30ad29ab2186..e63e4822d3c3a 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -2687,7 +2687,7 @@ class PrintExpr : public ExprVisitor, printField(E->getRawDiscriminator(), "discriminator", DiscriminatorColor); - switch (auto isolation = E->getActorIsolation()) { + switch (auto isolation = E->getClosureActorIsolation()) { case ClosureActorIsolation::Nonisolated: break; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 13fd5f618359c..7b21fe3cd6336 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -10305,7 +10305,7 @@ bool VarDecl::isSelfParamCaptureIsolated() const { } if (auto closure = dyn_cast(dc)) { - switch (auto isolation = closure->getActorIsolation()) { + switch (auto isolation = closure->getClosureActorIsolation()) { case ClosureActorIsolation::Nonisolated: case ClosureActorIsolation::GlobalActor: return false; diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index c83d7709546de..b07decba81d4d 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2019,7 +2019,7 @@ Expr *AbstractClosureExpr::getSingleExpressionBody() const { ClosureActorIsolation swift::__AbstractClosureExpr_getActorIsolation(AbstractClosureExpr *CE) { - return CE->getActorIsolation(); + return CE->getClosureActorIsolation(); } llvm::function_ref diff --git a/lib/IDE/CompletionLookup.cpp b/lib/IDE/CompletionLookup.cpp index baa0d146eb516..de5e962103092 100644 --- a/lib/IDE/CompletionLookup.cpp +++ b/lib/IDE/CompletionLookup.cpp @@ -777,7 +777,7 @@ void CompletionLookup::analyzeActorIsolation( if (isolation != ClosureActorIsolations.end()) { return isolation->second; } else { - return CE->getActorIsolation(); + return CE->getClosureActorIsolation(); } }; auto contextIsolation = getActorIsolationOfContext( diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index 31c0f3471f180..ecb3499d5449b 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -1380,7 +1380,7 @@ void SILGenFunction::emitProlog( } } else if (auto *closureExpr = dyn_cast(FunctionDC)) { bool wantExecutor = F.isAsync() || wantDataRaceChecks; - auto actorIsolation = closureExpr->getActorIsolation(); + auto actorIsolation = closureExpr->getClosureActorIsolation(); switch (actorIsolation.getKind()) { case ClosureActorIsolation::Nonisolated: break; @@ -1593,7 +1593,7 @@ void SILGenFunction::emitHopToActorValue(SILLocation loc, ManagedValue actor) { } auto isolation = getActorIsolationOfContext(FunctionDC, [](AbstractClosureExpr *CE) { - return CE->getActorIsolation(); + return CE->getClosureActorIsolation(); }); if (isolation != ActorIsolation::Nonisolated && isolation != ActorIsolation::Unspecified) { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index f6375a4430c84..ad5f9177d116b 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2968,7 +2968,7 @@ static bool okToRemoveGlobalActor(ConstraintSystem &cs, // So, I expect the existing isolation to always be set to the default. // If the assertion below starts tripping, then this ad-hoc inference // is no longer needed! - auto existingIso = ace->getActorIsolation(); + auto existingIso = ace->getClosureActorIsolation(); if (existingIso != ClosureActorIsolation()) { assert(false && "somebody set the closure's isolation already?"); return existingIso; From 54f5fef20ef2460180145aa4b6be7ad90be5dedd Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 15 Sep 2023 23:08:27 -0700 Subject: [PATCH 2/4] [Concurrency] Store ActorIsolation in AbstractClosureExpr instead of ClosureActorIsolation. --- include/swift/AST/ActorIsolation.h | 22 +++++++++++--------- include/swift/AST/Expr.h | 32 ++++++++++++++++++++++++++---- lib/AST/Decl.cpp | 27 +++++++++++++++++++++++++ lib/AST/Expr.cpp | 4 ++-- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index f831a17074e7f..654ffb954f304 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -83,7 +83,7 @@ class ActorIsolation { private: union { - NominalTypeDecl *actor; + llvm::PointerUnion actorInstance; Type globalActor; void *pointer; }; @@ -91,9 +91,9 @@ class ActorIsolation { 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), @@ -117,6 +117,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); @@ -151,10 +155,9 @@ class ActorIsolation { } } - NominalTypeDecl *getActor() const { - assert(getKind() == ActorInstance); - return actor; - } + NominalTypeDecl *getActor() const; + + VarDecl *getCapturedActor() const; bool isGlobalActor() const { return getKind() == GlobalActor || getKind() == GlobalActorUnsafe; @@ -200,7 +203,8 @@ class ActorIsolation { 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: diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index fa7e83b5aadd6..32c21c3cde416 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -3871,14 +3871,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; } @@ -3951,14 +3952,37 @@ class AbstractClosureExpr : public DeclContext, public Expr { /// returns nullptr if the closure doesn't have a body BraceStmt *getBody() const; - ClosureActorIsolation getClosureActorIsolation() const { + ActorIsolation getActorIsolation() const { return actorIsolation; } - void setActorIsolation(ClosureActorIsolation actorIsolation) { + void setActorIsolation(ActorIsolation actorIsolation) { this->actorIsolation = actorIsolation; } + ClosureActorIsolation getClosureActorIsolation() const { + bool preconcurrency = actorIsolation.preconcurrency(); + + switch (actorIsolation) { + case ActorIsolation::Unspecified: + case ActorIsolation::Nonisolated: + return ClosureActorIsolation::forNonisolated(preconcurrency); + + case ActorIsolation::ActorInstance: + return ClosureActorIsolation::forActorInstance( + actorIsolation.getCapturedActor(), preconcurrency); + + case ActorIsolation::GlobalActor: + case ActorIsolation::GlobalActorUnsafe: + return ClosureActorIsolation::forGlobalActor( + actorIsolation.getGlobalActor(), preconcurrency); + } + } + + void setActorIsolation(ClosureActorIsolation closureIsolation) { + this->actorIsolation = closureIsolation.getActorIsolation(); + } + static bool classof(const Expr *E) { return E->getKind() >= ExprKind::First_AbstractClosureExpr && E->getKind() <= ExprKind::Last_AbstractClosureExpr; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 7b21fe3cd6336..884d19f5bdc86 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -10597,6 +10597,33 @@ void swift::simple_display(llvm::raw_ostream &out, AnyFunctionRef fn) { out << "closure"; } +ActorIsolation::ActorIsolation(Kind kind, NominalTypeDecl *actor, + unsigned parameterIndex) + : actorInstance(actor), kind(kind), + isolatedByPreconcurrency(false), + parameterIndex(parameterIndex) { } + +ActorIsolation::ActorIsolation(Kind kind, VarDecl *capturedActor) + : actorInstance(capturedActor), kind(kind), + isolatedByPreconcurrency(false), + parameterIndex(0) { } + +NominalTypeDecl *ActorIsolation::getActor() const { + assert(getKind() == ActorInstance); + + if (auto *instance = actorInstance.dyn_cast()) { + return instance->getTypeInContext() + ->getReferenceStorageReferent()->getAnyActor(); + } + + return actorInstance.get(); +} + +VarDecl *ActorIsolation::getCapturedActor() const { + assert(getKind() == ActorInstance); + return actorInstance.dyn_cast(); +} + bool ActorIsolation::isMainActor() const { if (isGlobalActor()) { if (auto *nominal = getGlobalActor()->getAnyNominal()) diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index b07decba81d4d..40b31a5f155c6 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -1895,8 +1895,8 @@ ActorIsolation ClosureActorIsolation::getActorIsolation() const { selfDecl->getTypeInContext()->getReferenceStorageReferent()->getAnyActor(); assert(actor && "Bad closure actor isolation?"); // FIXME: This could be a parameter... or a capture... hmmm. - return ActorIsolation::forActorInstanceSelf(actor).withPreconcurrency( - preconcurrency()); + return ActorIsolation::forActorInstanceCapture( + getActorInstance()).withPreconcurrency(preconcurrency()); } } } From 97f1e617fd1ee0e99f58ccbab72d7114946ba466 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Sat, 16 Sep 2023 10:46:41 -0700 Subject: [PATCH 3/4] [Concurrency] Replace ClosureActorIsolation with ActorIsolation throughout the isolation query APIs. --- include/swift/AST/ActorIsolation.h | 32 +++++++--------- include/swift/AST/Expr.h | 2 +- include/swift/IDE/CompletionLookup.h | 4 +- include/swift/IDE/PostfixCompletion.h | 2 +- include/swift/Sema/IDETypeChecking.h | 4 +- lib/AST/Decl.cpp | 6 +-- lib/AST/Expr.cpp | 9 +---- lib/IDE/CompletionLookup.cpp | 2 +- lib/IDE/PostfixCompletion.cpp | 4 +- lib/SILGen/SILGenProlog.cpp | 2 +- lib/Sema/CSSimplify.cpp | 14 +++---- lib/Sema/TypeCheckConcurrency.cpp | 53 +++++++++++++++------------ lib/Sema/TypeCheckConcurrency.h | 2 +- 13 files changed, 67 insertions(+), 69 deletions(-) diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index 654ffb954f304..a5c8b8c471e43 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/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 -_getRef__AbstractClosureExpr_getActorIsolation(); /// Determine whether the given types are (canonically) equal, declared here /// to avoid having to include Types.h. @@ -100,6 +88,11 @@ class ActorIsolation { 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); } @@ -157,7 +150,7 @@ class ActorIsolation { NominalTypeDecl *getActor() const; - VarDecl *getCapturedActor() const; + VarDecl *getActorInstance() const; bool isGlobalActor() const { return getKind() == GlobalActor || getKind() == GlobalActorUnsafe; @@ -227,6 +220,10 @@ 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 @@ -234,9 +231,8 @@ ActorIsolation getActorIsolation(ValueDecl *value); /// actor isolation of closures in the constraint system solution. ActorIsolation getActorIsolationOfContext( DeclContext *dc, - llvm::function_ref - getClosureActorIsolation = - _getRef__AbstractClosureExpr_getActorIsolation()); + llvm::function_ref + getClosureActorIsolation = __AbstractClosureExpr_getActorIsolation); /// Check if both the value, and context are isolated to the same actor. bool isSameActorIsolated(ValueDecl *value, DeclContext *dc); @@ -260,9 +256,9 @@ bool usesFlowSensitiveIsolation(AbstractFunctionDecl const *fn); /// \return true if it is safe to drop the global-actor qualifier. bool safeToDropGlobalActor( DeclContext *dc, Type globalActor, Type ty, - llvm::function_ref + llvm::function_ref getClosureActorIsolation = - _getRef__AbstractClosureExpr_getActorIsolation()); + __AbstractClosureExpr_getActorIsolation); void simple_display(llvm::raw_ostream &out, const ActorIsolation &state); diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 32c21c3cde416..dbd17efddec2f 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -3970,7 +3970,7 @@ class AbstractClosureExpr : public DeclContext, public Expr { case ActorIsolation::ActorInstance: return ClosureActorIsolation::forActorInstance( - actorIsolation.getCapturedActor(), preconcurrency); + actorIsolation.getActorInstance(), preconcurrency); case ActorIsolation::GlobalActor: case ActorIsolation::GlobalActorUnsafe: diff --git a/include/swift/IDE/CompletionLookup.h b/include/swift/IDE/CompletionLookup.h index a52a7c2a96587..d17ae9ae074a1 100644 --- a/include/swift/IDE/CompletionLookup.h +++ b/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 + llvm::DenseMap ClosureActorIsolations; bool HaveDot = false; bool IsUnwrappedOptional = false; @@ -253,7 +253,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer { } void setClosureActorIsolations( - llvm::DenseMap + llvm::DenseMap ClosureActorIsolations) { this->ClosureActorIsolations = ClosureActorIsolations; } diff --git a/include/swift/IDE/PostfixCompletion.h b/include/swift/IDE/PostfixCompletion.h index aa8d6068b43c9..f40b7d5704d36 100644 --- a/include/swift/IDE/PostfixCompletion.h +++ b/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 + llvm::DenseMap ClosureActorIsolations; /// Checks whether this result has the same \c BaseTy and \c BaseDecl as diff --git a/include/swift/Sema/IDETypeChecking.h b/include/swift/Sema/IDETypeChecking.h index 5dea28c50998c..7fffd10b44530 100644 --- a/include/swift/Sema/IDETypeChecking.h +++ b/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 getType, - llvm::function_ref + llvm::function_ref getClosureActorIsolation); /// If the capture list shadows any declarations using shorthand syntax, i.e. diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 884d19f5bdc86..1e6cd0b17a05f 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -10333,7 +10333,7 @@ ActorIsolation swift::getActorIsolation(ValueDecl *value) { ActorIsolation swift::getActorIsolationOfContext( DeclContext *dc, - llvm::function_ref + llvm::function_ref getClosureActorIsolation) { auto dcToUse = dc; // Defer bodies share actor isolation of their enclosing context. @@ -10349,7 +10349,7 @@ ActorIsolation swift::getActorIsolationOfContext( return getActorIsolation(var); if (auto *closure = dyn_cast(dcToUse)) { - return getClosureActorIsolation(closure).getActorIsolation(); + return getClosureActorIsolation(closure); } if (auto *tld = dyn_cast(dcToUse)) { @@ -10619,7 +10619,7 @@ NominalTypeDecl *ActorIsolation::getActor() const { return actorInstance.get(); } -VarDecl *ActorIsolation::getCapturedActor() const { +VarDecl *ActorIsolation::getActorInstance() const { assert(getKind() == ActorInstance); return actorInstance.dyn_cast(); } diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 40b31a5f155c6..e062f3316d35b 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2017,14 +2017,9 @@ Expr *AbstractClosureExpr::getSingleExpressionBody() const { return nullptr; } -ClosureActorIsolation +ActorIsolation swift::__AbstractClosureExpr_getActorIsolation(AbstractClosureExpr *CE) { - return CE->getClosureActorIsolation(); -} - -llvm::function_ref -swift::_getRef__AbstractClosureExpr_getActorIsolation() { - return __AbstractClosureExpr_getActorIsolation; + return CE->getActorIsolation(); } #define FORWARD_SOURCE_LOCS_TO(CLASS, NODE) \ diff --git a/lib/IDE/CompletionLookup.cpp b/lib/IDE/CompletionLookup.cpp index de5e962103092..baa0d146eb516 100644 --- a/lib/IDE/CompletionLookup.cpp +++ b/lib/IDE/CompletionLookup.cpp @@ -777,7 +777,7 @@ void CompletionLookup::analyzeActorIsolation( if (isolation != ClosureActorIsolations.end()) { return isolation->second; } else { - return CE->getClosureActorIsolation(); + return CE->getActorIsolation(); } }; auto contextIsolation = getActorIsolationOfContext( diff --git a/lib/IDE/PostfixCompletion.cpp b/lib/IDE/PostfixCompletion.cpp index 9ff2031d08959..f4ee4bf52eb5c 100644 --- a/lib/IDE/PostfixCompletion.cpp +++ b/lib/IDE/PostfixCompletion.cpp @@ -111,7 +111,7 @@ void PostfixCompletionCallback::fallbackTypeCheck(DeclContext *DC) { [&](const Solution &S) { sawSolution(S); }); } -static ClosureActorIsolation +static ActorIsolation getClosureActorIsolation(const Solution &S, AbstractClosureExpr *ACE) { auto getType = [&S](Expr *E) -> Type { // Prefer the contextual type of the closure because it might be 'weaker' @@ -213,7 +213,7 @@ void PostfixCompletionCallback::sawSolutionImpl( isImplicitSingleExpressionReturn(CS, CompletionExpr); bool IsInAsyncContext = isContextAsync(S, DC); - llvm::DenseMap + llvm::DenseMap ClosureActorIsolations; for (auto SAT : S.targets) { if (auto ACE = getAsExpr(SAT.second.getAsASTNode())) { diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index ecb3499d5449b..9cc18dc52363d 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -1593,7 +1593,7 @@ void SILGenFunction::emitHopToActorValue(SILLocation loc, ManagedValue actor) { } auto isolation = getActorIsolationOfContext(FunctionDC, [](AbstractClosureExpr *CE) { - return CE->getClosureActorIsolation(); + return CE->getActorIsolation(); }); if (isolation != ActorIsolation::Nonisolated && isolation != ActorIsolation::Unspecified) { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index ad5f9177d116b..d00025df460f1 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2960,7 +2960,7 @@ static bool isPreconcurrency(ConstraintSystem &cs, DeclContext *dc) { static bool okToRemoveGlobalActor(ConstraintSystem &cs, DeclContext *dc, Type globalActor, Type ty) { - auto findGlobalActorForClosure = [&](AbstractClosureExpr *ace) -> ClosureActorIsolation { + auto findGlobalActorForClosure = [&](AbstractClosureExpr *ace) -> ActorIsolation { // FIXME: Because the actor isolation checking happens after constraint // solving, the closure expression does not yet have its actor isolation // set, i.e., the code that would call @@ -2968,8 +2968,8 @@ static bool okToRemoveGlobalActor(ConstraintSystem &cs, // So, I expect the existing isolation to always be set to the default. // If the assertion below starts tripping, then this ad-hoc inference // is no longer needed! - auto existingIso = ace->getClosureActorIsolation(); - if (existingIso != ClosureActorIsolation()) { + auto existingIso = ace->getActorIsolation(); + if (existingIso != ActorIsolation::forUnspecified()) { assert(false && "somebody set the closure's isolation already?"); return existingIso; } @@ -2980,8 +2980,8 @@ static bool okToRemoveGlobalActor(ConstraintSystem &cs, if (auto closType = GetClosureType{cs}(ace)) { if (auto fnTy = closType->getAs()) { if (auto globActor = fnTy->getGlobalActor()) { - return ClosureActorIsolation::forGlobalActor(globActor, - isPreconcurrency(cs, ace)); + return ActorIsolation::forGlobalActor(globActor, /*unsafe=*/false) + .withPreconcurrency(isPreconcurrency(cs, ace)); } } } @@ -2989,8 +2989,8 @@ static bool okToRemoveGlobalActor(ConstraintSystem &cs, // otherwise, check for an explicit annotation if (auto *ce = dyn_cast(ace)) { if (auto globActor = getExplicitGlobalActor(ce)) { - return ClosureActorIsolation::forGlobalActor(globActor, - isPreconcurrency(cs, ce)); + return ActorIsolation::forGlobalActor(globActor, /*unsafe=*/false) + .withPreconcurrency(isPreconcurrency(cs, ace)); } } diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 041a6adb5eaad..e45f675d45171 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -1745,7 +1745,7 @@ static bool checkedByFlowIsolation(DeclContext const *refCxt, /// Get the actor isolation of the innermost relevant context. static ActorIsolation getInnermostIsolatedContext( const DeclContext *dc, - llvm::function_ref + llvm::function_ref getClosureActorIsolation) { // Retrieve the actor isolation of the context. auto mutableDC = const_cast(dc); @@ -1785,7 +1785,7 @@ bool swift::isAsyncDecl(ConcreteDeclRef declRef) { bool swift::safeToDropGlobalActor( DeclContext *dc, Type globalActor, Type ty, - llvm::function_ref + llvm::function_ref getClosureActorIsolation) { auto funcTy = ty->getAs(); if (!funcTy) @@ -1922,7 +1922,7 @@ namespace { SmallVector, 4> opaqueValues; SmallVector patternBindingStack; llvm::function_ref getType; - llvm::function_ref + llvm::function_ref getClosureActorIsolation; /// Keeps track of the capture context of variables that have been @@ -2161,7 +2161,7 @@ namespace { ActorIsolationChecker( const DeclContext *dc, llvm::function_ref getType = __Expr_getType, - llvm::function_ref + llvm::function_ref getClosureActorIsolation = __AbstractClosureExpr_getActorIsolation) : ctx(dc->getASTContext()), getType(getType), getClosureActorIsolation(getClosureActorIsolation) { @@ -2451,14 +2451,15 @@ namespace { if (auto closure = dyn_cast(dc)) { auto isolation = getClosureActorIsolation(closure); switch (isolation) { - case ClosureActorIsolation::Nonisolated: + case ActorIsolation::Unspecified: + case ActorIsolation::Nonisolated: if (isSendableClosure(closure, /*forActorIsolation=*/true)) { return ReferencedActor(var, isPotentiallyIsolated, ReferencedActor::SendableClosure); } return ReferencedActor(var, isPotentiallyIsolated, specificNonIsoClosureKind(dc)); - case ClosureActorIsolation::ActorInstance: + case ActorIsolation::ActorInstance: // If the closure is isolated to the same variable, we're all set. if (isPotentiallyIsolated && (var == isolation.getActorInstance() || @@ -2470,7 +2471,8 @@ namespace { return ReferencedActor(var, isPotentiallyIsolated, specificNonIsoClosureKind(dc)); - case ClosureActorIsolation::GlobalActor: + case ActorIsolation::GlobalActor: + case ActorIsolation::GlobalActorUnsafe: return ReferencedActor::forGlobalActor( var, isPotentiallyIsolated, isolation.getGlobalActor()); } @@ -3312,7 +3314,7 @@ namespace { /// /// This function assumes that enclosing closures have already had their /// isolation checked. - ClosureActorIsolation determineClosureIsolation( + ActorIsolation determineClosureIsolation( AbstractClosureExpr *closure) { bool preconcurrency = false; @@ -3320,22 +3322,24 @@ namespace { preconcurrency = explicitClosure->isIsolatedByPreconcurrency(); // If the closure specifies a global actor, use it. - if (Type globalActorType = resolveGlobalActorType(explicitClosure)) - return ClosureActorIsolation::forGlobalActor(globalActorType, - preconcurrency); + if (Type globalActor = resolveGlobalActorType(explicitClosure)) + return ActorIsolation::forGlobalActor(globalActor, /*unsafe=*/false) + .withPreconcurrency(preconcurrency); } // If a closure has an isolated parameter, it is isolated to that // parameter. for (auto param : *closure->getParameters()) { if (param->isIsolated()) - return ClosureActorIsolation::forActorInstance(param, preconcurrency); + return ActorIsolation::forActorInstanceCapture(param) + .withPreconcurrency(preconcurrency); } - // Sendable closures are actor-independent unless the closure has + // Sendable closures are nonisolated unless the closure has // specifically opted into inheriting actor isolation. if (isSendableClosure(closure, /*forActorIsolation=*/true)) - return ClosureActorIsolation::forNonisolated(preconcurrency); + return ActorIsolation::forNonisolated() + .withPreconcurrency(preconcurrency); // A non-Sendable closure gets its isolation from its context. auto parentIsolation = getActorIsolationOfContext( @@ -3346,21 +3350,24 @@ namespace { switch (parentIsolation) { case ActorIsolation::Nonisolated: case ActorIsolation::Unspecified: - return ClosureActorIsolation::forNonisolated(preconcurrency); + return ActorIsolation::forNonisolated() + .withPreconcurrency(preconcurrency); case ActorIsolation::GlobalActor: case ActorIsolation::GlobalActorUnsafe: { - Type globalActorType = closure->mapTypeIntoContext( + Type globalActor = closure->mapTypeIntoContext( parentIsolation.getGlobalActor()->mapTypeOutOfContext()); - return ClosureActorIsolation::forGlobalActor(globalActorType, - preconcurrency); + return ActorIsolation::forGlobalActor(globalActor, /*unsafe=*/false) + .withPreconcurrency(preconcurrency); } case ActorIsolation::ActorInstance: { if (auto param = closure->getCaptureInfo().getIsolatedParamCapture()) - return ClosureActorIsolation::forActorInstance(param, preconcurrency); + return ActorIsolation::forActorInstanceCapture(param) + .withPreconcurrency(preconcurrency); - return ClosureActorIsolation::forNonisolated(preconcurrency); + return ActorIsolation::forNonisolated() + .withPreconcurrency(preconcurrency); } } } @@ -3456,9 +3463,9 @@ void swift::checkPropertyWrapperActorIsolation( expr->walk(checker); } -ClosureActorIsolation swift::determineClosureActorIsolation( +ActorIsolation swift::determineClosureActorIsolation( AbstractClosureExpr *closure, llvm::function_ref getType, - llvm::function_ref + llvm::function_ref getClosureActorIsolation) { ActorIsolationChecker checker(closure->getParent(), getType, getClosureActorIsolation); @@ -5592,7 +5599,7 @@ ActorReferenceResult ActorReferenceResult::forReference( llvm::Optional actorInstance, llvm::Optional knownDeclIsolation, llvm::Optional knownContextIsolation, - llvm::function_ref + llvm::function_ref getClosureActorIsolation) { // If not provided, compute the isolation of the declaration, adjusted // for references. diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h index 9efcc9fd79d8b..65a5af522b1fc 100644 --- a/lib/Sema/TypeCheckConcurrency.h +++ b/lib/Sema/TypeCheckConcurrency.h @@ -240,7 +240,7 @@ struct ActorReferenceResult { llvm::Optional actorInstance = llvm::None, llvm::Optional knownDeclIsolation = llvm::None, llvm::Optional knownContextIsolation = llvm::None, - llvm::function_ref + llvm::function_ref getClosureActorIsolation = __AbstractClosureExpr_getActorIsolation); operator Kind() const { return kind; } From 549b45250f0af05d885ec8d7bd728bf5e80c28ec Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Sat, 16 Sep 2023 12:19:04 -0700 Subject: [PATCH 4/4] [Concurrency] Remove ClosureActorIsolation. --- include/swift/AST/Expr.h | 109 -------------------------------- lib/AST/ASTDumper.cpp | 10 +-- lib/AST/Decl.cpp | 10 +-- lib/AST/Expr.cpp | 23 ------- lib/SILGen/SILGenProlog.cpp | 10 +-- lib/Sema/TypeCheckConcurrency.h | 1 - 6 files changed, 18 insertions(+), 145 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index dbd17efddec2f..783f1e88f5fce 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -3777,92 +3777,6 @@ class SequenceExpr final : public Expr, } }; -/// Actor isolation for a closure. -class ClosureActorIsolation { -public: - enum Kind { - /// The closure is not isolated to any actor. - Nonisolated, - - /// 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, 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 forNonisolated(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::Nonisolated; - - if (storage.getPointer().is()) - return Kind::ActorInstance; - - return Kind::GlobalActor; - } - - /// Whether the closure is isolated at all. - explicit operator bool() const { - return getKind() != Kind::Nonisolated; - } - - /// Whether the closure is isolated at all. - operator Kind() const { - return getKind(); - } - - VarDecl *getActorInstance() const { - return storage.getPointer().dyn_cast(); - } - - Type getGlobalActor() const { - return storage.getPointer().dyn_cast(); - } - - bool preconcurrency() const { - return storage.getInt(); - } - - ActorIsolation getActorIsolation() const; -}; - /// A base class for closure expressions. class AbstractClosureExpr : public DeclContext, public Expr { CaptureInfo Captures; @@ -3960,29 +3874,6 @@ class AbstractClosureExpr : public DeclContext, public Expr { this->actorIsolation = actorIsolation; } - ClosureActorIsolation getClosureActorIsolation() const { - bool preconcurrency = actorIsolation.preconcurrency(); - - switch (actorIsolation) { - case ActorIsolation::Unspecified: - case ActorIsolation::Nonisolated: - return ClosureActorIsolation::forNonisolated(preconcurrency); - - case ActorIsolation::ActorInstance: - return ClosureActorIsolation::forActorInstance( - actorIsolation.getActorInstance(), preconcurrency); - - case ActorIsolation::GlobalActor: - case ActorIsolation::GlobalActorUnsafe: - return ClosureActorIsolation::forGlobalActor( - actorIsolation.getGlobalActor(), preconcurrency); - } - } - - void setActorIsolation(ClosureActorIsolation closureIsolation) { - this->actorIsolation = closureIsolation.getActorIsolation(); - } - static bool classof(const Expr *E) { return E->getKind() >= ExprKind::First_AbstractClosureExpr && E->getKind() <= ExprKind::Last_AbstractClosureExpr; diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index e63e4822d3c3a..86c4d7479b8fb 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -2687,16 +2687,18 @@ class PrintExpr : public ExprVisitor, printField(E->getRawDiscriminator(), "discriminator", DiscriminatorColor); - switch (auto isolation = E->getClosureActorIsolation()) { - case ClosureActorIsolation::Nonisolated: + switch (auto isolation = E->getActorIsolation()) { + case ActorIsolation::Unspecified: + case ActorIsolation::Nonisolated: break; - case ClosureActorIsolation::ActorInstance: + case ActorIsolation::ActorInstance: printFieldQuoted(isolation.getActorInstance()->printRef(), "actor_isolated", CapturesColor); break; - case ClosureActorIsolation::GlobalActor: + case ActorIsolation::GlobalActor: + case ActorIsolation::GlobalActorUnsafe: printFieldQuoted(isolation.getGlobalActor().getString(), "global_actor_isolated", CapturesColor); break; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 1e6cd0b17a05f..0397439d0a9c5 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -10305,12 +10305,14 @@ bool VarDecl::isSelfParamCaptureIsolated() const { } if (auto closure = dyn_cast(dc)) { - switch (auto isolation = closure->getClosureActorIsolation()) { - case ClosureActorIsolation::Nonisolated: - case ClosureActorIsolation::GlobalActor: + switch (auto isolation = closure->getActorIsolation()) { + case ActorIsolation::Unspecified: + case ActorIsolation::Nonisolated: + case ActorIsolation::GlobalActor: + case ActorIsolation::GlobalActorUnsafe: return false; - case ClosureActorIsolation::ActorInstance: + case ActorIsolation::ActorInstance: auto isolatedVar = isolation.getActorInstance(); return isolatedVar->isSelfParameter() || isolatedVar-isSelfParamCapture(); diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index e062f3316d35b..dd3fe9f7ac524 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -1878,29 +1878,6 @@ RebindSelfInConstructorExpr::getCalledConstructor(bool &isChainToSuper) const { return otherCtorRef; } -ActorIsolation ClosureActorIsolation::getActorIsolation() const { - switch (getKind()) { - case ClosureActorIsolation::Nonisolated: - return ActorIsolation::forNonisolated().withPreconcurrency( - preconcurrency()); - - case ClosureActorIsolation::GlobalActor: { - return ActorIsolation::forGlobalActor(getGlobalActor(), /*unsafe=*/false) - .withPreconcurrency(preconcurrency()); - } - - case ClosureActorIsolation::ActorInstance: { - auto selfDecl = getActorInstance(); - auto actor = - selfDecl->getTypeInContext()->getReferenceStorageReferent()->getAnyActor(); - assert(actor && "Bad closure actor isolation?"); - // FIXME: This could be a parameter... or a capture... hmmm. - return ActorIsolation::forActorInstanceCapture( - getActorInstance()).withPreconcurrency(preconcurrency()); - } - } -} - unsigned AbstractClosureExpr::getDiscriminator() const { auto raw = getRawDiscriminator(); if (raw != InvalidDiscriminator) diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index 9cc18dc52363d..6f6bbd7b1c6c1 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/lib/SILGen/SILGenProlog.cpp @@ -1380,19 +1380,21 @@ void SILGenFunction::emitProlog( } } else if (auto *closureExpr = dyn_cast(FunctionDC)) { bool wantExecutor = F.isAsync() || wantDataRaceChecks; - auto actorIsolation = closureExpr->getClosureActorIsolation(); + auto actorIsolation = closureExpr->getActorIsolation(); switch (actorIsolation.getKind()) { - case ClosureActorIsolation::Nonisolated: + case ActorIsolation::Unspecified: + case ActorIsolation::Nonisolated: break; - case ClosureActorIsolation::ActorInstance: { + case ActorIsolation::ActorInstance: { if (wantExecutor) { loadExpectedExecutorForLocalVar(actorIsolation.getActorInstance()); } break; } - case ClosureActorIsolation::GlobalActor: + case ActorIsolation::GlobalActor: + case ActorIsolation::GlobalActorUnsafe: if (wantExecutor) { ExpectedExecutor = emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor()); diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h index 65a5af522b1fc..3947fa1d3b6a2 100644 --- a/lib/Sema/TypeCheckConcurrency.h +++ b/lib/Sema/TypeCheckConcurrency.h @@ -33,7 +33,6 @@ class ActorIsolation; class AnyFunctionType; class ASTContext; class ClassDecl; -class ClosureActorIsolation; class ClosureExpr; class ConcreteDeclRef; class CustomAttr;