From 3d3b1ca50a6cf02645018d08fce0bfc7f31d49ca Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 19 Mar 2025 18:45:32 +0000 Subject: [PATCH 1/7] [NFC] AST: Turn `getParameterList` into a method on `ValueDecl` --- include/swift/AST/Decl.h | 20 ++++----- lib/AST/ASTMangler.cpp | 2 +- lib/AST/Decl.cpp | 43 +++++++++++-------- lib/AST/DeclContext.cpp | 2 +- lib/AST/FeatureSet.cpp | 2 +- lib/AST/RawComment.cpp | 2 +- lib/AST/Type.cpp | 4 +- lib/IDE/Formatting.cpp | 2 +- lib/IDE/SourceEntityWalker.cpp | 2 +- lib/Sema/CSApply.cpp | 4 +- lib/Sema/CSDiagnostics.cpp | 2 +- lib/Sema/CSSimplify.cpp | 6 +-- lib/Sema/TypeCheckConcurrency.cpp | 4 +- lib/Sema/TypeCheckDeclOverride.cpp | 4 +- lib/Sema/TypeCheckDeclPrimary.cpp | 2 +- lib/Sema/TypeCheckEffects.cpp | 2 +- lib/Sema/TypeCheckProtocol.cpp | 17 +++----- .../lib/SwiftLang/SwiftLangSupport.cpp | 2 +- 18 files changed, 58 insertions(+), 64 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index f985a5dc053cb..4909814471c04 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3320,6 +3320,14 @@ class ValueDecl : public Decl { /// parameter lists, for example an enum element without associated values. bool hasParameterList() const; + /// Returns the parameter list directly associated with this declaration, + /// or `nullptr` if there is none. + /// + /// Note that some declarations with function interface types do not have + /// parameter lists. For example, an enum element without associated values. + ParameterList *getParameterList(); + const ParameterList *getParameterList() const; + /// Returns the number of curry levels in the declaration's interface type. unsigned getNumCurryLevels() const; @@ -9720,14 +9728,6 @@ inline bool ValueDecl::hasCurriedSelf() const { return false; } -inline bool ValueDecl::hasParameterList() const { - if (auto *eed = dyn_cast(this)) - return eed->hasAssociatedValues(); - if (auto *macro = dyn_cast(this)) - return macro->parameterList != nullptr; - return isa(this) || isa(this); -} - inline unsigned ValueDecl::getNumCurryLevels() const { unsigned curryLevels = 0; if (hasParameterList()) @@ -9815,10 +9815,6 @@ inline EnumElementDecl *EnumDecl::getUniqueElement(bool hasValue) const { return result; } -/// Retrieve the parameter list for a given declaration, or nullptr if there -/// is none. -ParameterList *getParameterList(ValueDecl *source); - /// Retrieve the parameter list for a given declaration context, or nullptr if /// there is none. ParameterList *getParameterList(DeclContext *source); diff --git a/lib/AST/ASTMangler.cpp b/lib/AST/ASTMangler.cpp index 19c75b343f4d1..316f876088369 100644 --- a/lib/AST/ASTMangler.cpp +++ b/lib/AST/ASTMangler.cpp @@ -1074,7 +1074,7 @@ static unsigned getUnnamedParamIndex(const ParamDecl *D) { if (isa(DC)) { ParamList = cast(DC)->getParameters(); } else { - ParamList = getParameterList(cast(DC->getAsDecl())); + ParamList = cast(DC->getAsDecl())->getParameterList(); } unsigned UnnamedIndex = 0; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index f8b6d375b3660..1e2adae980c0c 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1316,8 +1316,7 @@ bool Decl::hasUnderscoredNaming() const { // underscore, it's a private function or subscript. if (isa(D) || isa(D)) { const auto VD = cast(D); - if (getParameterList(const_cast(VD)) - ->hasInternalParameter("_")) { + if (VD->getParameterList()->hasInternalParameter("_")) { return true; } } @@ -4096,6 +4095,26 @@ ValueDecl::getCachedOpaqueResultTypeDecl() const { .getCachedResult(); } +ParameterList *ValueDecl::getParameterList() { + if (auto *function = dyn_cast(this)) { + return function->getParameters(); + } else if (auto *enumElement = dyn_cast(this)) { + return enumElement->getParameterList(); + } else if (auto *subscript = dyn_cast(this)) { + return subscript->getIndices(); + } else if (auto *macro = dyn_cast(this)) { + return macro->parameterList; + } + + return nullptr; +} + +const ParameterList *ValueDecl::getParameterList() const { + return const_cast(this)->getParameterList(); +} + +bool ValueDecl::hasParameterList() const { return (bool)getParameterList(); } + bool ValueDecl::isObjC() const { ASTContext &ctx = getASTContext(); return evaluateOrDefault(ctx.evaluator, @@ -9578,24 +9597,10 @@ DeclName AbstractFunctionDecl::getEffectiveFullName() const { return DeclName(); } -ParameterList *swift::getParameterList(ValueDecl *source) { - if (auto *AFD = dyn_cast(source)) { - return AFD->getParameters(); - } else if (auto *EED = dyn_cast(source)) { - return EED->getParameterList(); - } else if (auto *SD = dyn_cast(source)) { - return SD->getIndices(); - } else if (auto *MD = dyn_cast(source)) { - return MD->parameterList; - } - - return nullptr; -} - ParameterList *swift::getParameterList(DeclContext *source) { if (auto *D = source->getAsDecl()) { if (auto *VD = dyn_cast(D)) { - return getParameterList(VD); + return VD->getParameterList(); } } else if (auto *CE = dyn_cast(source)) { return CE->getParameters(); @@ -9607,7 +9612,7 @@ ParameterList *swift::getParameterList(DeclContext *source) { const ParamDecl *swift::getParameterAt(ConcreteDeclRef declRef, unsigned index) { auto *source = declRef.getDecl(); - if (auto *params = getParameterList(const_cast(source))) { + if (auto *params = source->getParameterList()) { unsigned origIndex = params->getOrigParamIndex(declRef.getSubstitutions(), index); return params->get(origIndex); @@ -9617,7 +9622,7 @@ const ParamDecl *swift::getParameterAt(ConcreteDeclRef declRef, const ParamDecl *swift::getParameterAt(const ValueDecl *source, unsigned index) { - if (auto *params = getParameterList(const_cast(source))) { + if (auto *params = source->getParameterList()) { return index < params->size() ? params->get(index) : nullptr; } return nullptr; diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp index 743ae3d349a0d..c75a3e3a10dbc 100644 --- a/lib/AST/DeclContext.cpp +++ b/lib/AST/DeclContext.cpp @@ -501,7 +501,7 @@ swift::FragileFunctionKindRequest::evaluate(Evaluator &evaluator, dc = dc->getParent(); auto *VD = cast(dc->getAsDecl()); - assert(VD->hasParameterList()); + ASSERT(VD->hasParameterList()); if (VD->getDeclContext()->isLocalContext()) { auto kind = VD->getDeclContext()->getFragileFunctionKind(); diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index 8371f4e9ca9cf..eb49250ef48ee 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -497,7 +497,7 @@ static bool usesFeatureExecutionAttribute(Decl *decl) { }; // Check if any parameters that have `@execution` attribute. - if (auto *PL = getParameterList(VD)) { + if (auto *PL = VD->getParameterList()) { for (auto *P : *PL) { if (hasExecutionAttr(P->getTypeRepr())) return true; diff --git a/lib/AST/RawComment.cpp b/lib/AST/RawComment.cpp index e1d6c69f1e2f8..3c9b42e7eef97 100644 --- a/lib/AST/RawComment.cpp +++ b/lib/AST/RawComment.cpp @@ -242,7 +242,7 @@ static bool hasDoubleUnderscore(const Decl *D) { // If it's a function or subscript with a parameter with leading // double underscore, it's a private function or subscript. if (isa(D) || isa(D)) { - auto *params = getParameterList(cast(const_cast(D))); + auto *params = cast(D)->getParameterList(); if (params->hasInternalParameter(Prefix)) return true; } diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 921ed5e07720b..806ea4e3017b6 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -1336,13 +1336,11 @@ ParameterListInfo::ParameterListInfo( return; // Find the corresponding parameter list. - const ParameterList *paramList = - getParameterList(const_cast(paramOwner)); + auto *paramList = paramOwner->getParameterList(); // No parameter list means no default arguments - hand back the zeroed // bitvector. if (!paramList) { - assert(!paramOwner->hasParameterList()); return; } diff --git a/lib/IDE/Formatting.cpp b/lib/IDE/Formatting.cpp index 46ce189e1a572..2f2e80c6def41 100644 --- a/lib/IDE/Formatting.cpp +++ b/lib/IDE/Formatting.cpp @@ -518,7 +518,7 @@ class RangeWalker: protected ASTWalker { if (!handleBraces(cast(D)->getBracesRange(), ContextLoc)) return Action::Stop(); } - auto *PL = getParameterList(cast(D)); + auto *PL = cast(D)->getParameterList(); if (!handleParens(PL->getLParenLoc(), PL->getRParenLoc(), ContextLoc)) return Action::Stop(); } else if (auto *PGD = dyn_cast(D)) { diff --git a/lib/IDE/SourceEntityWalker.cpp b/lib/IDE/SourceEntityWalker.cpp index 2032751a803f6..eb9acc1716e95 100644 --- a/lib/IDE/SourceEntityWalker.cpp +++ b/lib/IDE/SourceEntityWalker.cpp @@ -169,7 +169,7 @@ ASTWalker::PreWalkAction SemaAnnotator::walkToDeclPreProper(Decl *D) { }; if (isa(VD) || isa(VD)) { - auto ParamList = getParameterList(VD); + auto ParamList = VD->getParameterList(); if (!ReportParamList(ParamList)) return Action::Stop(); } diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index ccd6b2febf7f5..9f7ec0b6b270b 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -6230,8 +6230,8 @@ ArgumentList *ExprRewriter::coerceCallArguments( // If bindings were substituted we need to find "original" // (or contextless) parameter index for the default argument. if (shouldSubstituteBindings) { - auto *paramList = getParameterList(callee.getDecl()); - assert(paramList); + auto *paramList = callee.getDecl()->getParameterList(); + ASSERT(paramList); paramIdxForDefault = paramList->getOrigParamIndex(callee.getSubstitutions(), paramIdx); } diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 8dda76009c7aa..a0f490cb9615c 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -9354,7 +9354,7 @@ bool DefaultExprTypeMismatch::diagnoseAsError() { auto overload = getSolution().getCalleeOverloadChoice(locator); - auto *PD = getParameterList(overload.choice.getDecl())->get(paramIdx); + auto *PD = overload.choice.getDecl()->getParameterList()->get(paramIdx); auto note = emitDiagnosticAt(PD->getLoc(), diag::default_value_declared_here); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 797a8e886fe55..adc820a5c4bd9 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1703,7 +1703,7 @@ static ConstraintSystem::TypeMatchResult matchCallArguments( if (parameterBindings[paramIdx].empty() && callee) { // Type inference from default value expressions. { - auto *paramList = getParameterList(callee); + auto *paramList = callee->getParameterList(); if (!paramList) continue; @@ -5758,7 +5758,7 @@ bool ConstraintSystem::repairFailures( // Ignore decls that don't have meaningful parameter lists - this // matches variables and parameters with function types. - auto *paramList = getParameterList(overload->choice.getDecl()); + auto *paramList = overload->choice.getDecl()->getParameterList(); if (!paramList) return true; @@ -6052,7 +6052,7 @@ bool ConstraintSystem::repairFailures( if (auto overload = findSelectedOverloadFor(calleeLocator)) { if (auto *decl = overload->choice.getDeclOrNull()) { - if (auto paramList = getParameterList(decl)) { + if (auto paramList = decl->getParameterList()) { if (paramList->get(paramIdx)->getTypeOfDefaultExpr()) { conversionsOrFixes.push_back( IgnoreDefaultExprTypeMismatch::create(*this, lhs, rhs, loc)); diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 5f31cfbb15ee3..7fa6d830c50b5 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -5644,7 +5644,7 @@ static OverrideIsolationResult validOverrideIsolation( /// Retrieve the index of the first isolated parameter of the given /// declaration, if there is one. static std::optional getIsolatedParamIndex(ValueDecl *value) { - auto params = getParameterList(value); + auto *params = value->getParameterList(); if (!params) return std::nullopt; @@ -5864,7 +5864,7 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator, if (auto paramIdx = getIsolatedParamIndex(value)) { checkDeclWithIsolatedParameter(value); - ParamDecl *param = getParameterList(value)->get(*paramIdx); + ParamDecl *param = value->getParameterList()->get(*paramIdx); Type paramType = param->getDeclContext()->mapTypeIntoContext( param->getInterfaceType()); Type actorType; diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 5384d1254d649..35b046b52148e 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -648,8 +648,8 @@ static bool parameterTypesMatch(const ValueDecl *derivedDecl, if ((isa(derivedDecl) && isa(baseDecl)) || isa(baseDecl)) { - derivedParams = getParameterList(const_cast(derivedDecl)); - baseParams = getParameterList(const_cast(baseDecl)); + derivedParams = derivedDecl->getParameterList(); + baseParams = baseDecl->getParameterList(); } if (!derivedParams && !baseParams) { diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index b943b4fc46c5d..5664aff3f48f3 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1365,7 +1365,7 @@ DefaultArgumentInitializer * DefaultArgumentInitContextRequest::evaluate(Evaluator &eval, ParamDecl *param) const { auto *parentDC = param->getDeclContext(); - auto *paramList = getParameterList(cast(parentDC->getAsDecl())); + auto *paramList = cast(parentDC->getAsDecl())->getParameterList(); // In order to compute the initializer context for this parameter, we need to // know its index in the parameter list. Therefore iterate over the parameters diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp index 595523f9d748a..231719f6f1128 100644 --- a/lib/Sema/TypeCheckEffects.cpp +++ b/lib/Sema/TypeCheckEffects.cpp @@ -398,7 +398,7 @@ class AbstractFunction { .getParameterType(); case Kind::Function: { - auto params = getParameterList(static_cast(getFunction())); + auto *params = getFunction()->getParameters(); auto origIndex = params->getOrigParamIndex(getSubstitutions(), substIndex); return params->get(origIndex)->getInterfaceType(); } diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index b9e2a83307718..50f96b6fdbfa9 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -804,10 +804,10 @@ RequirementMatch swift::matchWitness( return RequirementMatch(witness, MatchKind::TypeConflict, witnessType); - ParameterList *witnessParamList = getParameterList(witness); + ParameterList *witnessParamList = witness->getParameterList(); assert(witnessParamList->size() == witnessParams.size()); - ParameterList *reqParamList = getParameterList(req); + ParameterList *reqParamList = req->getParameterList(); assert(reqParamList->size() == reqParams.size()); // Match each of the parameters. @@ -2837,10 +2837,8 @@ SourceLoc OptionalAdjustment::getOptionalityLoc(ValueDecl *witness) const { } // For parameter adjustments, dig out the pattern. - ParameterList *params = nullptr; - if (isa(witness) || isa(witness)) { - params = getParameterList(witness); - } else { + auto *params = witness->getParameterList(); + if (!params) { return SourceLoc(); } @@ -6010,14 +6008,11 @@ static bool isGeneric(ValueDecl *decl) { /// Determine whether this is an unlabeled initializer or subscript. static bool isUnlabeledInitializerOrSubscript(ValueDecl *value) { - ParameterList *paramList = nullptr; - if (isa(value) || isa(value)) { - paramList = getParameterList(value); - } else { + if (!(isa(value) || isa(value))) { return false; } - for (auto param : *paramList) { + for (auto param : *value->getParameterList()) { if (!param->getArgumentName().empty()) return false; } diff --git a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp index 8926be20789e2..e577ef8a74983 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp @@ -966,7 +966,7 @@ void SwiftLangSupport::printMemberDeclDescription(const swift::ValueDecl *VD, OS << ')'; }; if (isa(VD) || isa(VD)) { - if (const auto ParamList = getParameterList(const_cast(VD))) { + if (const auto ParamList = VD->getParameterList()) { printParams(ParamList); } } else if (isa(VD)) { From 182bfc6c79aa66c4646990fd02f9a5f13ccfa31e Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 19 Mar 2025 18:48:08 +0000 Subject: [PATCH 2/7] [NFC] ClangImporter: Remove some redundant `hasParameterList` calls --- lib/ClangImporter/ClangDerivedConformances.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ClangImporter/ClangDerivedConformances.cpp b/lib/ClangImporter/ClangDerivedConformances.cpp index 7533d3e52aa92..028e34c3d7197 100644 --- a/lib/ClangImporter/ClangDerivedConformances.cpp +++ b/lib/ClangImporter/ClangDerivedConformances.cpp @@ -81,8 +81,6 @@ static FuncDecl *getInsertFunc(NominalTypeDecl *decl, FuncDecl *insert = nullptr; for (auto candidate : inserts) { if (auto candidateMethod = dyn_cast(candidate)) { - if (!candidateMethod->hasParameterList()) - continue; auto params = candidateMethod->getParameters(); if (params->size() != 1) continue; @@ -158,7 +156,7 @@ static ValueDecl *lookupOperator(NominalTypeDecl *decl, Identifier id, static ValueDecl *getEqualEqualOperator(NominalTypeDecl *decl) { auto isValid = [&](ValueDecl *equalEqualOp) -> bool { auto equalEqual = dyn_cast(equalEqualOp); - if (!equalEqual || !equalEqual->hasParameterList()) + if (!equalEqual) return false; auto params = equalEqual->getParameters(); if (params->size() != 2) @@ -187,7 +185,7 @@ static FuncDecl *getMinusOperator(NominalTypeDecl *decl) { auto isValid = [&](ValueDecl *minusOp) -> bool { auto minus = dyn_cast(minusOp); - if (!minus || !minus->hasParameterList()) + if (!minus) return false; auto params = minus->getParameters(); if (params->size() != 2) @@ -218,7 +216,7 @@ static FuncDecl *getMinusOperator(NominalTypeDecl *decl) { static FuncDecl *getPlusEqualOperator(NominalTypeDecl *decl, Type distanceTy) { auto isValid = [&](ValueDecl *plusEqualOp) -> bool { auto plusEqual = dyn_cast(plusEqualOp); - if (!plusEqual || !plusEqual->hasParameterList()) + if (!plusEqual) return false; auto params = plusEqual->getParameters(); if (params->size() != 2) From 487d3b4ae794173f57f9e15bf620eb32c4fd49c5 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 19 Mar 2025 18:55:47 +0000 Subject: [PATCH 3/7] [NFC] Simplify some code using `ValueDecl::getParameterList` --- lib/AST/NameLookup.cpp | 13 +++++++------ lib/Sema/TypeCheckConcurrency.cpp | 2 +- lib/Sema/TypeCheckGeneric.cpp | 6 ++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index ab18b712a4ad4..91b0503c58723 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -3710,13 +3710,14 @@ createOpaqueParameterGenericParams(GenericContext *genericContext, GenericParamL return { }; // Functions, initializers, and subscripts can contain opaque parameters. - ParameterList *params = nullptr; - if (auto func = dyn_cast(value)) - params = func->getParameters(); - else if (auto subscript = dyn_cast(value)) - params = subscript->getIndices(); - else + // FIXME: What's wrong with allowing them in macro decls? + if (isa(value)) { return { }; + } + auto *params = value->getParameterList(); + if (!params) { + return {}; + } // Look for parameters that have "some" types in them. unsigned index = parsedGenericParams ? parsedGenericParams->size() : 0; diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 7fa6d830c50b5..b754a46d65972 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -7262,7 +7262,7 @@ static AnyFunctionType *applyUnsafeConcurrencyToFunctionType( SmallVector newTypeParams; auto typeParams = fnType->getParams(); - auto paramDecls = func ? func->getParameters() : subscript->getIndices(); + auto paramDecls = decl->getParameterList(); assert(typeParams.size() == paramDecls->size()); bool knownUnsafeParams = func && hasKnownUnsafeSendableFunctionParams(func); bool stripConcurrency = diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp index 8ed5b44d73893..61fd5a426494a 100644 --- a/lib/Sema/TypeCheckGeneric.cpp +++ b/lib/Sema/TypeCheckGeneric.cpp @@ -714,10 +714,8 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator, /*unboundTyOpener*/ nullptr, /*placeholderHandler*/ nullptr, /*packElementOpener*/ nullptr); - auto params = func ? func->getParameters() - : subscr ? subscr->getIndices() - : macro->parameterList; - for (auto param : *params) { + + for (auto param : *VD->getParameterList()) { auto *typeRepr = param->getTypeRepr(); if (typeRepr == nullptr) continue; From 6512aa1f5a4b3e42bd963c7b24b278b0ae11cfc3 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Thu, 20 Mar 2025 00:24:46 +0000 Subject: [PATCH 4/7] AST: Introduce and use `ValueDecl::isAsync` --- include/swift/AST/Decl.h | 5 ++ lib/AST/Decl.cpp | 18 ++++++ lib/AST/DeclContext.cpp | 13 ++-- lib/Sema/TypeCheckAttr.cpp | 2 +- lib/Sema/TypeCheckConcurrency.cpp | 64 +++++++------------ lib/Sema/TypeCheckConcurrency.h | 3 - lib/Sema/TypeCheckDeclObjC.cpp | 17 +---- lib/Sema/TypeCheckProtocol.cpp | 7 +- test/attr/attr_availability_noasync.swift | 11 ++++ .../lib/SwiftLang/SwiftDocSupport.cpp | 7 +- 10 files changed, 68 insertions(+), 79 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 4909814471c04..bc9bdd9507a67 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3197,6 +3197,11 @@ class ValueDecl : public Decl { /// Is this declaration marked with 'dynamic'? bool isDynamic() const; + /// Returns whether accesses to this declaration are asynchronous. + /// If the declaration is neither `AbstractFunctionDecl` nor + /// `AbstractStorageDecl`, returns `false`. + bool isAsync() const; + private: bool isObjCDynamic() const { return isObjC() && isDynamic(); diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 1e2adae980c0c..c01066004f933 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -4180,6 +4180,24 @@ bool ValueDecl::isDynamic() const { getAttrs().hasAttribute()); } +bool ValueDecl::isAsync() const { + if (auto *function = dyn_cast(this)) { + return function->hasAsync(); + } + + // Async storage declarations must be get-only. Don't consider it async + // otherwise, even if it has an async getter. + if (auto *storage = dyn_cast(this)) { + if (storage->getAllAccessors().size() == 1) { + if (auto *getter = storage->getAccessor(AccessorKind::Get)) { + return getter->hasAsync(); + } + } + } + + return false; +} + bool ValueDecl::isObjCDynamicInGenericClass() const { if (!isObjCDynamic()) return false; diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp index c75a3e3a10dbc..d59dfd5beb028 100644 --- a/lib/AST/DeclContext.cpp +++ b/lib/AST/DeclContext.cpp @@ -1615,15 +1615,10 @@ bool DeclContext::isAsyncContext() const { return getParent()->isAsyncContext(); case DeclContextKind::AbstractClosureExpr: return cast(this)->isBodyAsync(); - case DeclContextKind::AbstractFunctionDecl: { - const AbstractFunctionDecl *function = cast(this); - return function->hasAsync(); - } - case DeclContextKind::SubscriptDecl: { - AccessorDecl *getter = - cast(this)->getAccessor(AccessorKind::Get); - return getter != nullptr && getter->hasAsync(); - } + case DeclContextKind::AbstractFunctionDecl: + return cast(this)->hasAsync(); + case DeclContextKind::SubscriptDecl: + return cast(this)->isAsync(); } llvm_unreachable("Unhandled DeclContextKind switch"); } diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index ea9329f913dbb..0d205eca4c73d 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -7607,7 +7607,7 @@ void AttributeChecker::visitKnownToBeLocalAttr(KnownToBeLocalAttr *attr) { void AttributeChecker::visitSendableAttr(SendableAttr *attr) { if ((isa(D) || isa(D)) && - !isAsyncDecl(cast(D))) { + !cast(D)->isAsync()) { auto value = cast(D); ActorIsolation isolation = getActorIsolation(value); if (isolation.isActorIsolated()) { diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index b754a46d65972..27ef35ff1d4cb 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -699,7 +699,7 @@ static bool isAsyncCall( if (lookup->isImplicitlyAsync()) return true; - return isAsyncDecl(lookup->getDecl()); + return lookup->getDecl().getDecl()->isAsync(); } /// Determine whether we should diagnose data races within the current context. @@ -2042,24 +2042,6 @@ static ActorIsolation getInnermostIsolatedContext( } } -/// Determine whether this declaration is always accessed asynchronously. -bool swift::isAsyncDecl(ConcreteDeclRef declRef) { - auto decl = declRef.getDecl(); - - // An async function is asynchronously accessed. - if (auto func = dyn_cast(decl)) - return func->hasAsync(); - - // A computed property or subscript that has an 'async' getter - // is asynchronously accessed. - if (auto storageDecl = dyn_cast(decl)) { - if (auto effectfulGetter = storageDecl->getEffectfulGetAccessor()) - return effectfulGetter->hasAsync(); - } - - return false; -} - AbstractFunctionDecl *swift::enclosingUnsafeInheritsExecutor( const DeclContext *dc) { for (; dc; dc = dc->getParent()) { @@ -4443,7 +4425,7 @@ namespace { // Make sure isolated conformances are formed in the right context. checkIsolatedConformancesInContext(declRef, loc, getDeclContext()); - auto decl = declRef.getDecl(); + auto *const decl = declRef.getDecl(); // If this declaration is a callee from the enclosing application, // it's already been checked via the call. @@ -4500,7 +4482,7 @@ namespace { // An escaping partial application of something that is part of // the actor's isolated state is never permitted. - if (partialApply && partialApply->isEscaping && !isAsyncDecl(declRef)) { + if (partialApply && partialApply->isEscaping && !decl->isAsync()) { ctx.Diags.diagnose(loc, diag::actor_isolated_partial_apply, decl); return true; } @@ -5612,9 +5594,9 @@ static OverrideIsolationResult validOverrideIsolation( // It's okay to enter the actor when the overridden declaration is // asynchronous (because it will do the switch) or is accessible from // anywhere. - if (isAsyncDecl(overridden) || - isAccessibleAcrossActors( - overridden, refResult.isolation, declContext)) { + if (overridden->isAsync() || + isAccessibleAcrossActors(overridden, refResult.isolation, + declContext)) { return OverrideIsolationResult::Sendable; } @@ -7713,13 +7695,15 @@ ActorReferenceResult ActorReferenceResult::forReference( std::optional knownContextIsolation, llvm::function_ref getClosureActorIsolation) { + auto *const decl = declRef.getDecl(); + // If not provided, compute the isolation of the declaration, adjusted // for references. ActorIsolation declIsolation = ActorIsolation::forUnspecified(); if (knownDeclIsolation) { declIsolation = *knownDeclIsolation; } else { - declIsolation = getActorIsolationForReference(declRef.getDecl(), fromDC); + declIsolation = getActorIsolationForReference(decl, fromDC); if (declIsolation.requiresSubstitution()) declIsolation = declIsolation.subst(declRef.getSubstitutions()); } @@ -7731,7 +7715,7 @@ ActorReferenceResult ActorReferenceResult::forReference( // FIXME: Actor constructors are modeled as isolated to the actor // so that Sendable checking is applied to their arguments, but the // call itself does not hop to another executor. - if (auto ctor = dyn_cast(declRef.getDecl())) { + if (auto ctor = dyn_cast(decl)) { if (auto nominal = ctor->getDeclContext()->getSelfNominalTypeDecl()) { if (nominal->isAnyActor()) options |= Flags::OnlyArgsCrossIsolation; @@ -7740,7 +7724,7 @@ ActorReferenceResult ActorReferenceResult::forReference( // If the entity we are referencing is not a value, we're in the same // concurrency domain. - if (isNonValueReference(declRef.getDecl())) + if (isNonValueReference(decl)) return forSameConcurrencyDomain(declIsolation, options); // Compute the isolation of the context, if not provided. @@ -7757,9 +7741,10 @@ ActorReferenceResult ActorReferenceResult::forReference( if (!declIsolation.isActorIsolated()) { // If the declaration is asynchronous and we are in an actor-isolated // context (of any kind), then we exit the actor to the nonisolated context. - if (isAsyncDecl(declRef) && contextIsolation.isActorIsolated() && - !declRef.getDecl()->getAttrs() - .hasAttribute()) + if (decl->isAsync() && contextIsolation.isActorIsolated() && + !declRef.getDecl() + ->getAttrs() + .hasAttribute()) return forExitsActorToNonisolated(contextIsolation, options); // Otherwise, we stay in the same concurrency domain, whether on an actor @@ -7788,11 +7773,9 @@ ActorReferenceResult ActorReferenceResult::forReference( if ((declIsolation.isActorIsolated() && contextIsolation.isGlobalActor()) || declIsolation.isGlobalActor()) { auto *init = dyn_cast(fromDC); - auto *decl = declRef.getDecl(); if (init && init->isDesignatedInit() && isStoredProperty(decl) && (!actorInstance || actorInstance->isSelf())) { - auto type = - fromDC->mapTypeIntoContext(declRef.getDecl()->getInterfaceType()); + auto type = fromDC->mapTypeIntoContext(decl->getInterfaceType()); if (!type->isSendableType()) { // Treat the decl isolation as 'preconcurrency' to downgrade violations // to warnings, because violating Sendable here is accepted by the @@ -7806,16 +7789,14 @@ ActorReferenceResult ActorReferenceResult::forReference( // If there is an instance and it is checked by flow isolation, treat it // as being in the same concurrency domain. if (actorInstance && - checkedByFlowIsolation( - fromDC, *actorInstance, declRef.getDecl(), declRefLoc, useKind)) + checkedByFlowIsolation(fromDC, *actorInstance, decl, declRefLoc, useKind)) return forSameConcurrencyDomain(declIsolation, options); // If we are delegating to another initializer, treat them as being in the // same concurrency domain. // FIXME: This has a lot of overlap with both the stored-property checks // below and the flow-isolation checks above. - if (actorInstance && actorInstance->isSelf() && - isa(declRef.getDecl()) && + if (actorInstance && actorInstance->isSelf() && isa(decl) && isa(fromDC)) return forSameConcurrencyDomain(declIsolation, options); @@ -7824,16 +7805,15 @@ ActorReferenceResult ActorReferenceResult::forReference( // global-actor-qualified type, then we have problems if the stored property // type is non-Sendable. Note that if we get here, the type must be Sendable. if (actorInstance && actorInstance->isSelf() && - isNonInheritedStorage(declRef.getDecl(), fromDC) && - declIsolation.isGlobalActor() && + isNonInheritedStorage(decl, fromDC) && declIsolation.isGlobalActor() && (isa(fromDC) || isa(fromDC))) return forSameConcurrencyDomain(declIsolation, options); // At this point, we are accessing the target from outside the actor. // First, check whether it is something that can be accessed directly, // without any kind of promotion. - if (isAccessibleAcrossActors( - declRef.getDecl(), declIsolation, fromDC, options, actorInstance)) + if (isAccessibleAcrossActors(decl, declIsolation, fromDC, options, + actorInstance)) return forEntersActor(declIsolation, options); // This is a cross-actor reference. @@ -7843,7 +7823,7 @@ ActorReferenceResult ActorReferenceResult::forReference( options |= Flags::Preconcurrency; // If the declaration isn't asynchronous, promote to async. - if (!isAsyncDecl(declRef)) + if (!decl->isAsync()) options |= Flags::AsyncPromotion; // If the declaration is isolated to a distributed actor and we are not diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h index 7ec9985f889ee..6e74ecbf1ff52 100644 --- a/lib/Sema/TypeCheckConcurrency.h +++ b/lib/Sema/TypeCheckConcurrency.h @@ -617,9 +617,6 @@ ProtocolConformance *deriveImplicitSendableConformance(Evaluator &evaluator, /// returns a pointer to the declaration. const AbstractFunctionDecl *isActorInitOrDeInitContext(const DeclContext *dc); -/// Determine whether this declaration is always accessed asynchronously. -bool isAsyncDecl(ConcreteDeclRef declRef); - /// Determine whether this declaration can throw errors. bool isThrowsDecl(ConcreteDeclRef declRef); diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp index 7d78aa911bd19..3bf4ff5d53f6b 100644 --- a/lib/Sema/TypeCheckDeclObjC.cpp +++ b/lib/Sema/TypeCheckDeclObjC.cpp @@ -3307,19 +3307,6 @@ class ObjCImplementationChecker { } private: - static bool hasAsync(ValueDecl *member) { - if (!member) - return false; - - if (auto func = dyn_cast(member)) - return func->hasAsync(); - - if (auto storage = dyn_cast(member)) - return hasAsync(storage->getEffectfulGetAccessor()); - - return false; - } - static ValueDecl *getAsyncAlternative(ValueDecl *req) { if (auto func = dyn_cast(req)) { auto asyncFunc = func->getAsyncAlternative(); @@ -3394,7 +3381,7 @@ class ObjCImplementationChecker { // Skip async versions of members. We'll match against the completion // handler versions, hopping over to `getAsyncAlternative()` if needed. - if (hasAsync(VD)) + if (VD->isAsync()) return; auto inserted = unmatchedRequirements.insert(VD); @@ -3859,7 +3846,7 @@ class ObjCImplementationChecker { ObjCSelector explicitObjCName) const { // If the candidate we're considering is async, see if the requirement has // an async alternate and try to match against that instead. - if (hasAsync(cand)) + if (cand->isAsync()) if (auto asyncAltReq = getAsyncAlternative(req)) return matchesImpl(asyncAltReq, cand, explicitObjCName); diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 50f96b6fdbfa9..2213d30b13f72 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -3363,7 +3363,7 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement, // Witnessing `async` requirement with an isolated synchronous // declaration is done via async witness thunk which requires // a hop to the expected concurrency domain. - if (isAsyncDecl(requirement) && !isAsyncDecl(witness)) + if (requirement->isAsync() && !witness->isAsync()) return refResult.isolation; // Otherwise, we're done. @@ -3410,8 +3410,7 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement, // FIXME: feels like ActorReferenceResult should be reporting this back to // us somehow. // To enter the actor, we always need the requirement to be `async`. - if (!sameConcurrencyDomain && - !isAsyncDecl(requirement) && + if (!sameConcurrencyDomain && !requirement->isAsync() && !isAccessibleAcrossActors(witness, refResult.isolation, DC)) missingOptions |= MissingFlags::RequirementAsync; @@ -3502,7 +3501,7 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement, return std::nullopt; if (refResult.isolation.isActorIsolated()) { - if (isAsyncDecl(requirement) && !isAsyncDecl(witness)) + if (requirement->isAsync() && !witness->isAsync()) return refResult.isolation; // Always treat `@preconcurrency` witnesses as isolated. diff --git a/test/attr/attr_availability_noasync.swift b/test/attr/attr_availability_noasync.swift index 88df62cbdc7dd..9fb5be3dee95c 100644 --- a/test/attr/attr_availability_noasync.swift +++ b/test/attr/attr_availability_noasync.swift @@ -54,6 +54,17 @@ func asyncFunc() async { func unavailableAsyncFunc() async { } +do { + struct S { + @available(*, noasync) + subscript(_: Int) -> Int { + get async {} + // expected-error@+1 {{'set' accessor is not allowed on property with 'get' accessor that is 'async' or 'throws'}} + set {} + } + } +} + @available(SwiftStdlib 5.5, *) protocol BadSyncable { // expected-error@+2{{asynchronous property 'isSyncd' must be available from asynchronous contexts}} diff --git a/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp b/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp index cda9216374062..d039135a71930 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp @@ -451,11 +451,8 @@ static bool initDocEntityInfo(const Decl *D, Info.IsUnavailable = D->isUnavailable(); Info.IsDeprecated = D->isDeprecated(); Info.IsOptional = D->getAttrs().hasAttribute(); - if (auto *AFD = dyn_cast(D)) { - Info.IsAsync = AFD->hasAsync(); - } else if (auto *Storage = dyn_cast(D)) { - if (auto *Getter = Storage->getAccessor(AccessorKind::Get)) - Info.IsAsync = Getter->hasAsync(); + if (auto *valueDecl = dyn_cast(D)) { + Info.IsAsync = valueDecl->isAsync(); } if (!IsRef) { From d73402af253019ffa7d1e47b0949896bfba70366 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Fri, 21 Mar 2025 12:49:03 +0000 Subject: [PATCH 5/7] AST: Restrict `@execution` to `func` and `init` declarations --- include/swift/AST/DeclAttr.def | 2 +- lib/AST/FeatureSet.cpp | 6 ----- test/ModuleInterface/execution_attr.swift | 32 ----------------------- test/attr/attr_execution.swift | 32 +++++++++-------------- 4 files changed, 13 insertions(+), 59 deletions(-) diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def index e0d31a18b8a91..60274846dce76 100644 --- a/include/swift/AST/DeclAttr.def +++ b/include/swift/AST/DeclAttr.def @@ -862,7 +862,7 @@ DECL_ATTR(abi, ABI, DECL_ATTR_FEATURE_REQUIREMENT(ABI, ABIAttribute) DECL_ATTR(execution, Execution, - OnAbstractFunction, + OnFunc | OnConstructor, ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove, 166) DECL_ATTR_FEATURE_REQUIREMENT(Execution, ExecutionAttribute) diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index eb49250ef48ee..2e02f230a0143 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -466,12 +466,6 @@ static bool usesFeatureBuiltinEmplaceTypedThrows(Decl *decl) { } static bool usesFeatureExecutionAttribute(Decl *decl) { - if (auto *ASD = dyn_cast(decl)) { - if (auto *getter = ASD->getAccessor(AccessorKind::Get)) - return usesFeatureExecutionAttribute(getter); - return false; - } - if (decl->getAttrs().hasAttribute()) return true; diff --git a/test/ModuleInterface/execution_attr.swift b/test/ModuleInterface/execution_attr.swift index 5bbd5d69b0b6b..cabb1f572b31d 100644 --- a/test/ModuleInterface/execution_attr.swift +++ b/test/ModuleInterface/execution_attr.swift @@ -30,37 +30,5 @@ public struct Test { // CHECK-NEXT: public func other(_: () async -> Swift.Void) // CHECK-NEXT: #endif public func other(_: @execution(caller) () async -> Void) {} - - // CHECK: #if compiler(>=5.3) && $ExecutionAttribute - // CHECK-NEXT: public var test: Swift.Int { - // CHECK-NEXT: @execution(caller) get async - // CHECK-NEXT: } - // CHECK-NEXT: #else - // CHECK-NEXT: public var test: Swift.Int { - // CHECK-NEXT: get async - // CHECK-NEXT: } - // CHECK-NEXT: #endif - public var test: Int { - @execution(caller) - get async { - 42 - } - } - - // CHECK: #if compiler(>=5.3) && $ExecutionAttribute - // CHECK-NEXT: public subscript(x: Swift.Int) -> Swift.Bool { - // CHECK-NEXT: @execution(caller) get async - // CHECK-NEXT: } - // CHECK-NEXT: #else - // CHECK-NEXT: public subscript(x: Swift.Int) -> Swift.Bool { - // CHECK-NEXT: get async - // CHECK-NEXT: } - // CHECK-NEXT: #endif - public subscript(x: Int) -> Bool { - @execution(caller) - get async { - false - } - } } diff --git a/test/attr/attr_execution.swift b/test/attr/attr_execution.swift index b7c4537288b80..0edfb66d62f08 100644 --- a/test/attr/attr_execution.swift +++ b/test/attr/attr_execution.swift @@ -20,6 +20,8 @@ struct Test { @execution(concurrent) init() {} // expected-error@-1 {{cannot use '@execution' on non-async initializer 'init()'}} + @execution(concurrent) init(async: Void) async {} + @execution(concurrent) func member() {} // expected-error@-1 {{cannot use '@execution' on non-async instance method 'member()'}} @@ -27,15 +29,23 @@ struct Test { // expected-error@+1 {{'@execution(caller)' attribute cannot be applied to this declaration}} @execution(caller) subscript(a: Int) -> Bool { - get { false } + @execution(concurrent) get { false } + // expected-error@-1 {{@execution(concurrent)' attribute cannot be applied to this declaration}} @execution(concurrent) set { } - // expected-error@-1 {{cannot use '@execution' on non-async setter for subscript 'subscript(_:)'}} + // expected-error@-1 {{@execution(concurrent)' attribute cannot be applied to this declaration}} } @execution(caller) var x: Int // expected-error@-1 {{'@execution(caller)' attribute cannot be applied to this declaration}} } +do { + class C { + @execution(caller) deinit {} + // expected-error@-1 {{'@execution(caller)' attribute cannot be applied to this declaration}} + } +} + do { @execution(caller) func local() async {} // Ok } @@ -93,21 +103,3 @@ _ = { @execution(concurrent) () -> Int in _ = { @execution(caller) (x: isolated (any Actor)?) in // expected-error@-1 {{cannot use '@execution' together with an isolated parameter}} } - -struct TestDifferentPositions { - @execution(caller) - init() async { - } - - var x: Int { - @execution(concurrent) - get async { - } - } - - subscript(x: Int) -> Bool { - @execution(concurrent) - get async { - } - } -} From 8ad2e02596139f188569b8f6f5d2622b504abf05 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 19 Mar 2025 17:07:06 +0000 Subject: [PATCH 6/7] Sema: Allow `@execution` on storage declarations --- include/swift/AST/DeclAttr.def | 2 +- lib/AST/FeatureSet.cpp | 10 +++-- lib/Sema/TypeCheckAttr.cpp | 19 +++++--- ...e_decl_attribute_feature_requirement.swift | 12 ++++- test/ModuleInterface/execution_attr.swift | 32 ++++++++++++++ test/attr/attr_execution.swift | 44 +++++++++++++++++-- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def index 60274846dce76..2b3fc996fbd90 100644 --- a/include/swift/AST/DeclAttr.def +++ b/include/swift/AST/DeclAttr.def @@ -862,7 +862,7 @@ DECL_ATTR(abi, ABI, DECL_ATTR_FEATURE_REQUIREMENT(ABI, ABIAttribute) DECL_ATTR(execution, Execution, - OnFunc | OnConstructor, + OnFunc | OnConstructor | OnSubscript | OnVar, ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove, 166) DECL_ATTR_FEATURE_REQUIREMENT(Execution, ExecutionAttribute) diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index 2e02f230a0143..be481c941aca3 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -466,13 +466,13 @@ static bool usesFeatureBuiltinEmplaceTypedThrows(Decl *decl) { } static bool usesFeatureExecutionAttribute(Decl *decl) { + if (!DeclAttribute::canAttributeAppearOnDecl(DeclAttrKind::Execution, decl)) { + return false; + } + if (decl->getAttrs().hasAttribute()) return true; - auto VD = dyn_cast(decl); - if (!VD) - return false; - auto hasExecutionAttr = [](TypeRepr *R) { if (!R) return false; @@ -490,6 +490,8 @@ static bool usesFeatureExecutionAttribute(Decl *decl) { }); }; + auto *VD = cast(decl); + // Check if any parameters that have `@execution` attribute. if (auto *PL = VD->getParameterList()) { for (auto *P : *PL) { diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 0d205eca4c73d..2dfe8c4e7e9d0 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -257,16 +257,20 @@ class AttributeChecker : public AttributeVisitor { public: void visitExecutionAttr(ExecutionAttr *attr) { - auto *F = dyn_cast(D); - if (!F) + auto *const decl = cast(D); + + auto *const storage = dyn_cast(decl); + if (storage && storage->hasStorage()) { + diagnoseAndRemoveAttr(attr, diag::attr_not_on_stored_properties, attr); return; + } - if (!F->hasAsync()) { - diagnoseAndRemoveAttr(attr, diag::attr_execution_only_on_async, F); + if (!decl->isAsync()) { + diagnoseAndRemoveAttr(attr, diag::attr_execution_only_on_async, decl); return; } - auto parameters = F->getParameters(); + auto *parameters = decl->getParameterList(); if (!parameters) return; @@ -278,7 +282,8 @@ class AttributeChecker : public AttributeVisitor { // isolated parameters affect isolation of the function itself if (isa(repr)) { diagnoseAndRemoveAttr( - attr, diag::attr_execution_incompatible_isolated_parameter, F, P); + attr, diag::attr_execution_incompatible_isolated_parameter, decl, + P); return; } @@ -287,7 +292,7 @@ class AttributeChecker : public AttributeVisitor { diagnoseAndRemoveAttr( attr, diag::attr_execution_incompatible_dynamically_isolated_parameter, - F, P); + decl, P); return; } } diff --git a/test/IDE/complete_decl_attribute_feature_requirement.swift b/test/IDE/complete_decl_attribute_feature_requirement.swift index 45dc028559deb..8fabadd74d3f1 100644 --- a/test/IDE/complete_decl_attribute_feature_requirement.swift +++ b/test/IDE/complete_decl_attribute_feature_requirement.swift @@ -54,7 +54,7 @@ @#^ON_GLOBALVAR^# var globalVar // ON_GLOBALVAR: Begin completions // ON_GLOBALVAR_ENABLED-DAG: Keyword/None: abi[#Var Attribute#]; name=abi -// ON_GLOBALVAR_ENABLED-NOT: Keyword/None: execution[#{{.*}} Attribute#]; name=execution +// ON_GLOBALVAR_ENABLED-DAG: Keyword/None: execution[#Var Attribute#]; name=execution // ON_GLOBALVAR_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi // ON_GLOBALVAR_DISABLED-NOT: Keyword/None: execution[#{{.*}} Attribute#]; name=execution // ON_GLOBALVAR: End completions @@ -71,11 +71,19 @@ struct _S { @#^ON_PROPERTY^# var foo // ON_PROPERTY: Begin completions // ON_PROPERTY_ENABLED-DAG: Keyword/None: abi[#Var Attribute#]; name=abi -// ON_PROPERTY_ENABLED-NOT: Keyword/None: execution[#{{.*}} Attribute#]; name=execution +// ON_PROPERTY_ENABLED-DAG: Keyword/None: execution[#Var Attribute#]; name=execution // ON_PROPERTY_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi // ON_PROPERTY_DISABLED-NOT: Keyword/None: execution[#{{.*}} Attribute#]; name=execution // ON_PROPERTY: End completions + @#^ON_SUBSCR^# subscript +// ON_SUBSCR: Begin completions +// ON_SUBSCR_ENABLED-DAG: Keyword/None: abi[#Declaration Attribute#]; name=abi +// ON_SUBSCR_ENABLED-DAG: Keyword/None: execution[#Declaration Attribute#]; name=execution +// ON_SUBSCR_DISABLED-NOT: Keyword/None: abi[#{{.*}} Attribute#]; name=abi +// ON_SUBSCR_DISABLED-NOT: Keyword/None: execution[#{{.*}} Attribute#]; name=execution +// ON_SUBSCR: End completions + @#^ON_METHOD^# private func foo() // ON_METHOD: Begin completions diff --git a/test/ModuleInterface/execution_attr.swift b/test/ModuleInterface/execution_attr.swift index cabb1f572b31d..d3b2e4ada0a76 100644 --- a/test/ModuleInterface/execution_attr.swift +++ b/test/ModuleInterface/execution_attr.swift @@ -30,5 +30,37 @@ public struct Test { // CHECK-NEXT: public func other(_: () async -> Swift.Void) // CHECK-NEXT: #endif public func other(_: @execution(caller) () async -> Void) {} + + // CHECK: #if compiler(>=5.3) && $ExecutionAttribute + // CHECK-NEXT: @execution(caller) public var testOnVar: Swift.Int { + // CHECK-NEXT: get async + // CHECK-NEXT: } + // CHECK-NEXT: #else + // CHECK-NEXT: public var testOnVar: Swift.Int { + // CHECK-NEXT: get async + // CHECK-NEXT: } + // CHECK-NEXT: #endif + @execution(caller) + public var testOnVar: Int { + get async { + 42 + } + } + + // CHECK: #if compiler(>=5.3) && $ExecutionAttribute + // CHECK-NEXT: @execution(caller) public subscript(onSubscript _: Swift.Int) -> Swift.Bool { + // CHECK-NEXT: get async + // CHECK-NEXT: } + // CHECK-NEXT: #else + // CHECK-NEXT: public subscript(onSubscript _: Swift.Int) -> Swift.Bool { + // CHECK-NEXT: get async + // CHECK-NEXT: } + // CHECK-NEXT: #endif + @execution(caller) + public subscript(onSubscript _: Int) -> Bool { + get async { + false + } + } } diff --git a/test/attr/attr_execution.swift b/test/attr/attr_execution.swift index 0edfb66d62f08..68e7531d197ab 100644 --- a/test/attr/attr_execution.swift +++ b/test/attr/attr_execution.swift @@ -8,6 +8,14 @@ @execution(concurrent) @execution(caller) func mutipleAttrs() async {} // expected-error@-1 {{duplicate attribute}} expected-note@-1 {{attribute already specified here}} +do { + @execution(caller) struct S {} + // expected-error@-1 {{'@execution(caller)' attribute cannot be applied to this declaration}} + + func f(@execution(caller) param: Int) {} + // expected-error@-1 {{'@execution(caller)' attribute cannot be applied to this declaration}} +} + @execution(concurrent) func nonAsync1() {} // expected-error@-1 {{cannot use '@execution' on non-async global function 'nonAsync1()'}} @@ -27,16 +35,29 @@ struct Test { @execution(concurrent) func member() async {} // Ok - // expected-error@+1 {{'@execution(caller)' attribute cannot be applied to this declaration}} - @execution(caller) subscript(a: Int) -> Bool { + @execution(concurrent) var syncP: Int { + // expected-error@-1 {{cannot use '@execution' on non-async property 'syncP'}} + get {} + } + @execution(concurrent) var asyncP: Int { + get async {} + } + + // expected-error@+1 {{cannot use '@execution' on non-async subscript 'subscript(sync:)'}} + @execution(caller) subscript(sync _: Int) -> Bool { @execution(concurrent) get { false } // expected-error@-1 {{@execution(concurrent)' attribute cannot be applied to this declaration}} @execution(concurrent) set { } // expected-error@-1 {{@execution(concurrent)' attribute cannot be applied to this declaration}} } + @execution(caller) subscript(async _: Int) -> Bool { + get async {} + } - @execution(caller) var x: Int - // expected-error@-1 {{'@execution(caller)' attribute cannot be applied to this declaration}} + @execution(caller) var storedVar: Int + // expected-error@-1 {{'@execution(caller)' must not be used on stored properties}} + @execution(caller) let storedLet: Int + // expected-error@-1 {{'@execution(caller)' must not be used on stored properties}} } do { @@ -48,6 +69,13 @@ do { do { @execution(caller) func local() async {} // Ok + + protocol P { + @execution(caller) var syncP: Int { get } + // expected-error@-1 {{cannot use '@execution' on non-async property 'syncP'}} + + @execution(caller) var asyncP: Int { get async } + } } struct TestAttributeCollisions { @@ -55,9 +83,17 @@ struct TestAttributeCollisions { @execution(concurrent) func test(arg: isolated MainActor) async {} // expected-error@-1 {{cannot use '@execution' on instance method 'test(arg:)' because it has an isolated parameter: 'arg'}} + @execution(concurrent) subscript(test arg: isolated MainActor) -> Int { + // expected-error@-1 {{cannot use '@execution' on subscript 'subscript(test:)' because it has an isolated parameter: 'arg'}} + get async {} + } @execution(concurrent) func testIsolationAny(arg: @isolated(any) () -> Void) async {} // expected-error@-1 {{cannot use '@execution' on instance method 'testIsolationAny(arg:)' because it has a dynamically isolated parameter: 'arg'}} + @execution(concurrent) subscript(testIsolationAny arg: @isolated(any) () -> Void) -> Int { + // expected-error@-1 {{cannot use '@execution' on subscript 'subscript(testIsolationAny:)' because it has a dynamically isolated parameter: 'arg'}} + get async {} + } @MainActor @execution(concurrent) func testGlobalActor() async {} // expected-warning @-1 {{instance method 'testGlobalActor()' has multiple actor-isolation attributes ('MainActor' and 'execution(concurrent)')}} From a56695bc6dd4d3db87a7fd4cbfc6ff2612989d1f Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Thu, 20 Mar 2025 13:03:00 +0000 Subject: [PATCH 7/7] Sema: Extend adoption mode for `AsyncCallerExecution` to storage declarations --- lib/Sema/AsyncCallerExecutionMigration.cpp | 44 ++++++++++----- .../attr_execution/adoption_mode.swift | 54 +++++++++++++++++-- 2 files changed, 83 insertions(+), 15 deletions(-) diff --git a/lib/Sema/AsyncCallerExecutionMigration.cpp b/lib/Sema/AsyncCallerExecutionMigration.cpp index 84480a20d221d..79ecf9f2b1c89 100644 --- a/lib/Sema/AsyncCallerExecutionMigration.cpp +++ b/lib/Sema/AsyncCallerExecutionMigration.cpp @@ -62,21 +62,29 @@ void AsyncCallerExecutionMigrationTarget::diagnose() const { ASSERT(node); ASSERT(ctx.LangOpts.getFeatureState(feature).isEnabledForAdoption()); - AbstractFunctionDecl *functionDecl = nullptr; + ValueDecl *decl = nullptr; ClosureExpr *closure = nullptr; FunctionTypeRepr *functionRepr = nullptr; - if (auto *decl = node.dyn_cast()) { + if ((decl = node.dyn_cast())) { // Diagnose only explicit nodes. if (decl->isImplicit()) { return; } - // Diagnose only functions. - functionDecl = dyn_cast(decl); - if (!functionDecl) { + // If the attribute cannot appear on this kind of declaration, we can't + // diagnose it. + if (!DeclAttribute::canAttributeAppearOnDecl(DeclAttrKind::Execution, + decl)) { return; } + + // For storage, make sure we have an explicit getter to diagnose. + if (auto *storageDecl = dyn_cast(decl)) { + if (!storageDecl->getParsedAccessor(AccessorKind::Get)) { + return; + } + } } else if (auto *anyClosure = node.dyn_cast()) { // Diagnose only explicit nodes. if (anyClosure->isImplicit()) { @@ -107,8 +115,8 @@ void AsyncCallerExecutionMigrationTarget::diagnose() const { // If the intended behavior is specified explicitly, don't diagnose. { const DeclAttributes *attrs = nullptr; - if (functionDecl) { - attrs = &functionDecl->getAttrs(); + if (decl) { + attrs = &decl->getAttrs(); } else if (closure) { attrs = &closure->getAttrs(); } @@ -121,8 +129,8 @@ void AsyncCallerExecutionMigrationTarget::diagnose() const { // The execution behavior changes only for async functions. { bool isAsync = false; - if (functionDecl) { - isAsync = functionDecl->hasAsync(); + if (decl) { + isAsync = decl->isAsync(); } else if (closure) { isAsync = closure->isBodyAsync(); } else { @@ -137,14 +145,26 @@ void AsyncCallerExecutionMigrationTarget::diagnose() const { const ExecutionAttr attr(ExecutionKind::Concurrent, /*implicit=*/true); const auto featureName = getFeatureName(feature); - if (functionDecl) { + if (decl) { + // Diagnose the function, but slap the attribute on the storage declaration + // instead if the function is an accessor. + auto *functionDecl = dyn_cast(decl); + if (!functionDecl) { + auto *storageDecl = cast(decl); + + // This whole logic assumes that an 'async' storage declaration only has + // a getter. Yell for an update if this ever changes. + ASSERT(!storageDecl->getAccessor(AccessorKind::Set)); + + functionDecl = storageDecl->getParsedAccessor(AccessorKind::Get); + } + ctx.Diags .diagnose(functionDecl->getLoc(), diag::attr_execution_nonisolated_behavior_will_change_decl, featureName, functionDecl, &attr) .fixItInsertAttribute( - functionDecl->getAttributeInsertionLoc(/*forModifier=*/false), - &attr); + decl->getAttributeInsertionLoc(/*forModifier=*/false), &attr); } else if (closure) { ctx.Diags .diagnose(closure->getLoc(), diff --git a/test/Concurrency/attr_execution/adoption_mode.swift b/test/Concurrency/attr_execution/adoption_mode.swift index 8bb9f60ccf215..9dba405126319 100644 --- a/test/Concurrency/attr_execution/adoption_mode.swift +++ b/test/Concurrency/attr_execution/adoption_mode.swift @@ -1,4 +1,5 @@ -// RUN: %target-typecheck-verify-swift -target %target-swift-5.1-abi-triple -enable-experimental-feature ExecutionAttribute -enable-experimental-feature AsyncCallerExecution:adoption +// RUN: %target-typecheck-verify-swift -target %target-swift-5.1-abi-triple -swift-version 5 -enable-experimental-feature ExecutionAttribute -enable-experimental-feature AsyncCallerExecution:adoption +// RUN: %target-typecheck-verify-swift -target %target-swift-5.1-abi-triple -swift-version 6 -enable-experimental-feature ExecutionAttribute -enable-experimental-feature AsyncCallerExecution:adoption // REQUIRES: swift_feature_ExecutionAttribute // REQUIRES: swift_feature_AsyncCallerExecution @@ -19,8 +20,8 @@ do { isolation: isolated (any Actor)? = #isolation ) async {} - // expected-warning@+1:8 {{feature 'AsyncCallerExecution' will cause nonisolated async local function 'asyncF' to run on the caller's actor; use @execution(concurrent) to preserve behavior}}{{3-3=@execution(concurrent) }}{{none}} - func asyncF() async {} + // expected-warning@+1:20 {{feature 'AsyncCallerExecution' will cause nonisolated async local function 'asyncF' to run on the caller's actor; use @execution(concurrent) to preserve behavior}}{{3-3=@execution(concurrent) }}{{none}} + nonisolated func asyncF() async {} struct S { init(sync: ()) {} @@ -36,6 +37,53 @@ do { nonisolated public func asyncF() async {} } + + protocol P { + // FIXME: Not diagnosed + func asyncF() async + } +} + +// MARK: Storage +do { + struct S { + var storedVar: Int + let storedLet: Int + + var syncS: Int { get {} set {} } + subscript(syncS _: Int) -> Int { get {} } + + @execution(concurrent) var executionAsyncS: Int { get async {} } + @execution(concurrent) subscript(executionAsyncS _: Int) -> Int { get async {} } + + @MainActor var mainActorAsyncS: Int { get async {} } + @MainActor subscript(mainActorAsyncS _: Int) -> Int { get async {} } + + // expected-warning@+2:7 {{feature 'AsyncCallerExecution' will cause nonisolated async getter for property 'asyncS' to run on the caller's actor; use @execution(concurrent) to preserve behavior}}{{-1:5-5=@execution(concurrent) }}{{none}} + var asyncS: Int { + get async {} + } + // expected-warning@+2:7 {{feature 'AsyncCallerExecution' will cause nonisolated async getter for subscript 'subscript' to run on the caller's actor; use @execution(concurrent) to preserve behavior}}{{-1:5-5=@execution(concurrent) }}{{none}} + subscript(asyncS _: Int) -> Int { + get async throws {} + } + } + + protocol P { + var syncS: Int { get } + subscript(syncS _: Int) -> Int { get } + + @execution(concurrent) var executionAsyncS: Int { get async } + @execution(concurrent) subscript(executionAsyncS _: Int) -> Int { get async } + + @MainActor var mainActorAsyncS: Int { get async } + @MainActor subscript(mainActorAsyncS _: Int) -> Int { get async } + + // expected-warning@+1:23 {{feature 'AsyncCallerExecution' will cause nonisolated async getter for property 'asyncS' to run on the caller's actor; use @execution(concurrent) to preserve behavior}}{{5-5=@execution(concurrent) }}{{none}} + var asyncS: Int { get async } + // FIXME: Not diagnosed + subscript(asyncS _: Int) -> Int { get async } + } } // MARK: Parameters