More @abi checking
#80383
Merged
More @abi checking
#80383
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beccadax
force-pushed
the
abi-inspected-your-appearance-again
branch
from
9049600 to
7815b82
Compare
beccadax
force-pushed
the
abi-inspected-your-appearance-again
branch
from
7815b82 to
62af8b7
Compare
beccadax
requested review from
CodaFi,
DougGregor,
ahoppen,
artemcm,
bnbarham,
hamishknight,
hborla,
jckarter,
rintaro,
slavapestov,
tshortli and
xedin
as code owners
Contributor
Author
|
With swiftlang/swift-syntax#3026 @swift-ci please test |
beccadax
force-pushed
the
abi-inspected-your-appearance-again
branch
from
62af8b7 to
9fbc6f7
Compare
Contributor
Author
|
With swiftlang/swift-syntax#3026 @swift-ci please test |
Contributor
Author
|
Yesterday's Linux failure was transient. With swiftlang/swift-syntax#3026 @swift-ci please test Linux platform |
nkcsgexi
approved these changes
Apr 17, 2025
Contributor
Author
|
With swiftlang/swift-syntax#3026 @swift-ci please test Linux platform |
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.
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.
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.
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.
It’s not clear how `@abi` would apply to the auxiliary decl used for `lazy`.
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.
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.
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.
beccadax
force-pushed
the
abi-inspected-your-appearance-again
branch
from
9fbc6f7 to
97bb6b1
Compare
Contributor
Author
|
The SwiftSyntax changes are proving a little tricky to negotiate, so I've moved the tests that will fail without SwiftSyntax adjustments into #79937 (the functional changes will still land in this PR). We can land them after SwiftSyntax makes a change here—either correcting the parsing with the experimental feature turned off, or simply removing the experimental feature because the proposal has been accepted. This will cut the dependency on swiftlang/swift-syntax#3026. Contrariwise, I've moved in a change I recently discovered was needed to fully prevent language feature |
Contributor
Author
|
@swift-ci please smoke test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refines the
@abichecking added in #79466 in several ways:VarDecls in the primary file were not fully checkedCustomAttr:@abiand may vary from the API counterpart@abi(they have no ABI impact)@abiand on an API counterpart (more design is needed to control the ABI of the auxiliary declarations)@abi(ABI-only decls have no peers or members)@abi, matching SwiftParser's behaviorlazyboth in@abiand on an API counterpart (like property wrapper attributes, more design is needed to control the ABI of auxiliary declarations)@inlinableand other inlining attributes@_borrowedThese changes bring the implementation into alignment with the revised
@abiproposal.Note that the macro expansion tests in this PR will require a matching SwiftSyntax PR that makes SwiftParser parse
@abiattributes even when the experimental feature is turned off; the recovery behavior without that patch is just too catastrophic for macro tests to emit sensible diagnostics. See swiftlang/swift-syntax#3026.Reviewers: This PR is a bit of a grab bag. You may find that the easiest way to review it is commit-by-commit, reading the individual commit message before the diff so you'll understand the intent of each change.