Draft
Conversation
The astute reader will note that we seem to _lose_ some node types in the process. Apparently, these were unreachable in the grammar, and the newer version of tree-sitter removes such "dead code".
This caused any field containing an _expression to appear as if it could countain any number of such nodes. It also threw away the information that there was a `?` marker there. To fix it, we simply move the definition into its own named node.
We make _referenceable_operator a named node. This prevents it from bleeding through to the _expression definition. It likely also makes the output easier to deal with, as bare operators used as arguments now have a named node wrapping them in the AST. Also removes a duplicated inclusion of _comparison_operator that served no purpose.
Before, the `condition` field of an if statement supposedly could contain things like parentheses and commas, due to bleeding from referenced anonymous nodes. Making the node named makes this issue go away.
Supertypes are a honking great idea. We should use more of them. This massively cleans up the node types, without polluting the AST with `expression` nodes.
Gets rid of a bunch of ad-hoc node type unions.
You know the drill. We just make an anonymous node named instead. In this case, however, we have to be a bit more clever about how to rewrite it. We turn the sequence of a type followed by an optional ! into a _choice_ between mere type or type followed by bang (the latter being our new named node).
Adds a new type `nested_type_identifier`, which contains the choice-branch that previously allowed those tokens to bleed through into the closest parent field.
Because `_type` was anonymous, its body was inlined in all of the places it appeared. Because this body contained a `name` field, this field was _also_ inlined. This caused a bunch of nodes to have spurious `name` fields, and for some of them (that already had such a field) it caused that field have multiplicity greater than one. To fix this, we make the `_type` node named, which prevents the errant field from escaping.
Same pattern we've seen many times before: a field on an anonymous node gets attached to the parent node instead. I'm not 100% sure this is the right solution, but it seemed wrong to just make `_parenthesized_type` named instead (we don't usually name parentheticals). At the very least, this cleans up the spurious navigation_expression.element and tuple_type_item.element fields.
Same procedure as before -- we change the anonymous node to a named node, and the problem magically goes away.
Hides a bunch of huge unions under (hopefully) sensible supertypes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| /** Gets the node corresponding to the field `element`. */ | ||
| final AstNode getElement(int i) { swift_array_literal_element(this, i, result) } | ||
| final Expression getElement(int i) { swift_array_literal_element(this, i, result) } |
|
|
||
| /** Gets the node corresponding to the field `raw_value`. */ | ||
| final AstNode getRawValue(int i) { swift_enum_entry_raw_value(this, i, result) } | ||
| final Expression getRawValue(int i) { swift_enum_entry_raw_value(this, i, result) } |
|
|
||
| /** Gets the node corresponding to the field `default_value`. */ | ||
| final AstNode getDefaultValue(int i) { | ||
| final Expression getDefaultValue(int i) { |
|
|
||
| /** Gets the node corresponding to the field `name`. */ | ||
| final AstNode getName(int i) { swift_guard_statement_name(this, i, result) } | ||
| final IfCondition getCondition(int i) { swift_guard_statement_condition(this, i, result) } |
| final override string getAPrimaryQlClass() { result = "IfStatement" } | ||
|
|
||
| /** Gets the node corresponding to the field `condition`. */ | ||
| final IfCondition getCondition(int i) { swift_if_statement_condition(this, i, result) } |
|
|
||
| /** Gets the node corresponding to the field `default_value`. */ | ||
| final AstNode getDefaultValue(int i) { swift_macro_declaration_default_value(this, i, result) } | ||
| final Expression getDefaultValue(int i) { |
|
|
||
| /** Gets the node corresponding to the field `default_value`. */ | ||
| final AstNode getDefaultValue(int i) { | ||
| final Expression getDefaultValue(int i) { |
|
|
||
| /** Gets the node corresponding to the field `name`. */ | ||
| final AstNode getName(int i) { swift_repeat_while_statement_name(this, i, result) } | ||
| final IfCondition getCondition(int i) { |
|
|
||
| /** Gets the node corresponding to the field `default_value`. */ | ||
| final AstNode getDefaultValue(int i) { | ||
| final Expression getDefaultValue(int i) { |
|
|
||
| /** Gets the node corresponding to the field `name`. */ | ||
| final AstNode getName(int i) { swift_while_statement_name(this, i, result) } | ||
| final IfCondition getCondition(int i) { swift_while_statement_condition(this, i, result) } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A lot of commits, but most of the churn is just files being regenerated. This only happens in separate commits, so those commits do not need to be reviewed.