Skip to content
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

SwiftIfConfig: A library to evaluate #if conditionals within a Swift syntax tree. #1816

Merged
merged 42 commits into from
Jul 9, 2024

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Jun 20, 2023

Swift provides the ability to conditionally compile parts of a source file based on various built-time conditions, including information about the target (operating system, processor architecture, environment), information about the compiler (version, supported attributes and features), and user-supplied conditions specified as part of the build (e.g., DEBUG), which we collectively refer to as the build configuration. These conditions can occur within a #if in the source code, e.g.,

func f() {
#if DEBUG
  log("called f")
#endif

#if os(Linux)
  // use Linux API
#elseif os(iOS) || os(macOS)
  // use iOS/macOS API
#else
  #error("unsupported platform")
#endif
}

The syntax tree and its parser do not reason about the build configuration. Rather, the syntax tree produced by parsing this code will include IfConfigDeclSyntax nodes wherever there is a #if, and each such node contains the a list of clauses, each with a condition to check (e.g., os(Linux)) and a list of syntax nodes that are conditionally part of the program. Therefore, the syntax tree captures all the information needed to process the source file for any build configuration.

The SwiftIfConfig library provides utilities to determine which syntax nodes are part of a particular build configuration. Each utility requires that one provide a specific build configuration (i.e., an instance of a type that conforms to the doc:BuildConfiguration protocol), and provides a different view on essentially the same information:

  • doc:ActiveSyntaxVisitor and doc:ActiveSyntaxAnyVisitor are visitor types that only visit the syntax nodes that are included ("active") for a given build configuration, implicitly skipping any nodes within inactive #if clauses.
  • SyntaxProtocol.removingInactive(in:) produces a syntax node that removes all inactive regions (and their corresponding IfConfigDeclSyntax nodes) from the given syntax tree, returning a new tree that is free of #if conditions.
  • IfConfigDeclSyntax.activeClause(in:) determines which of the clauses of an #if is active for the given build configuration, returning the active clause.
  • SyntaxProtocol.isActive(in:) determines whether the given syntax node is active for the given build configuration.

@DougGregor DougGregor requested a review from ahoppen as a code owner June 20, 2023 05:51
@DougGregor
Copy link
Member Author

@swift-ci please test

@harlanhaskins
Copy link
Contributor

I would love to use this in the part of the compiler that strips inactive #if conditions out of module interfaces

Sources/SwiftIfConfig/BuildConfiguration.swift Outdated Show resolved Hide resolved
Sources/SwiftIfConfig/BuildConfiguration.swift Outdated Show resolved Hide resolved
Sources/SwiftIfConfig/IfConfigFunctions.swift Show resolved Hide resolved
Sources/SwiftIfConfig/IfConfigState.swift Outdated Show resolved Hide resolved
Sources/SwiftIfConfig/SyntaxLiteralUtils.swift Outdated Show resolved Hide resolved
Tests/SwiftIfConfigTest/EvaluateTests.swift Outdated Show resolved Hide resolved
Tests/SwiftIfConfigTest/EvaluateTests.swift Outdated Show resolved Hide resolved
Tests/SwiftIfConfigTest/EvaluateTests.swift Outdated Show resolved Hide resolved
Tests/SwiftIfConfigTest/VisitorTests.swift Outdated Show resolved Hide resolved
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test Windows

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

DougGregor commented Jul 5, 2024

Alright, major update for this library today, including:

  • Introduce complete handling for diagnostics produced via #if condition evaluation.
  • Add nice test harnesses for all of the APIs.
  • Distinguish between "inactive" and "unparsed" regions so we know where to emit syntactic diagnostics.
  • Add an API to get the "configured regions" for a tree, akin to the compiler/SourceKit's "active regions"

@DougGregor
Copy link
Member Author

@swift-ci please test

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

I forgot about PostfixIfConfigExprSyntax, so I'll have to fix that before this becomes mergable.

Building on top of the parser and operator-precedence parsing library,
introduce a new library that evaluates `#if` conditions against a
particular build configuration. The build configuration is described
by the aptly named `BuildConfiguration` protocol, which has queries
for various build settings (e.g., configuration flags), compiler
capabilities (features and attributes), and target information (OS,
architecture, endianness, etc.).

At present, the only user-facing operation is the `IfConfigState`
initializer, which takes in an expression (the `#if` condition) and a
build configuration, then evaluates that expression against the build
condition to determine whether code covered by that condition is
active, inactive, or completely unparsed. This is a fairly low-level
API, meant to be a building block for more useful higher-level APIs
that query which `#if` clause is active and whether a particular syntax
node is active.
`IfConfigDeclSyntax.activeClause(in:)` determines which clause is
active within an `#if` syntax node.

`SyntaxProtocol.isActive(in:)` determines whether a given syntax node
is active in the program, based on the nested stack of `#if`
configurations.
This is the last kind of check! Remove the `default` fallthrough from
the main evaluation function.
The `ActiveSyntax(Any)Visitor` visitor classes provide visitors that
only visit the regions of a syntax tree that are active according to
a particular build configuration, meaning that those nodes would be
included in a program that is built with that configuration.
The operation `SyntaxProtocol.removingInactive(in:)` returns a syntax
tree derived from `self` that has removed all inactive syntax nodes based
on the provided configuration.
Postfix `#if` expressions have a different syntactic form than
other `#if` clauses because they don't fit into a list-like position in
the grammar. Implement a separate, recursive folding algorithm to handle
these clauses.
When a check is "versioned" and fails, we allow the failed #if block to
have syntactic errors in it. Start to reflect this distinction when
determining the `IfConfigState` for a particular condition, so that we
finally use the "unparsed" case.
…rsed

