More @abi checking
#80383
Open
More @abi checking
#80383
Conversation
This file contains 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
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).
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.
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.
beccadax
force-pushed
the
abi-inspected-your-appearance-again
branch
from
7815b82 to
62af8b7
Compare
beccadax
requested review from
hborla,
slavapestov,
xedin,
jckarter,
ahoppen,
bnbarham,
CodaFi,
DougGregor,
hamishknight,
rintaro,
artemcm and
tshortli
as code owners
|
With swiftlang/swift-syntax#3026 @swift-ci please test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.