Allow declarations to opt in to suppressing @isolated(any) instead of suppressing the entire declaration
#72008
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
rjmccall
requested review from
hborla,
slavapestov,
xedin,
ahoppen,
bnbarham,
CodaFi,
DougGregor,
hamishknight,
rintaro and
tshortli
as code owners
|
@swift-ci Please test |
rjmccall
force-pushed
the
isolated-any-feature-suppression
branch
from
75e6603 to
aa8dea7
Compare
|
@swift-ci Please test |
ahoppen
reviewed
Mar 1, 2024
|
@swift-ci Please test macOS |
The goal is to have a lightweight way to pass an unapplied diagnostic to general routines. Constructing a Diagnostic is quite expensive as something we're potentially doing in hot paths, as opposed to just when we're actually emitting the diagnostic. This design allows the expense to be delayed until we need it. I've also optimized the Diagnostic constructor to avoid copying arguments unnecessarily; this is a relatively small expense, since arguments are POD, but there's really no good reason not to do it.
Our standard conception of suppressible features assumes we should always suppress the feature if the compiler doesn't support it. This presumes that there's no harm in suppressing the feature, and that's a fine assumption for features that are just adding information or suppressing new diagnostics. Features that are semantically relevant, maybe even ABI-breaking, are not a good fit for this, and so instead of reprinting the decl with the feature suppressed, we just have to hide the decl entirely. The missing middle here is that it's sometimes useful to be able to adopt a type change to an existing declaration, and we'd like older compilers to be able to use the older version of the declaration. Making a type change this way is, of course, only really acceptable for @_alwaysEmitIntoClient declarations; but those represent quite a few declarations that we'd like to be able to refine the types of. Rather than trying to come up with heuristics based on @_alwaysEmitIntoClient or other sources of information, this design just requires the declaration to opt in with a new attribute, @_allowFeatureSuppress. When a declaration opts in to suppression for a conditionally-suppressible feature, the printer uses the suppression serially-print-with-downgraded-options approach; otherwise it uses the print-only-if-feature-is-available approach.
rjmccall
force-pushed
the
isolated-any-feature-suppression
branch
from
aa8dea7 to
fc538f3
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test Linux |
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 also includes a significant NFC improvement to how we use suspended diagnostics in the parser.