Split SwiftSyntaxParser into a separate module #309
Merged
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
|
CC @czechboy0 |
This comment has been minimized.
This comment has been minimized.
When SwiftSyntax is just being used to generate code and not to parse Swift source files, there is no reason that a compatible `_InternalSwiftSyntaxParser.dylib` needs to be present. We do, however, currently have the requirement of always needing a `_InternalSwiftSyntaxParser.dylib` because `SyntaxParser` lives inside the `SwiftSyntax` module and thus `SwiftSyntax` needs to link against `_InternalSwiftSyntaxParser.dylib`. To lift the requirement, split `SwiftSyntax` into two modules: `SwiftSyntax` has no requirement on `_InternalSwiftSyntaxParser.dylib`. It only offers functionality to programatically generate `SwiftSyntax` trees and walk them. The new `SwiftSyntaxParser` module links against `_InternalSwiftSyntaxParser.dylib` and provides the ability to generate `SwiftSyntax` trees from source files. To efficiently generate `SwiftSyntax` nodes from the parser, `RawSyntax` (and its child types) does depend on the layout of the C nodes (for example, `TokenData` stores `CTriviaPiece`s as trailing trivia and only constructs the trivia’s Swift representation when requested). I don’t think it’s possible to eliminate that behaviour without eager translation and thus decided to include a copy of the C nodes inside the `SwiftSyntax`. When `SwiftSyntax` is used on its own, they are never used and thus their value is irrelevant. When a source file is parsed through `SwiftSyntaxParser`, it first verifies that the C node layouts match those defined in `_InternalSwiftSyntaxParser.dylib`, similar to how we currently compute the node hash. If the layouts are not compatible, we throw an error. We also verify that the files definingt these types match when invoking `build-script` (e.g. in CI).
ahoppen
force-pushed
the
pr/split-swiftsyntax-parser
branch
from
cdb0af2 to
316f847
Compare
|
swiftlang/swift-stress-tester#163 @swift-ci Please test macOS |
akyrtzi
approved these changes
Sep 29, 2021
allevato
added a commit
to allevato/swift-format
that referenced
this pull request
Nov 4, 2021
This change addresses swiftlang/swift-syntax#309 and swiftlang/swift-syntax#325, which split `SwiftSyntaxParser` into its own module and then removed the `DiagnosticEngine` APIs (so only the `Diagnostic` type remains as the way diagnostics from the parser are sent back to the caller). Rather than remain coupled to swift-syntax's `Diagnostic` type, this change creates a similar `Finding` type (and related types) to describe the "lint findings" that tree-based rules, the pretty-printer, and the whitespace linter encounter during formatting/linting. This `Finding` type is the currency type for these kinds of findings/diagnostics emitted by the formatter API layer. The `swift-format` frontend tool now adopts `DiagnosticsEngine` from https://github.com/apple/swift-tools-support-core to manage and print these, but other clients using the API could use something different entirely. Since the usage of `TSC` is limited to the executable, API users don't have to take on a large and mostly unrelated dependency. There are still some opportunities for renaming here -- mainly replacing instances of `diagnose` in the internal code with `emitFinding` or something similar -- but I'll leave that as future cleanup.
dylansturg
pushed a commit
to dylansturg/swift-format
that referenced
this pull request
Apr 16, 2022
This change addresses swiftlang/swift-syntax#309 and swiftlang/swift-syntax#325, which split `SwiftSyntaxParser` into its own module and then removed the `DiagnosticEngine` APIs (so only the `Diagnostic` type remains as the way diagnostics from the parser are sent back to the caller). Rather than remain coupled to swift-syntax's `Diagnostic` type, this change creates a similar `Finding` type (and related types) to describe the "lint findings" that tree-based rules, the pretty-printer, and the whitespace linter encounter during formatting/linting. This `Finding` type is the currency type for these kinds of findings/diagnostics emitted by the formatter API layer. The `swift-format` frontend tool now adopts `DiagnosticsEngine` from https://github.com/apple/swift-tools-support-core to manage and print these, but other clients using the API could use something different entirely. Since the usage of `TSC` is limited to the executable, API users don't have to take on a large and mostly unrelated dependency. There are still some opportunities for renaming here -- mainly replacing instances of `diagnose` in the internal code with `emitFinding` or something similar -- but I'll leave that as future cleanup.
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.
When SwiftSyntax is just being used to generate code and not to parse Swift source files, there is no reason that a compatible
_InternalSwiftSyntaxParser.dylibneeds to be present. We do, however, currently have the requirement of always needing a_InternalSwiftSyntaxParser.dylibbecauseSyntaxParserlives inside theSwiftSyntaxmodule and thusSwiftSyntaxneeds to link against_InternalSwiftSyntaxParser.dylib.To lift the requirement, split
SwiftSyntaxinto two modules:SwiftSyntaxhas no requirement on_InternalSwiftSyntaxParser.dylib. It only offers functionality to programatically generateSwiftSyntaxtrees and walk them. The newSwiftSyntaxParsermodule links against_InternalSwiftSyntaxParser.dyliband provides the ability to generateSwiftSyntaxtrees from source files.To efficiently generate
SwiftSyntaxnodes from the parser,RawSyntax(and its child types) does depend on the layout of the C nodes (for example,TokenDatastoresCTriviaPieces as trailing trivia and only constructs the trivia’s Swift representation when requested). I don’t think it’s possible to eliminate that behaviour without eager translation and thus decided to include a copy of the C nodes inside theSwiftSyntax. WhenSwiftSyntaxis used on its own, they are never used and thus their value is irrelevant. When a source file is parsed throughSwiftSyntaxParser, it first verifies that the C node layouts match those defined in_InternalSwiftSyntaxParser.dylib, similar to how we currently compute the node hash. If the layouts are not compatible, we throw an error. We also verify that the files definingt these types match when invokingbuild-script(e.g. in CI).rdar://81158771