-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Avoid error handling duplication for starred, yield, lambda expressio…
…ns (#10809) ## Summary This PR updates the error handling logic for certain expressions in a way to either perform it automatically or provide an option for the user. The expression in discussion here are `lambda`, starred and `yield` expression. ### Problem The current parser allows these expressions at arbitrary context. This is because the mentioned expressions are parsed using `parse_lhs_expression` which is part of other higher level grammar rules. This means that the caller needs to validate the parsed expression and report an error if it isn't allowed in that context. This can get quite cumbersome to do so as it needs to be done for all of the call sites for following methods: 1. `parse_expression_list`: 14 references 2. `parse_star_expression_list`: 4 references 3. `parse_star_expression_or_higher`: 8 references 4. `parse_named_expression_or_higher`: 10 references 5. `parse_conditional_expression_or_higher`: 25 references 6. `parse_simple_expression`: 4 references The numbers corresponding to the methods are the number of references as of today. This list is also in the correct hierarchy of grammar precedence. For example, `parse_expression_list` calls into `parse_conditional_expression_or_higher` but not the other way around. ### Solution We'll take the above expression one at a time to understand the solution: #### Lambda expression Lambda expressions are only allowed in `expression` grammar rule which corresponds to `parse_conditional_expression_or_higher`. This means that this expression is only allowed when using either of the following functions: 1. `parse_expression_list` 2. `parse_star_expression_list` 3. `parse_star_expression_or_higher` 4. `parse_named_expression_or_higher` 5. `parse_conditional_expression_or_higher` The solution is to move the error handling in `parse_simple_expression` and parameterize it where any of the above listed function would always use `AllowLambdaExpression::Yes`. #### Starred expression There are two grammar rules related to starred expression: 1. `star_expression` which corresponds to `parse_star_expression_or_higher` 2. `starred_expression` which is parsed in LHS parsing Remember that LHS parsing isn't accessed directly but only via any of the above listed functions in the problem section. Now, starred expressions are allowed in a lot of places but sometimes in a limited capacity. For example, an assignment target can have a starred expression but only if it is a name node (`*x`). The solution here is to adopt the one used in star pattern matching which is to use a parameter. The following functions are parameterized: 1. `parse_expression_list` 2. `parse_named_expression_or_higher` 3. `parse_conditional_expression_or_higher` Now, `parse_star_expression_list` and `parse_star_expression_or_higher` aren't parameterized because they handle the `star_expression` grammar which means that the caller wants to parse a starred expression but with a limited precedence. #### Yield expression Yield expressions are only allowed in the following context: 1. Top level as yield statement 2. Parenthesized 3. F-string expression 4. Assignment (including annotated and augmented) value We could parameterize it similar to starred expression but that seems like a waste given the limited number of locations they're allowed. The solution is to add a `parse_yield_expression_or_else` method which parses a yield expression if the parser is at `yield` token or else calls the given method to parse the expression. The call site would like: ```rs // (yield_expr | named_expression) self.try_parse_yield_expression() .unwrap_or_else(|| self.parse_named_expression_or_higher()) // (yield_expr | star_expressions) self.try_parse_yield_expression() .unwrap_or_else(|| self.parse_star_expression_list()) ``` An added benefit for this is that the call site looks exactly like the grammar. ## Review * The reviewer would mainly just look at the de-duplication logic. * The reviewer doesn't really need to verify the call sites as they're verified by existing test cases. For nodes which aren't yet tested, they will be done so in their own PR. ## Test Plan Run existing test cases and verify the snapshot updates. Additional test cases will be added when working on specific nodes.
- Loading branch information
1 parent
a585856
commit 7673a5a
Showing
20 changed files
with
330 additions
and
319 deletions.
There are no files selected for viewing
This file contains 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
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.