Skip to content

Sema: Extend adoption mode for AsyncCallerExecution to storage declarations #80159

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 26, 2025
Merged

Sema: Extend adoption mode for AsyncCallerExecution to storage declarations #80159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3d3b1ca
[NFC] AST: Turn `getParameterList` into a method on `ValueDecl`
AnthonyLatsis Mar 19, 2025
182bfc6
[NFC] ClangImporter: Remove some redundant `hasParameterList` calls
AnthonyLatsis Mar 19, 2025
487d3b4
[NFC] Simplify some code using `ValueDecl::getParameterList`
AnthonyLatsis Mar 19, 2025
6512aa1
AST: Introduce and use `ValueDecl::isAsync`
AnthonyLatsis Mar 20, 2025
d73402a
AST: Restrict `@execution` to `func` and `init` declarations
AnthonyLatsis Mar 21, 2025
8ad2e02
Sema: Allow `@execution` on storage declarations
AnthonyLatsis Mar 19, 2025
a56695b
Sema: Extend adoption mode for `AsyncCallerExecution` to storage decl…
AnthonyLatsis Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -3320,6 +3325,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;

Expand Down Expand Up @@ -9720,14 +9733,6 @@ inline bool ValueDecl::hasCurriedSelf() const {
return false;
}

inline bool ValueDecl::hasParameterList() const {
if (auto *eed = dyn_cast<EnumElementDecl>(this))
return eed->hasAssociatedValues();
if (auto *macro = dyn_cast<MacroDecl>(this))
return macro->parameterList != nullptr;
return isa<AbstractFunctionDecl>(this) || isa<SubscriptDecl>(this);
}