Update the interface of this function to return an `IfConfigState` rather
than just a `Bool`. Then, check the enclosing versioned conditions to
distinguish between inactive vs. unparsed. Finally, add a marker-based
assertion function that makes it easy to test the active state of any
location in the source code. Use the new test to flush out an obvious
bug in my original implementation of `isActive(in: configuration)`.
This API produces information similar to the "active regions" API used
within the compiler and by SourceKit, a sorted array that indicates the #if
clauses that are active or inactive (including distinguishing inactive
vs. unparsed). When this array has already been computed for a syntax tree,
one can then use the new `SyntaxProtocol.isActive(inConfiguredRegions:)`
function to determine whether a given node is active. This can be more
efficient than the existing `SyntaxProtocol.isActive(in:)` when querying
for many nodes.

Test the new functionality by cross-checking the two `isActive`
implementations against each other on existing tests.
The new name better describes that we're talking about regions that
have to do with the build configuration.
@DougGregor
Copy link
Member Author

Turns out that PostfixIfConfigExprSyntax works. I've renamed IfConfigState to ConfiguredRegionState, which I think I like better. Although it's pretty long.

@DougGregor
Copy link
Member Author

@swift-ci please test

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please test

When determining active regions, treat clauses with invalid conditions
as "unparsed" regions, but don't abort the computation by throwing. This
provides behavior that is more consistent with the compiler, and is also
generally easy for most clients. Those clients that want to report
diagnostics can certainly do so, but are not forced to work with
throwing APIs for invalid code.

While here, improve the active syntax rewriting operation by making it
a two-pass operation. The first pass emits diagnostics and determines
whether there is any rewriting to do, and the second pass performs the
rewriting. This fixes an existing bug where the diagnostic locations
were wrong because we were emitting them against partially-rewritten
trees.
Rename source files in SwiftIfConfig to better reflect what they do, move
the public APIs up to the tops of files, and split the massive
IfConfigEvaluation.swift into several files. The file itself defines the
core logic for doing the evaluation (which is internal to the library),
and other source files provide public APIs on top of it.
@DougGregor
Copy link
Member Author

More improvements:

  • Improve handling of ill-formed code so that we treat an ill-formed condition as "unparsed" consistently and aligns with the compiler
  • Cleaned up file organization
  • Added support for the new _hasAtomicBitWidth in the compiler

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test Windows

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please test Windows

@DougGregor
Copy link
Member Author

Let's go ahead and merge this, and we can tweak the API over time.

@DougGregor DougGregor merged commit 70e3741 into swiftlang:main Jul 9, 2024
3 checks passed
@DougGregor DougGregor deleted the if-config branch July 9, 2024 16:14
Comment on lines +38 to +51
// First pass: Find all of the active clauses for the #ifs we need to
// visit, along with any diagnostics produced along the way. This process
// does not change the tree in any way.
let visitor = ActiveSyntaxVisitor(viewMode: .sourceAccurate, configuration: configuration)
visitor.walk(self)

// If there were no active clauses to visit, we're done!
if visitor.numIfClausesVisited == 0 {
return (Syntax(self), visitor.diagnostics)
}

// Second pass: Rewrite the syntax tree by removing the inactive clauses
// from each #if (along with the #ifs themselves).
let rewriter = ActiveSyntaxRewriter(configuration: configuration)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to have two passes here? Shouldn’t a single pass using an ActiveSyntaxRewriter be sufficient?

That way, we would also guarantee that the diagnostics match the rewritten tree if the implementation of BuildConfiguration is non-deterministic for some reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnostics are in terms of the tree before the rewrite occurs, because we will end up removing failed #ifs and therefore the nodes that the diagnostics point at. Hence, one diagnostics pass and one folding pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m pretty sure that the nodes passed into the SyntaxRewriter.visit functions are the ones pre-rewrite. So, I would expect that it should be possible to do it in a single pass. Or am I missing something here?

Comment on lines +122 to +125
// In a well-formed syntax tree, the element list is always the
// same type as List. However, handle a manually-constructed,
// ill-formed syntax tree gracefully by dropping the inner elements
// as well.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we emit a diagnostic if the children of the active clause are not of the same type as the surrounding? I imagine that hunting this sort of issue down could be quite annoying.

return dropInactive(outerBase: base, postfixIfConfig: postfixIfConfig)
}

preconditionFailure("Unhandled postfix expression in #if elimination")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to how we don’t crash on manually constructed syntax trees where the children of a #if clause are not of the same type of the surrounding type, should we also be error-tolerant here and emit a diagnostic and drop the postfix expression?

if let identified = node.asProtocol(NamedDeclSyntax.self) {
checkName(name: identified.name.text, node: node)
} else if let identPattern = node.as(IdentifierPatternSyntax.self) {
// FIXME: Should the above be an IdentifiedDeclSyntax?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you proposing to change the name of NamedDeclSyntax or IdentifierPatternSyntax? If so, could you file an issue for that instead of adding a FIXME here, which we probably won’t find again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same sort of issue we're seeing with the lexical name lookup work, where we want some kind of central notion of "syntax node that declares a name"

Release Notes/601.md Show resolved Hide resolved
@DougGregor
Copy link
Member Author

Addressed most of these comments in #2759

@ahoppen
Copy link
Member

ahoppen commented Jul 29, 2024

Thanks for addressing the comments. I’m addressing a few more minor ones in #2762.

The following discussions are still open. Composed this list partially for myself to keep track.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants