From d81ffe854f2c0e1a3efcc51fa659eb57356083da Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Thu, 20 Mar 2025 16:23:17 -0700 Subject: [PATCH 1/8] Typecheck ABI-only VarDecls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decl checker was effectively not being run on these because we weren’t typechecking the PBD and typechecking the VarDecl itself is basically a no-op. --- lib/AST/NameLookup.cpp | 7 +++++++ lib/Sema/TypeCheckAttrABI.cpp | 15 +++++++++++---- lib/Sema/TypeCheckStorage.cpp | 4 ++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index f1eb3e9967f5f..3ee047c9c434d 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -2317,6 +2317,13 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method, if (!ObjCMethodLookup && !createObjCMethodLookup()) return; + // Only record API decls. + Decl *abiRoleDecl = method; + if (auto accessor = dyn_cast(method)) + abiRoleDecl = accessor->getStorage(); + if (!ABIRoleInfo(abiRoleDecl).providesAPI()) + return; + // Record the method. bool isInstanceMethod = method->isObjCInstanceMethod(); auto &vec = (*ObjCMethodLookup)[{selector, isInstanceMethod}].Methods; diff --git a/lib/Sema/TypeCheckAttrABI.cpp b/lib/Sema/TypeCheckAttrABI.cpp index 1df97b9d35347..d4e9aefb037b1 100644 --- a/lib/Sema/TypeCheckAttrABI.cpp +++ b/lib/Sema/TypeCheckAttrABI.cpp @@ -1152,6 +1152,7 @@ void checkABIAttrPBD(PatternBindingDecl *APBD, VarDecl *VD) { } // Check that each pattern has the same number of variables. + bool didDiagnose = false; for (auto i : range(PBD->getNumPatternEntries())) { SmallVector VDs; SmallVector AVDs; @@ -1161,18 +1162,24 @@ void checkABIAttrPBD(PatternBindingDecl *APBD, VarDecl *VD) { if (VDs.size() < AVDs.size()) { for (auto AVD : drop_begin(AVDs, VDs.size())) { - AVD->diagnose(diag::attr_abi_mismatched_var, - AVD, /*isABI=*/true); + AVD->diagnose(diag::attr_abi_mismatched_var, AVD, /*isABI=*/true); + didDiagnose = true; } } else if (VDs.size() > AVDs.size()) { for (auto VD : drop_begin(VDs, AVDs.size())) { - VD->diagnose(diag::attr_abi_mismatched_var, - VD, /*isABI=*/false); + VD->diagnose(diag::attr_abi_mismatched_var, VD, /*isABI=*/false); + didDiagnose = true; } } } + if (didDiagnose) + return; + + // Check the ABI PBD--this is what checks the underlying vars. + TypeChecker::typeCheckDecl(APBD); } + } // end anonymous namespace void TypeChecker::checkDeclABIAttribute(Decl *D, ABIAttr *attr) { diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index c4d26ddd4011f..5a89e39e6f931 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -3888,6 +3888,10 @@ void HasStorageRequest::cacheResult(bool hasStorage) const { StorageImplInfo StorageImplInfoRequest::evaluate(Evaluator &evaluator, AbstractStorageDecl *storage) const { + auto abiRole = ABIRoleInfo(storage); + if (!abiRole.providesAPI() && abiRole.getCounterpart()) + return abiRole.getCounterpart()->getImplInfo(); + if (auto *param = dyn_cast(storage)) { return StorageImplInfo::getSimpleStored( param->isImmutableInFunctionBody() From 9d63cf84c73d9bc3a9263cad3d961c2c83e1ee6f Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Fri, 28 Mar 2025 19:18:29 -0700 Subject: [PATCH 2/8] =?UTF-8?q?Tweak=20redecl=20checking=E2=80=99s=20handl?= =?UTF-8?q?ing=20of=20generated=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Macro expansions are now treated like a part of the source file they belong to, for purposes of the “second declaration is the one that’s diagnosed” rule. This helps stabilize a behavior that was easy to perturb. --- lib/Sema/TypeCheckDeclPrimary.cpp | 8 ++++---- test/Macros/macro_expand.swift | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index c27bb4e0b3094..f7fef359f9466 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -618,15 +618,15 @@ static void checkGenericParams(GenericContext *ownerCtx) { /// Returns \c true if \p current and \p other are in the same source file /// \em and \c current appears before \p other in that file. static bool isBeforeInSameFile(Decl *current, Decl *other) { - if (current->getDeclContext()->getParentSourceFile() != - other->getDeclContext()->getParentSourceFile()) + if (current->getDeclContext()->getOutermostParentSourceFile() != + other->getDeclContext()->getOutermostParentSourceFile()) return false; - if (!current->getLoc().isValid()) + if (current->getLoc().isInvalid() || other->getLoc().isInvalid()) return false; return current->getASTContext().SourceMgr - .isBeforeInBuffer(current->getLoc(), other->getLoc()); + .isBefore(current->getLoc(), other->getLoc()); } template diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift index f861863a8fb9c..08c8c008cc059 100644 --- a/test/Macros/macro_expand.swift +++ b/test/Macros/macro_expand.swift @@ -134,6 +134,24 @@ class HasStoredPropertyClassInvalid { #AddStoredProperty((Self.self, 0).1) // expected-note {{in expansion of macro 'AddStoredProperty' here}} // CHECK-DIAGS: @__swiftmacro_9MacroUser0023macro_expandswift_elFCffMX[[@LINE-2]]_2_33_{{.*}}AddStoredPropertyfMf_.swift:1:22: error: covariant 'Self' type cannot be referenced from a stored property initializer } + +// Redeclaration checking should behave as though expansions are part of the +// source file. +struct RedeclChecking { + #varValue + + // expected-error@+1 {{invalid redeclaration of 'value'}} + var value: Int { 0 } +} + +// CHECK-DIAGS: macro_expand.swift:[[@LINE-3]]:7: error: invalid redeclaration of 'value' +// CHECK-DIAGS: @__swiftmacro_9MacroUser0023macro_expandswift_elFCffMX[[@LINE-8]]_2_33_4361AD9339943F52AE6186DD51E04E91Ll8varValuefMf_.swift:1:5: note: 'value' previously declared here +// CHECK-DIAGS: CONTENTS OF FILE @__swiftmacro_9MacroUser0023macro_expandswift_elFCffMX[[@LINE-9]]_2_33_4361AD9339943F52AE6186DD51E04E91Ll8varValuefMf_.swift: +// CHECK-DIAGS: var value: Int { +// CHECK-DIAGS: 1 +// CHECK-DIAGS: } +// CHECK-DIAGS: END CONTENTS OF FILE + #endif @freestanding(declaration) From f01c0a7580e4776a302f7e6832575b823d5dade2 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Fri, 18 Apr 2025 14:47:04 -0700 Subject: [PATCH 3/8] Diagnose CustomAttrs as needed in `@abi` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CustomAttr backs four different features, each of which requires a different behavior in `@abi`: • Global actors: Permitted (and permitted to vary) since they can affect mangling • Result builders: Forbidden inside an `@abi` since they have no ABI impact • Property wrappers: Forbidden both inside an `@abi` and on a decl with an `@abi` since it’s not clear how we would apply `@abi` to the auxiliary decls • Attached macros: Forbidden inside an `@abi` since an ABI-only decl has no body, accessors, members, peers, extensions, or (currently) conformances Implement these behaviors (outside of `ABIDeclChecker` since they can’t be described there). Macro-related tests are not included in this commit; they require matching swift-syntax changes which are being negotiated. --- include/swift/AST/DiagnosticsSema.def | 6 ++- lib/Sema/TypeCheckAttr.cpp | 23 ++++++++++++ lib/Sema/TypeCheckMacros.cpp | 6 +++ lib/Sema/TypeCheckPropertyWrapper.cpp | 10 ++++- test/attr/attr_abi.swift | 54 +++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 30591ed595705..359ecb7cf9285 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7457,7 +7457,7 @@ ERROR(property_wrapper_effectful,none, ERROR(property_with_wrapper_conflict_attribute,none, "property %0 with a wrapper cannot also be " - "%select{lazy|@NSCopying|@NSManaged|weak|unowned|unmanaged}1", + "%select{lazy|@NSCopying|@NSManaged|@abi|weak|unowned|unmanaged}1", (Identifier, int)) ERROR(property_wrapper_not_single_var, none, "property wrapper can only apply to a single variable", ()) @@ -8509,6 +8509,10 @@ ERROR(attr_abi_no_default_arguments,none, "affect the parameter's ABI", (Decl *)) +ERROR(attr_abi_no_macros,none, + "%kind0 cannot be expanded in '@abi' attribute", + (Decl *)) + // These macros insert 'final', 'non-final', or nothing depending on both the // current decl and its counterpart, such that 'non-final' is used if the // counterpart would be described as 'final' or 'static'. They must be kept in diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index f1eb8a9824c76..d69860e6a2e60 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -4406,6 +4406,12 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { } } + // Macros can't be attached to ABI-only decls. (If we diagnosed above, + // don't bother with this.) + if (attr->isValid() && !ABIRoleInfo(D).providesAPI()) { + diagnoseAndRemoveAttr(attr, diag::attr_abi_no_macros, macro); + } + return; } @@ -4487,16 +4493,25 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { // function, storage with an explicit getter, or parameter of function type. if (nominal->getAttrs().hasAttribute()) { ValueDecl *decl; + ValueDecl *abiRelevantDecl; if (auto param = dyn_cast(D)) { decl = param; + abiRelevantDecl = dyn_cast( + param->getDeclContext()->getAsDecl()); } else if (auto func = dyn_cast(D)) { decl = func; + abiRelevantDecl = func; } else if (auto storage = dyn_cast(D)) { decl = storage; + abiRelevantDecl = storage; // Check whether this is a storage declaration that is not permitted // to have a result builder attached. auto shouldDiagnose = [&]() -> bool { + // We'll diagnose use in @abi later. + if (!ABIRoleInfo(abiRelevantDecl).providesAPI()) + return false; + // An uninitialized stored property in a struct can have a function // builder attached. if (auto var = dyn_cast(decl)) { @@ -4540,6 +4555,14 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { return; } + // Result builders shouldn't be applied to an ABI-only decl because they + // have no ABI effect. + if (!ABIRoleInfo(abiRelevantDecl).providesAPI()) { + diagnoseAndRemoveAttr(attr, diag::attr_abi_forbidden_attr, + nominal->getNameStr(), /*isModifier=*/false); + return; + } + // Diagnose and ignore arguments. if (attr->hasArgs()) { diagnose(attr->getLocation(), diag::result_builder_arguments) diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index aca42f25c8a92..2f3e1de638205 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -1395,6 +1395,12 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, } } + // Macros are so spectacularly not valid in an `@abi` attribute that we cannot + // even attempt to expand them. + if (!ABIRoleInfo(attachedTo).providesAPI()) { + return nullptr; + } + ASTContext &ctx = dc->getASTContext(); auto moduleDecl = dc->getParentModule(); diff --git a/lib/Sema/TypeCheckPropertyWrapper.cpp b/lib/Sema/TypeCheckPropertyWrapper.cpp index 7969b31658b1d..ed8dbea775570 100644 --- a/lib/Sema/TypeCheckPropertyWrapper.cpp +++ b/lib/Sema/TypeCheckPropertyWrapper.cpp @@ -481,11 +481,15 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator, continue; } + auto abiRole = ABIRoleInfo(var); + bool hasOrIsABI = !abiRole.providesAPI() || !abiRole.providesABI(); + // Note: Getting the semantic attrs here would trigger a request cycle. auto attachedAttrs = var->getAttrs(); // Check for conflicting attributes. - if (attachedAttrs.hasAttribute() || + if (hasOrIsABI || + attachedAttrs.hasAttribute() || attachedAttrs.hasAttribute() || attachedAttrs.hasAttribute() || (attachedAttrs.hasAttribute() && @@ -498,9 +502,11 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator, whichKind = 1; else if (attachedAttrs.hasAttribute()) whichKind = 2; + else if (hasOrIsABI) + whichKind = 3; else { auto attr = attachedAttrs.getAttribute(); - whichKind = 2 + static_cast(attr->get()); + whichKind = 3 + static_cast(attr->get()); } var->diagnose(diag::property_with_wrapper_conflict_attribute, var->getName(), whichKind); diff --git a/test/attr/attr_abi.swift b/test/attr/attr_abi.swift index 96be8a8d29105..ad52dac0f12e0 100644 --- a/test/attr/attr_abi.swift +++ b/test/attr/attr_abi.swift @@ -1267,6 +1267,60 @@ nonisolated func isolation17() async {} @abi(@concurrent func isolation19() async) nonisolated(nonsending) func isolation19() async {} +// CustomAttr for property wrapper -- banned in and with '@abi' +// Banned because we would need to design behavior for its auxiliary decls. +@propertyWrapper struct PropertyWrapper { + var wrappedValue: Int { fatalError() } +} + +struct CustomAttrPropertyWrapper { + @abi(@PropertyWrapper var v1: Int) // expected-error {{property 'v1' with a wrapper cannot also be @abi}} + @PropertyWrapper var v1: Int // expected-error {{property 'v1' with a wrapper cannot also be @abi}} + + @abi(@PropertyWrapper var v2: Int) // expected-error {{property 'v2' with a wrapper cannot also be @abi}} + var v2: Int + + @abi(var v3: Int) + @PropertyWrapper var v3: Int // expected-error {{property 'v3' with a wrapper cannot also be @abi}} +} + +// CustomAttr for attached macro -- see Macros/macro_expand_peers.swift + +// CustomAttr for result builder -- banned in '@abi' +// Has no ABI impact on either a parameter or a decl. +@resultBuilder struct ResultBuilder { + static func buildBlock(_ values: Int...) -> Int { values.reduce(0, +) } +} + +struct CustomAttrResultBuilder { + @abi(@ResultBuilder var v1: Int) // expected-error {{unused 'ResultBuilder' attribute in '@abi'}} {{8-22=}} + @ResultBuilder var v1: Int { 0 } + + @abi(@ResultBuilder var v2: Int) // expected-error {{unused 'ResultBuilder' attribute in '@abi'}} {{8-22=}} + var v2: Int { 0 } + + @abi(var v3: Int) + @ResultBuilder var v3: Int { 0 } + + @abi(@ResultBuilder func fn11() -> Int) // expected-error {{unused 'ResultBuilder' attribute in '@abi'}} {{8-22=}} + @ResultBuilder func fn11() -> Int { 0 } + + @abi(@ResultBuilder func fn21() -> Int) // expected-error {{unused 'ResultBuilder' attribute in '@abi'}} {{8-22=}} + func fn21() -> Int { 0 } + + @abi(func fn31() -> Int) + @ResultBuilder func fn31() -> Int { 0 } + + @abi(func fn12(@ResultBuilder _: () -> Int) -> Int) // expected-error {{unused 'ResultBuilder' attribute in '@abi'}} {{18-32=}} + func fn12(@ResultBuilder _: () -> Int) -> Int { 0 } + + @abi(func fn22(@ResultBuilder _: () -> Int) -> Int) // expected-error {{unused 'ResultBuilder' attribute in '@abi'}} {{18-32=}} + func fn22(_: () -> Int) -> Int { 0 } + + @abi(func fn32(_: () -> Int) -> Int) + func fn32(@ResultBuilder _: () -> Int) -> Int { 0 } +} + // NSCopying - see attr/attr_abi_objc.swift // @LLDBDebuggerFunction -- banned in @abi From 25bf069ce3aadcadb5357d14743cff48fe657cd8 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Fri, 18 Apr 2025 14:48:42 -0700 Subject: [PATCH 4/8] [Legacy parser] No freestanding macros in `@abi` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SwiftSyntaxParser is already doing this, and we already diagnosed it in Sema anyway, so we’re just moving that diagnostic earlier so the ASTGen testing mode is happy. Also adding compiler tests for it. Macro-related tests are not included in this commit; they require matching swift-syntax changes which are being negotiated. --- include/swift/AST/DiagnosticsParse.def | 5 +++++ lib/Parse/ParseDecl.cpp | 15 ++++++++++++--- test/attr/attr_abi.swift | 1 + 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def index 06c64b5177e93..4e2d140d82412 100644 --- a/include/swift/AST/DiagnosticsParse.def +++ b/include/swift/AST/DiagnosticsParse.def @@ -1571,6 +1571,11 @@ ERROR(attr_unsupported_on_target, none, ERROR(attr_name_unsupported_on_target, none, "attribute '%0' is unsupported on target '%1'", (StringRef, StringRef)) +// abi attribute +ERROR(attr_abi_incompatible_kind,none, + "cannot use %0 in '@abi'", + (DescriptiveDeclKind)) + // availability ERROR(attr_availability_platform,none, "expected platform name or '*' for '%0' attribute", (StringRef)) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 4b117df7f0e1d..e3c61d877de02 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -3359,9 +3359,18 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes, } if (abiDecl) { - Attributes.add(new (Context) ABIAttr(abiDecl, - AtLoc, { Loc, rParenLoc }, - /*implicit=*/false)); + auto attr = new (Context) ABIAttr(abiDecl, AtLoc, { Loc, rParenLoc }, + /*implicit=*/false); + + // Diagnose syntactically invalid abiDecl kind here to match behavior of + // Swift parser. + if (!attr->canAppearOnDecl(abiDecl) && !isa(abiDecl)){ + diagnose(abiDecl->getLoc(), diag::attr_abi_incompatible_kind, + abiDecl->getDescriptiveKind()); + attr->setInvalid(); + } + + Attributes.add(attr); } break; diff --git a/test/attr/attr_abi.swift b/test/attr/attr_abi.swift index ad52dac0f12e0..53948a2bc590b 100644 --- a/test/attr/attr_abi.swift +++ b/test/attr/attr_abi.swift @@ -1285,6 +1285,7 @@ struct CustomAttrPropertyWrapper { } // CustomAttr for attached macro -- see Macros/macro_expand_peers.swift +// Freestanding macro in @abi -- see Macros/macro_expand.swift // CustomAttr for result builder -- banned in '@abi' // Has no ABI impact on either a parameter or a decl. From 239831403c8149d04fd48fbc3f283bf3ebadce81 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Thu, 20 Mar 2025 17:15:03 -0700 Subject: [PATCH 5/8] Forbid `lazy` with `@abi` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s not clear how `@abi` would apply to the auxiliary decl used for `lazy`. --- include/swift/AST/DeclAttr.def | 2 +- include/swift/AST/DiagnosticsSema.def | 3 +++ lib/Sema/TypeCheckAttr.cpp | 5 +++++ test/attr/attr_abi.swift | 13 +++++++------ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def index 52f1051dd029f..3ba7cf7d39763 100644 --- a/include/swift/AST/DeclAttr.def +++ b/include/swift/AST/DeclAttr.def @@ -143,7 +143,7 @@ SIMPLE_DECL_ATTR(NSManaged, NSManaged, CONTEXTUAL_SIMPLE_DECL_ATTR(lazy, Lazy, OnVar, - DeclModifier | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr, + DeclModifier | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | UnconstrainedInABIAttr, 16) SIMPLE_DECL_ATTR(LLDBDebuggerFunction, LLDBDebuggerFunction, diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 359ecb7cf9285..e3a6f368bc5ac 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -8512,6 +8512,9 @@ ERROR(attr_abi_no_default_arguments,none, ERROR(attr_abi_no_macros,none, "%kind0 cannot be expanded in '@abi' attribute", (Decl *)) +ERROR(attr_abi_no_lazy,none, + "'lazy' is not compatible with '@abi' attribute", + ()) // These macros insert 'final', 'non-final', or nothing depending on both the // current decl and its counterpart, such that 'non-final' is used if the diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index d69860e6a2e60..a19ccdcebf681 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -1110,6 +1110,11 @@ void AttributeChecker::visitLazyAttr(LazyAttr *attr) { // are already lazily initialized). if (VD->isStatic() || varDC->isModuleScopeContext()) diagnoseAndRemoveAttr(attr, diag::lazy_on_already_lazy_global); + + // 'lazy' can't be used in or with `@abi` because it has auxiliary decls. + auto abiRole = ABIRoleInfo(D); + if (!abiRole.providesABI() || !abiRole.providesAPI()) + diagnoseAndRemoveAttr(attr, diag::attr_abi_no_lazy); } bool AttributeChecker::visitAbstractAccessControlAttr( diff --git a/test/attr/attr_abi.swift b/test/attr/attr_abi.swift index 53948a2bc590b..6ed54dd49498c 100644 --- a/test/attr/attr_abi.swift +++ b/test/attr/attr_abi.swift @@ -1946,16 +1946,17 @@ class Required { required init(i3: Void) { fatalError() } // expected-note {{should match modifier here}} } -// lazy -- automatically cloned into @abi +// lazy -- banned both in and with @abi +// This introduces auxiliary decls whose ABI could not be controlled. class Lazy { - @abi(lazy var v1: Int) - lazy var v1: Int = 0 + @abi(lazy var v1: Int) // expected-error {{'lazy' is not compatible with '@abi' attribute}} {{8-12=}} + lazy var v1: Int = 0 // expected-error {{'lazy' is not compatible with '@abi' attribute}} {{3-8=}} - @abi(lazy var v2: Int) // expected-error {{extra 'lazy' modifier in '@abi'}} {{8-12=}} + @abi(lazy var v2: Int) // expected-error {{'lazy' is not compatible with '@abi' attribute}} {{8-12=}} var v2: Int = 0 - @abi(var v3: Int) // expected-remark {{inferred 'lazy' in '@abi' to match modifier on API}} - lazy var v3: Int = 0 // expected-note {{matches modifier here}} + @abi(var v3: Int) + lazy var v3: Int = 0 // expected-error {{'lazy' is not compatible with '@abi' attribute}} {{3-8=}} } // @_fixed_layout -- banned in @abi From aac6f64051bf5c6af75d5d8d98192bdd9ac8e0f0 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Thu, 20 Mar 2025 18:52:03 -0700 Subject: [PATCH 6/8] Make inlinability non-ABI for `@abi` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inlinability doesn’t affect the mangling except in function specializations, which are applied after the fact and should never mangle in information from an ABI-only decl. That means we can simply ban these from `@abi` instead of inferring them. Also adds some assertions to help double-check that SIL never tries to directly mangle or retrieve inlinability info from an ABI-only decl. --- include/swift/AST/DeclAttr.def | 8 +++---- lib/SIL/IR/SILDeclRef.cpp | 19 ++++++++++++--- test/attr/attr_abi.swift | 43 ++++++++++++++++++---------------- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def index 3ba7cf7d39763..066c023d12840 100644 --- a/include/swift/AST/DeclAttr.def +++ b/include/swift/AST/DeclAttr.def @@ -163,7 +163,7 @@ SIMPLE_DECL_ATTR(unsafe_no_objc_tagged_pointer, UnsafeNoObjCTaggedPointer, DECL_ATTR(inline, Inline, OnVar | OnSubscript | OnAbstractFunction, - ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr, + ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr, 20) DECL_ATTR(_semantics, Semantics, @@ -193,7 +193,7 @@ CONTEXTUAL_SIMPLE_DECL_ATTR(postfix, Postfix, SIMPLE_DECL_ATTR(_transparent, Transparent, OnFunc | OnAccessor | OnConstructor | OnVar | OnDestructor, - UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr, + UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr, 26) SIMPLE_DECL_ATTR(requires_stored_property_inits, RequiresStoredPropertyInits, @@ -216,7 +216,7 @@ SIMPLE_DECL_ATTR(_fixed_layout, FixedLayout, SIMPLE_DECL_ATTR(inlinable, Inlinable, OnVar | OnSubscript | OnAbstractFunction, - ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr, + ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr, 32) DECL_ATTR(_specialize, Specialize, @@ -466,7 +466,7 @@ DECL_ATTR(_private, PrivateImport, SIMPLE_DECL_ATTR(_alwaysEmitIntoClient, AlwaysEmitIntoClient, OnVar | OnSubscript | OnAbstractFunction, - UserInaccessible | ABIBreakingToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr, + UserInaccessible | ABIBreakingToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr, 83) SIMPLE_DECL_ATTR(_implementationOnly, ImplementationOnly, diff --git a/lib/SIL/IR/SILDeclRef.cpp b/lib/SIL/IR/SILDeclRef.cpp index 134ecc7d77e8b..5c809011d2c53 100644 --- a/lib/SIL/IR/SILDeclRef.cpp +++ b/lib/SIL/IR/SILDeclRef.cpp @@ -883,6 +883,9 @@ SerializedKind_t SILDeclRef::getSerializedKind() const { auto *d = getDecl(); + ASSERT(ABIRoleInfo(d).providesAPI() + && "should not get serialization info from ABI-only decl"); + // Default and property wrapper argument generators are serialized if the // containing declaration is public. if (isDefaultArgGenerator() || (isPropertyWrapperBackingInitializer() && @@ -1021,6 +1024,9 @@ bool SILDeclRef::isNoinline() const { return false; auto *decl = getDecl(); + ASSERT(ABIRoleInfo(decl).providesAPI() + && "should not get inline attr from ABI-only decl"); + if (auto *attr = decl->getAttrs().getAttribute()) if (attr->getKind() == InlineKind::Never) return true; @@ -1051,6 +1057,9 @@ bool SILDeclRef::isAlwaysInline() const { return false; } + ASSERT(ABIRoleInfo(decl).providesAPI() + && "should not get inline attr from ABI-only decl"); + if (auto attr = decl->getAttrs().getAttribute()) if (attr->getKind() == InlineKind::Always) return true; @@ -1070,6 +1079,10 @@ bool SILDeclRef::isBackDeployed() const { return false; auto *decl = getDecl(); + + ASSERT(ABIRoleInfo(decl).providesAPI() + && "should not get backDeployed from ABI-only decl"); + if (auto afd = dyn_cast(decl)) return afd->isBackDeployed(getASTContext()); @@ -1198,6 +1211,9 @@ static std::string mangleClangDecl(Decl *decl, bool isForeign) { } std::string SILDeclRef::mangle(ManglingKind MKind) const { + ASSERT(!hasDecl() || ABIRoleInfo(getDecl()).providesAPI() + && "SILDeclRef mangling ABI decl directly?"); + using namespace Mangle; ASTMangler mangler(getASTContext()); @@ -1268,9 +1284,6 @@ std::string SILDeclRef::mangle(ManglingKind MKind) const { if (auto *ACE = getAbstractClosureExpr()) return mangler.mangleClosureEntity(ACE, SKind); - ASSERT(ABIRoleInfo(getDecl()).providesAPI() - && "SILDeclRef mangling ABI decl directly?"); - // As a special case, functions can have manually mangled names. // Use the SILGen name only for the original non-thunked, non-curried entry // point. diff --git a/test/attr/attr_abi.swift b/test/attr/attr_abi.swift index 6ed54dd49498c..9981458bd81f8 100644 --- a/test/attr/attr_abi.swift +++ b/test/attr/attr_abi.swift @@ -1868,45 +1868,48 @@ func section2() {} @abi(func section3()) @_section("fnord") func section3() {} -// @inlinable -- automatically cloned into @abi -@abi(@inlinable func inlinable1()) +// @inlinable -- banned in @abi +// Although the inlining *does* occasionally get mangled, it's only done in the +// SpecializationManglers, which shouldn't get their serialization from an ABI +// attribute. +@abi(@inlinable func inlinable1()) // expected-error {{unused 'inlinable' attribute in '@abi'}} {{6-16=}} @inlinable func inlinable1() {} -@abi(@inlinable func inlinable2()) // expected-error {{extra 'inlinable' attribute in '@abi'}} {{6-16=}} +@abi(@inlinable func inlinable2()) // expected-error {{unused 'inlinable' attribute in '@abi'}} {{6-16=}} func inlinable2() {} -@abi(func inlinable3()) // expected-remark {{inferred '@inlinable' in '@abi' to match attribute on API}} -@inlinable func inlinable3() {} // expected-note {{matches attribute here}} +@abi(func inlinable3()) +@inlinable func inlinable3() {} -// @inline -- automatically cloned into @abi -@abi(@inline(never) func inline1()) +// @inlinable -- banned in @abi +@abi(@inline(never) func inline1()) // expected-error {{unused 'inline(never)' attribute in '@abi'}} {{6-20=}} @inline(never) func inline1() {} -@abi(@inline(never) func inline2()) // expected-error {{extra 'inline(never)' attribute in '@abi'}} {{6-20=}} +@abi(@inline(never) func inline2()) // expected-error {{unused 'inline(never)' attribute in '@abi'}} {{6-20=}} func inline2() {} -@abi(func inline3()) // expected-remark {{inferred '@inline(never)' in '@abi' to match attribute on API}} -@inline(never) func inline3() {} // expected-note {{matches attribute here}} +@abi(func inline3()) +@inline(never) func inline3() {} -// @_transparent -- automatically cloned into @abi -@abi(@_transparent func transparent1()) +// @_transparent -- banned in @abi +@abi(@_transparent func transparent1()) // expected-error {{unused '_transparent' attribute in '@abi'}} {{6-19=}} @_transparent func transparent1() {} -@abi(@_transparent func transparent2()) // expected-error {{extra '_transparent' attribute in '@abi'}} {{6-19=}} +@abi(@_transparent func transparent2()) // expected-error {{unused '_transparent' attribute in '@abi'}} {{6-19=}} func transparent2() {} -@abi(func transparent3()) // expected-remark {{inferred '@_transparent' in '@abi' to match attribute on API}} -@_transparent func transparent3() {} // expected-note {{matches attribute here}} +@abi(func transparent3()) +@_transparent func transparent3() {} -// @_alwaysEmitIntoClient -- automatically cloned into @abi -@abi(@_alwaysEmitIntoClient func alwaysEmitIntoClient1()) +// @_alwaysEmitIntoClient -- banned in @abi +@abi(@_alwaysEmitIntoClient func alwaysEmitIntoClient1()) // expected-error {{unused '_alwaysEmitIntoClient' attribute in '@abi'}} {{6-28=}} @_alwaysEmitIntoClient func alwaysEmitIntoClient1() {} -@abi(@_alwaysEmitIntoClient func alwaysEmitIntoClient2()) // expected-error {{extra '_alwaysEmitIntoClient' attribute in '@abi'}} {{6-28=}} +@abi(@_alwaysEmitIntoClient func alwaysEmitIntoClient2()) // expected-error {{unused '_alwaysEmitIntoClient' attribute in '@abi'}} {{6-28=}} func alwaysEmitIntoClient2() {} -@abi(func alwaysEmitIntoClient3()) // expected-remark {{inferred '@_alwaysEmitIntoClient' in '@abi' to match attribute on API}} -@_alwaysEmitIntoClient func alwaysEmitIntoClient3() {} // expected-note {{matches attribute here}} +@abi(func alwaysEmitIntoClient3()) +@_alwaysEmitIntoClient func alwaysEmitIntoClient3() {} // @_optimize(none) -- banned in @abi @abi(@_optimize(none) func optimize1()) // expected-error {{unused '_optimize(none)' attribute in '@abi'}} {{6-22=}} From 8f75878455d8b39f590d8892e982109eb0408dc6 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Thu, 20 Mar 2025 19:22:31 -0700 Subject: [PATCH 7/8] Forbid `@_borrowed` in `@abi` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It has indirect effects on the accessors, so it shouldn’t matter, but we can defensively redirect the query to the API counterpart anyway. This was the last `InferredInABIAttr` attribute, so we can now remove all of the infrastructure involved in supporting attribute inference. --- include/swift/AST/Attr.h | 11 ++--------- include/swift/AST/DeclAttr.def | 2 +- include/swift/AST/DiagnosticsSema.def | 3 --- include/swift/Basic/LangOptions.h | 3 --- include/swift/Option/Options.td | 4 ---- lib/AST/Attr.cpp | 2 +- lib/Frontend/CompilerInvocation.cpp | 2 -- lib/Sema/TypeCheckAttrABI.cpp | 20 -------------------- lib/Sema/TypeCheckStorage.cpp | 4 ++++ test/attr/attr_abi.swift | 12 ++++++------ test/attr/attr_abi_objc.swift | 2 +- 11 files changed, 15 insertions(+), 50 deletions(-) diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h index c92c236a06338..0a49634fec4f4 100644 --- a/include/swift/AST/Attr.h +++ b/include/swift/AST/Attr.h @@ -380,21 +380,14 @@ class DeclAttribute : public AttributeBase { /// valid if they match. EquivalentInABIAttr = 1ull << 18, - /// Attribute can be used in an \c \@abi attribute, but must match - /// equivalent on API decl; if omitted, API decl's attribute will be - /// cloned. Use where you would want to use \c EquivalentInABIAttr but - /// repeating the attribute is judged too burdensome. - InferredInABIAttr = 1ull << 19, - /// Use for attributes which are \em only valid on declarations that cannot /// have an \c @abi attribute, such as \c ImportDecl . - UnreachableInABIAttr = 1ull << 20, + UnreachableInABIAttr = 1ull << 19, }; enum : uint64_t { InABIAttrMask = ForbiddenInABIAttr | UnconstrainedInABIAttr - | EquivalentInABIAttr | InferredInABIAttr - | UnreachableInABIAttr + | EquivalentInABIAttr | UnreachableInABIAttr }; LLVM_READNONE diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def index 066c023d12840..a67f68f360949 100644 --- a/include/swift/AST/DeclAttr.def +++ b/include/swift/AST/DeclAttr.def @@ -456,7 +456,7 @@ DECL_ATTR(_dynamicReplacement, DynamicReplacement, SIMPLE_DECL_ATTR(_borrowed, Borrowed, OnVar | OnSubscript, - UserInaccessible | NotSerialized | ABIBreakingToAdd | ABIBreakingToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr, + UserInaccessible | NotSerialized | ABIBreakingToAdd | ABIBreakingToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr, 81) DECL_ATTR(_private, PrivateImport, diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index e3a6f368bc5ac..a24b04e264314 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -8467,9 +8467,6 @@ ERROR(attr_abi_extra_attr,none, ERROR(attr_abi_forbidden_attr,none, "unused '%0' %select{attribute|modifier}1 in '@abi'", (StringRef, bool)) -REMARK(abi_attr_inferred_attribute,none, - "inferred '%0' in '@abi' to match %select{attribute|modifier}1 on API", - (StringRef, bool)) ERROR(attr_abi_mismatched_attr,none, "'%0' %select{attribute|modifier}1 in '@abi' should match '%2'", diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index 8a73b09290079..5d9cac773637b 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -269,9 +269,6 @@ namespace swift { /// Emit a remark on early exit in explicit interface build bool EnableSkipExplicitInterfaceModuleBuildRemarks = false; - /// Emit a remark when \c \@abi infers an attribute or modifier. - bool EnableABIInferenceRemarks = false; - /// /// Support for alternate usage modes /// diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td index 48af6f4b37382..7ac36a3b705ef 100644 --- a/include/swift/Option/Options.td +++ b/include/swift/Option/Options.td @@ -468,10 +468,6 @@ def remark_module_serialization : Flag<["-"], "Rmodule-serialization">, Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>, HelpText<"Emit remarks about module serialization">; -def remark_abi_inference : Flag<["-"], "Rabi-inference">, - Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>, - HelpText<"Emit a remark when an '@abi' attribute adds an attribute or modifier to the ABI declaration based on its presence in the API">; - def emit_tbd : Flag<["-"], "emit-tbd">, HelpText<"Emit a TBD file">, Flags<[FrontendOption, NoInteractiveOption, SupplementaryOutput]>; diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp index 33d62846331b0..37169d63a30b5 100644 --- a/lib/AST/Attr.cpp +++ b/lib/AST/Attr.cpp @@ -61,7 +61,7 @@ static_assert(IsTriviallyDestructible::value, DeclAttribute::APIBreakingToRemove | DeclAttribute::APIStableToRemove), \ #Name " needs to specify either APIBreakingToRemove or APIStableToRemove"); \ static_assert(DeclAttribute::hasOneBehaviorFor##Id(DeclAttribute::InABIAttrMask), \ - #Name " needs to specify exactly one of ForbiddenInABIAttr, UnconstrainedInABIAttr, EquivalentInABIAttr, InferredInABIAttr, or UnreachableInABIAttr"); + #Name " needs to specify exactly one of ForbiddenInABIAttr, UnconstrainedInABIAttr, EquivalentInABIAttr, or UnreachableInABIAttr"); #include "swift/AST/DeclAttr.def" #define TYPE_ATTR(_, Id) \ diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 3b5430331fb76..b0f625e9a2272 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1422,8 +1422,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args, Opts.EnableSkipExplicitInterfaceModuleBuildRemarks = Args.hasArg(OPT_remark_skip_explicit_interface_build); - Opts.EnableABIInferenceRemarks = Args.hasArg(OPT_remark_abi_inference); - if (Args.hasArg(OPT_experimental_skip_non_exportable_decls)) { // Only allow -experimental-skip-non-exportable-decls if either library // evolution is enabled (in which case the module's ABI is independent of diff --git a/lib/Sema/TypeCheckAttrABI.cpp b/lib/Sema/TypeCheckAttrABI.cpp index d4e9aefb037b1..1049b7058f242 100644 --- a/lib/Sema/TypeCheckAttrABI.cpp +++ b/lib/Sema/TypeCheckAttrABI.cpp @@ -880,26 +880,6 @@ class ABIDeclChecker : public ASTComparisonVisitor { return false; - case DeclAttribute::InferredInABIAttr: - if (!abi && api->canClone()) { - // Infer an identical attribute. - abi = api->clone(ctx); - abi->setImplicit(true); - abiDecl->getAttrs().add(abi); - - if (ctx.LangOpts.EnableABIInferenceRemarks) { - SmallString<64> scratch; - auto abiAttrAsString = printAttr(abi, abiDecl, scratch); - - abiDecl->diagnose(diag::abi_attr_inferred_attribute, - abiAttrAsString, api->isDeclModifier()); - noteAttrHere(api, apiDecl, /*isMatch=*/true); - } - } - - // Other than the cloning behavior, Inferred behaves like Equivalent. - LLVM_FALLTHROUGH; - case DeclAttribute::EquivalentInABIAttr: // Diagnose if API doesn't have attribute. if (!api) { diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 5a89e39e6f931..6c3c552bcaca1 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -849,6 +849,10 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator, OpaqueReadOwnership OpaqueReadOwnershipRequest::evaluate(Evaluator &evaluator, AbstractStorageDecl *storage) const { + auto abiRole = ABIRoleInfo(storage); + if (!abiRole.providesAPI() && abiRole.getCounterpart()) + return abiRole.getCounterpart()->getOpaqueReadOwnership(); + enum class DiagKind { BorrowedAttr, NoncopyableType diff --git a/test/attr/attr_abi.swift b/test/attr/attr_abi.swift index 9981458bd81f8..76f5fbd929cfd 100644 --- a/test/attr/attr_abi.swift +++ b/test/attr/attr_abi.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature Extern -enable-experimental-feature ABIAttribute -enable-experimental-feature AddressableParameters -enable-experimental-feature NoImplicitCopy -enable-experimental-feature SymbolLinkageMarkers -enable-experimental-feature StrictMemorySafety -enable-experimental-feature LifetimeDependence -enable-experimental-feature CImplementation -import-bridging-header %S/Inputs/attr_abi.h -parse-as-library -Rabi-inference -debugger-support +// RUN: %target-typecheck-verify-swift -enable-experimental-feature Extern -enable-experimental-feature ABIAttribute -enable-experimental-feature AddressableParameters -enable-experimental-feature NoImplicitCopy -enable-experimental-feature SymbolLinkageMarkers -enable-experimental-feature StrictMemorySafety -enable-experimental-feature LifetimeDependence -enable-experimental-feature CImplementation -import-bridging-header %S/Inputs/attr_abi.h -parse-as-library -debugger-support // REQUIRES: swift_feature_ABIAttribute // REQUIRES: swift_feature_AddressableParameters @@ -2034,15 +2034,15 @@ extension DynamicReplacement { // @_weakLinked -- tested in attr/attr_weaklinked.swift -// @_borrowed -- automatically cloned into @abi +// @_borrowed -- banned in @abi protocol BorrowedAttr { - @abi(@_borrowed var v1: Int) + @abi(@_borrowed var v1: Int) // expected-error {{unused '_borrowed' attribute in '@abi'}} {{8-18=}} @_borrowed var v1: Int { get set } - @abi(var v2: Int) // expected-remark {{inferred '@_borrowed' in '@abi' to match attribute on API}} - @_borrowed var v2: Int { get set } // expected-note {{matches attribute here}} + @abi(var v2: Int) + @_borrowed var v2: Int { get set } - @abi(@_borrowed var v3: Int) // expected-error {{extra '_borrowed' attribute in '@abi'}} {{8-18=}} + @abi(@_borrowed var v3: Int) // expected-error {{unused '_borrowed' attribute in '@abi'}} {{8-18=}} var v3: Int { get set } } diff --git a/test/attr/attr_abi_objc.swift b/test/attr/attr_abi_objc.swift index 581d1e5ab358a..8f604bf79d8ec 100644 --- a/test/attr/attr_abi_objc.swift +++ b/test/attr/attr_abi_objc.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature ABIAttribute -parse-as-library -Rabi-inference +// RUN: %target-typecheck-verify-swift -enable-experimental-feature ABIAttribute -parse-as-library // REQUIRES: swift_feature_ABIAttribute // REQUIRES: objc_interop From 97bb6b1150003c50c5e68be843577b8ec3b313ba Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Thu, 17 Apr 2025 16:49:04 -0700 Subject: [PATCH 8/8] Prevent `#if` in `@abi` in module interfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a language feature is used inside an `@abi` attribute, we should behave as though it was used on its counterpart. This was already half-implemented—we ensured the counterpart would use the feature—but we didn’t make the ABI decl aware that the counterpart was its parent for feature detection purposes. As a result, we would print `#if` inside the `@abi` attribute, which isn’t valid. --- lib/AST/FeatureSet.cpp | 6 +++++- test/ModuleInterface/attrs.swift | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index af00c2d00750e..a1297614c3ea4 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -709,8 +709,12 @@ FeatureSet swift::getUniqueFeaturesUsed(Decl *decl) { // Remove all the features used by all enclosing declarations. Decl *enclosingDecl = decl; while (!features.empty()) { + // If we were in an @abi attribute, collect from the API counterpart. + auto abiRole = ABIRoleInfo(enclosingDecl); + if (!abiRole.providesAPI() && abiRole.getCounterpart()) + enclosingDecl = abiRole.getCounterpart(); // Find the next outermost enclosing declaration. - if (auto accessor = dyn_cast(enclosingDecl)) + else if (auto accessor = dyn_cast(enclosingDecl)) enclosingDecl = accessor->getStorage(); else enclosingDecl = enclosingDecl->getDeclContext()->getAsDecl(); diff --git a/test/ModuleInterface/attrs.swift b/test/ModuleInterface/attrs.swift index 974f9d209bde6..7d56394eed3f7 100644 --- a/test/ModuleInterface/attrs.swift +++ b/test/ModuleInterface/attrs.swift @@ -85,6 +85,20 @@ public struct MutatingTest { @abi(func abiSpiFunc()) @_spi(spiGroup) public func abiSpiFunc() {} +// We should print feature guards outside, but not inside, an @abi attribute. +@abi(func sendingABI() -> sending Any?) +public func sendingABI() -> Any? { nil } +// CHECK: #if {{.*}} && $ABIAttribute +// CHECK: @abi(func sendingABI() -> sending Any?) +// CHECK: public func sendingABI() -> Any? +// CHECK: #elseif {{.*}} && $SendingArgsAndResults +// CHECK: @_silgen_name("$s5attrs10sendingABIypSgyF") +// CHECK: public func sendingABI() -> Any? +// CHECK: #else +// CHECK: @_silgen_name("$s5attrs10sendingABIypSgyF") +// CHECK: public func sendingABI() -> Any? +// CHECK: #endif + @concurrent public func testExecutionConcurrent() async {} // CHECK: @concurrent public func testExecutionConcurrent() async