inline unsigned ValueDecl::getNumCurryLevels() const {
unsigned curryLevels = 0;
if (hasParameterList())
Expand Down Expand Up @@ -9815,10 +9820,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);
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/DeclAttr.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ DECL_ATTR(abi, ABI,
DECL_ATTR_FEATURE_REQUIREMENT(ABI, ABIAttribute)

DECL_ATTR(execution, Execution,
OnAbstractFunction,
OnFunc | OnConstructor | OnSubscript | OnVar,
ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
166)
DECL_ATTR_FEATURE_REQUIREMENT(Execution, ExecutionAttribute)
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTMangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ static unsigned getUnnamedParamIndex(const ParamDecl *D) {
if (isa<AbstractClosureExpr>(DC)) {
ParamList = cast<AbstractClosureExpr>(DC)->getParameters();
} else {
ParamList = getParameterList(cast<ValueDecl>(DC->getAsDecl()));
ParamList = cast<ValueDecl>(DC->getAsDecl())->getParameterList();
}

unsigned UnnamedIndex = 0;
Expand Down
61 changes: 42 additions & 19 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1316,8 +1316,7 @@ bool Decl::hasUnderscoredNaming() const {
// underscore, it's a private function or subscript.
if (isa<AbstractFunctionDecl>(D) || isa<SubscriptDecl>(D)) {
const auto VD = cast<ValueDecl>(D);
if (getParameterList(const_cast<ValueDecl *>(VD))
->hasInternalParameter("_")) {
if (VD->getParameterList()->hasInternalParameter("_")) {
return true;
}
}
Expand Down Expand Up @@ -4096,6 +4095,26 @@ ValueDecl::getCachedOpaqueResultTypeDecl() const {
.getCachedResult();
}

ParameterList *ValueDecl::getParameterList() {
if (auto *function = dyn_cast<AbstractFunctionDecl>(this)) {
return function->getParameters();
} else if (auto *enumElement = dyn_cast<EnumElementDecl>(this)) {
return enumElement->getParameterList();
} else if (auto *subscript = dyn_cast<SubscriptDecl>(this)) {
return subscript->getIndices();
} else if (auto *macro = dyn_cast<MacroDecl>(this)) {
return macro->parameterList;
}

return nullptr;
}

const ParameterList *ValueDecl::getParameterList() const {
return const_cast<ValueDecl *>(this)->getParameterList();
}

bool ValueDecl::hasParameterList() const { return (bool)getParameterList(); }

bool ValueDecl::isObjC() const {
ASTContext &ctx = getASTContext();
return evaluateOrDefault(ctx.evaluator,
Expand Down Expand Up @@ -4161,6 +4180,24 @@ bool ValueDecl::isDynamic() const {
getAttrs().hasAttribute<DynamicAttr>());
}

bool ValueDecl::isAsync() const {
if (auto *function = dyn_cast<AbstractFunctionDecl>(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<AbstractStorageDecl>(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;
Expand Down Expand Up @@ -9578,24 +9615,10 @@ DeclName AbstractFunctionDecl::getEffectiveFullName() const {
return DeclName();
}

ParameterList *swift::getParameterList(ValueDecl *source) {
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(source)) {
return AFD->getParameters();
} else if (auto *EED = dyn_cast<EnumElementDecl>(source)) {
return EED->getParameterList();
} else if (auto *SD = dyn_cast<SubscriptDecl>(source)) {
return SD->getIndices();
} else if (auto *MD = dyn_cast<MacroDecl>(source)) {
return MD->parameterList;
}

return nullptr;
}

ParameterList *swift::getParameterList(DeclContext *source) {
if (auto *D = source->getAsDecl()) {
if (auto *VD = dyn_cast<ValueDecl>(D)) {
return getParameterList(VD);
return VD->getParameterList();
}
} else if (auto *CE = dyn_cast<AbstractClosureExpr>(source)) {
return CE->getParameters();
Expand All @@ -9607,7 +9630,7 @@ ParameterList *swift::getParameterList(DeclContext *source) {
const ParamDecl *swift::getParameterAt(ConcreteDeclRef declRef,
unsigned index) {
auto *source = declRef.getDecl();
if (auto *params = getParameterList(const_cast<ValueDecl *>(source))) {
if (auto *params = source->getParameterList()) {
unsigned origIndex = params->getOrigParamIndex(declRef.getSubstitutions(),
index);
return params->get(origIndex);
Expand All @@ -9617,7 +9640,7 @@ const ParamDecl *swift::getParameterAt(ConcreteDeclRef declRef,

const ParamDecl *swift::getParameterAt(const ValueDecl *source,
unsigned index) {
if (auto *params = getParameterList(const_cast<ValueDecl *>(source))) {
if (auto *params = source->getParameterList()) {
return index < params->size() ? params->get(index) : nullptr;
}
return nullptr;
Expand Down
15 changes: 5 additions & 10 deletions lib/AST/DeclContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ swift::FragileFunctionKindRequest::evaluate(Evaluator &evaluator,
dc = dc->getParent();

auto *VD = cast<ValueDecl>(dc->getAsDecl());
assert(VD->hasParameterList());
ASSERT(VD->hasParameterList());

if (VD->getDeclContext()->isLocalContext()) {
auto kind = VD->getDeclContext()->getFragileFunctionKind();
Expand Down Expand Up @@ -1615,15 +1615,10 @@ bool DeclContext::isAsyncContext() const {
return getParent()->isAsyncContext();
case DeclContextKind::AbstractClosureExpr:
return cast<AbstractClosureExpr>(this)->isBodyAsync();
case DeclContextKind::AbstractFunctionDecl: {
const AbstractFunctionDecl *function = cast<AbstractFunctionDecl>(this);
return function->hasAsync();
}
case DeclContextKind::SubscriptDecl: {
AccessorDecl *getter =
cast<SubscriptDecl>(this)->getAccessor(AccessorKind::Get);
return getter != nullptr && getter->hasAsync();
}
case DeclContextKind::AbstractFunctionDecl:
return cast<AbstractFunctionDecl>(this)->hasAsync();
case DeclContextKind::SubscriptDecl:
return cast<SubscriptDecl>(this)->isAsync();
Copy link
Contributor

@xedin xedin Mar 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the behavior change here would have any effect on source compatibility because we won't consider something as async which we previously did...

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only case where the outcome would change here is a failure path of a storage decl with both an asynchronous getter and a setter, so I do not expect any of the new isAsync calls to break source.

Copy link
Contributor

@xedin xedin Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an existing API which changed behavior now with use of isAsync(), I wonder if we can find a test-case that show that...

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Mar 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is one:

struct S {
  @available(*, noasync)
  subscript(_: Int) -> Int {
    get async {}
    set {}
  }
}

Currently both of the following errors are produced. With this change, only the first one is.

error: 'set' accessor is not allowed on property with 'get' accessor that is 'async' or 'throws'
error: asynchronous subscript 'subscript(_:)' must be available from asynchronous contexts

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be the only case. I am pretty sure we do not call isAsyncContext on anything other than functions, files, and TopLevelCodeDecl anywhere else.

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this test.

}
llvm_unreachable("Unhandled DeclContextKind switch");
}
Expand Down
12 changes: 4 additions & 8 deletions lib/AST/FeatureSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,19 +466,13 @@ static bool usesFeatureBuiltinEmplaceTypedThrows(Decl *decl) {
}

static bool usesFeatureExecutionAttribute(Decl *decl) {
if (auto *ASD = dyn_cast<AbstractStorageDecl>(decl)) {
if (auto *getter = ASD->getAccessor(AccessorKind::Get))
return usesFeatureExecutionAttribute(getter);
if (!DeclAttribute::canAttributeAppearOnDecl(DeclAttrKind::Execution, decl)) {
return false;
}

if (decl->getAttrs().hasAttribute<ExecutionAttr>())
return true;

auto VD = dyn_cast<ValueDecl>(decl);
if (!VD)
return false;

auto hasExecutionAttr = [](TypeRepr *R) {
if (!R)
return false;
Expand All @@ -496,8 +490,10 @@ static bool usesFeatureExecutionAttribute(Decl *decl) {
});
};

auto *VD = cast<ValueDecl>(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;
Expand Down
13 changes: 7 additions & 6 deletions lib/AST/NameLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<AbstractFunctionDecl>(value))
params = func->getParameters();
else if (auto subscript = dyn_cast<SubscriptDecl>(value))
params = subscript->getIndices();
else
// FIXME: What's wrong with allowing them in macro decls?
if (isa<MacroDecl>(value)) {
return { };
}
auto *params = value->getParameterList();
if (!params) {
return {};
}

// Look for parameters that have "some" types in them.
unsigned index = parsedGenericParams ? parsedGenericParams->size() : 0;
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/RawComment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<AbstractFunctionDecl>(D) || isa<SubscriptDecl>(D)) {
auto *params = getParameterList(cast<ValueDecl>(const_cast<Decl *>(D)));
auto *params = cast<ValueDecl>(D)->getParameterList();
if (params->hasInternalParameter(Prefix))
return true;
}
Expand Down
4 changes: 1 addition & 3 deletions lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1336,13 +1336,11 @@ ParameterListInfo::ParameterListInfo(
return;

// Find the corresponding parameter list.
const ParameterList *paramList =
getParameterList(const_cast<ValueDecl *>(paramOwner));
auto *paramList = paramOwner->getParameterList();

// No parameter list means no default arguments - hand back the zeroed
// bitvector.
if (!paramList) {
assert(!paramOwner->hasParameterList());
return;
}

Expand Down
8 changes: 3 additions & 5 deletions lib/ClangImporter/ClangDerivedConformances.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ static FuncDecl *getInsertFunc(NominalTypeDecl *decl,
FuncDecl *insert = nullptr;
for (auto candidate : inserts) {
if (auto candidateMethod = dyn_cast<FuncDecl>(candidate)) {
if (!candidateMethod->hasParameterList())
continue;
auto params = candidateMethod->getParameters();
if (params->size() != 1)
continue;
Expand Down Expand Up @@ -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<FuncDecl>(equalEqualOp);
if (!equalEqual || !equalEqual->hasParameterList())
if (!equalEqual)
return false;
auto params = equalEqual->getParameters();
if (params->size() != 2)
Expand Down Expand Up @@ -187,7 +185,7 @@ static FuncDecl *getMinusOperator(NominalTypeDecl *decl) {

auto isValid = [&](ValueDecl *minusOp) -> bool {
auto minus = dyn_cast<FuncDecl>(minusOp);
if (!minus || !minus->hasParameterList())
if (!minus)
return false;
auto params = minus->getParameters();
if (params->size() != 2)
Expand Down Expand Up @@ -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<FuncDecl>(plusEqualOp);
if (!plusEqual || !plusEqual->hasParameterList())
if (!plusEqual)
return false;
auto params = plusEqual->getParameters();
if (params->size() != 2)
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/Formatting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ class RangeWalker: protected ASTWalker {
if (!handleBraces(cast<SubscriptDecl>(D)->getBracesRange(), ContextLoc))
return Action::Stop();
}
auto *PL = getParameterList(cast<ValueDecl>(D));
auto *PL = cast<ValueDecl>(D)->getParameterList();
if (!handleParens(PL->getLParenLoc(), PL->getRParenLoc(), ContextLoc))
return Action::Stop();
} else if (auto *PGD = dyn_cast<PrecedenceGroupDecl>(D)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/SourceEntityWalker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ ASTWalker::PreWalkAction SemaAnnotator::walkToDeclPreProper(Decl *D) {
};

if (isa<AbstractFunctionDecl>(VD) || isa<SubscriptDecl>(VD)) {
auto ParamList = getParameterList(VD);
auto ParamList = VD->getParameterList();
if (!ReportParamList(ParamList))
return Action::Stop();
}
Expand Down
Loading