-
Notifications
You must be signed in to change notification settings - Fork 439
[SwiftIfConfig] Make ConfiguredRegions and a BuildConfiguration generally interchangeable #2834
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.
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
DougGregor
merged 7 commits into
swiftlang:main
from
DougGregor:configured-regions-everywhere
Sep 4, 2024
Merged
[SwiftIfConfig] Make ConfiguredRegions and a BuildConfiguration generally interchangeable #2834
DougGregor
merged 7 commits into
swiftlang:main
from
DougGregor:configured-regions-everywhere
Sep 4, 2024
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
…ving inactive regions The compiler's handling of inlinable text for Swift interface checking requires the ability to retain `#if`s involving compiler checks (e.g., `#if compiler(>=6.0)`) and `$`-based feature checks (`#if $AsyncAwait`) when stripping inactive regions. Expand the `removingInactive` function with a parameter that implements this behavior.
Introduce a new `SyntaxProtocol.descriptionWithoutComments` that removes comments from the given syntax, replacing them with either a space (if the comment has no newlines) or a newline (if it does have newlines).
…compiler SPI I'm not sure we have any clients for this particular option outside of the compiler, so for now, limit it to the compiler. We can expose it later if we'd like.
The operation that removes all inactive (and unparsed) regions from a syntax tree is a two-pass algorithm. For clients that have already created a ConfiguredRegions instance, let that serve as the first pass. ConfiguredRegions now tracks the active clauses of any evaluated `#if`s, and the ActiveSyntaxRewriter uses that information to guide its traversal and rewrite rather than re-evaluating all of the clauses. This required reworking the way in which we handle recursion in ActiveSyntaxRewriter. Before, we were rewriting nodes "outside-in", but doing so broke node identity and, therefore, lookup of the active #if clauses. Instead, make sure we rewrite from the leaves up to the root. Clients still have the option of removing inactive/unparsed code from a syntax node given just a build configuration, in which case we'll first create a ConfiguredRegions.
…ldConfiguration Instead of always forcing the construction of ConfiguredRegions ahead of time, introduce the notion of an ActiveClauseEvaluator that stores either configured regions or a build configuration, and can query for the active clause within a given #if declaration from either representation. Use this in ActiveSyntaxRewriter to make the operation itself single-pass, always. Always test both paths to make sure they stay in sync.
…onfiguration Rework the implementations of ActiveSyntax(Any)Visitor to allow configured regions, so that clients can re-use the work of evaluating the build configuration for later traversals. As a side effect of this, ActiveSyntax(Any)Visitor are no longer generic over the type of build configuration, which should make them easier to use.
@swift-ci please test |
@swift-ci please test macOS |
@swift-ci please test Linux |
ahoppen
reviewed
Sep 4, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Looks good to me 👍🏽
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.
Make the API surface area available for build configurations also available for configured regions. This allows clients that are going to provide repeated
#if
-related queries to produce configured regions once (for the whole syntax tree) and then perform more queries and traversals against it. Overall, this affects a few APIs:removingInactive(in:)
API is now available onConfiguredRegions
as well as on a syntax node (with build configuration). In both cases, it requires a single pass (whereas it was previously two-pass and evaluated#if
s multiple times). This require a bit of a rework in how the recursion works in this operation.ActiveSyntax(Any)Visitor
visitor classes can be initialized with eitherConfiguredRegions
orsome BuildConfiguration
. These visitor classes are no longer generic, which makes them simpler to use.