-
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.
Implement rules around star expressions with different precedence (#1…
…0623) ## Summary This PR adds implementations around the following grammar rules to support the other expression and statement parsing. This PR doesn't update the related expressions and statements as they will be done in their own PRs. * `star_expressions` * `star_expression` * `'*' bitwise_or` ### Problem There are two grammar entries around star expressions: ``` star_expression: | '*' bitwise_or | expression starred_expression: '*' expression ``` The main difference is the precedence at which the expression should be parsed if a `*` token is encountered. Currently, we parse both rules at the same precedence which makes the parser more lenient. There are various expressions and statements which uses either one of the above rules. ### Solutions #### Current solution Add the following new methods to the parser: 1. `parse_star_expression_list` which would be similar to `parse_expression_list` and would match the `star_expressions` grammar rule 2. `parse_star_expression_or_higher` which would match the `star_expression` or `star_named_expression` grammar rule 3. `parse_expression_with_bitwise_or_precedence` which would match the `bitwise_or` grammar rule Here, we could possibly merge the `parse_expression_list` and `parse_star_expression_list` into a single function as it only differs in the method it delegates the parsing to. The parameter could be called `allow_star_expression`. It could be confusing because even if it’s false, we would allow starred expression but at a higher binding power. This solution closely resembles the actual grammar and is easier to follow in the parser code. It doesn't require a lot of verification by the caller as the precedence parser makes sure to not allow that. #### Other solution Add a new `verify_bitwise_or_precedence` method which reports an error if the grammar expected a starred expression with `bitwise_or` precedence: ```rs pub(super) fn verify_star_expressions( &mut self, expr: &Expr, allow_named_expression: AllowNamedExpression, ) { match expr { Expr::Tuple(ast::ExprTuple { elts, .. }) => { for expr in elts { self.verify_star_expressions(expr, allow_named_expression); } } Expr::Starred(ast::ExprStarred { value, .. }) => { let expr_kind = match &**value { Expr::Compare(_) => "comparison", Expr::UnaryOp(ast::ExprUnaryOp { op: ast::UnaryOp::Not, .. }) => "unary `not`", Expr::BoolOp(_) => "boolean", Expr::If(_) => "`if-else`", Expr::Lambda(_) => "lambda", Expr::Named(_) if allow_named_expression.is_no() => "named", _ => return, }; self.add_error( ParseErrorType::OtherError(format!( "{expr_kind} expression cannot be used here", )), expr.range(), ); } _ => {} } } ``` For this solution, the parser will need to verify the precedence of a starred expression wherever grammar expected the `star_expressions`, `star_expression` or `star_named_expression` rule. We could possibly adopt this solution for better error recovery but I think for now having a strict parser is better suited because we don't really know what complete error recovery looks like. ### Follow-up There's a related follow-up issue which I'm working on and is related to starred expressions. The starred expression is only allowed in certain expressions and context which the parser needs to make sure and report an error if found otherwise. ## Test Plan Tested this locally by changing various expression and statement to use these methods. As mentioned they will be in their own PR.
- Loading branch information
1 parent
9b98f52
commit d5bb50d
Showing
3 changed files
with
159 additions
and
22 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
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