From b20571ad0fb4b6a0d60fa146751d238cda8df527 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Sun, 18 Dec 2022 17:32:31 -0800 Subject: [PATCH] Simplify parenthesized context manager parsing with LALRPOP conditions Signed-off-by: Anders Kaseorg --- parser/python.lalrpop | 313 ++++++++++++++++++++---------------------- parser/src/with.rs | 134 ------------------ 2 files changed, 149 insertions(+), 298 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index c1f6ee0..f586f0a 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -11,7 +11,6 @@ use crate::{ context::set_context, string::parse_strings, token::StringKind, - with::{ExprOrWithitems, TupleOrWithitems}, }; use num_bigint::BigInt; @@ -129,7 +128,7 @@ ExpressionStatement: ast::Stmt = { }, } }, - ":" => { + > ":" > => { let simple = matches!(target.node, ast::ExprKind::Name { .. }); ast::Stmt { custom: (), @@ -161,7 +160,7 @@ TestOrStarExprList: ast::Expr = { }; TestOrStarExpr: ast::Expr = { - Test, + Test<"all">, StarExpr, }; @@ -170,12 +169,6 @@ TestOrStarNamedExpr: ast::Expr = { StarExpr, }; -TestOrStarNamedExprOrWithitem: (ast::Expr, Option>) = { - => (e, None), - => (e, None), - "as" => (e, Some(Box::new(v))), -} - AugAssign: ast::Operator = { "+=" => ast::Operator::Add, "-=" => ast::Operator::Sub, @@ -237,7 +230,7 @@ RaiseStatement: ast::Stmt = { node: ast::StmtKind::Raise { exc: None, cause: None }, } }, - "raise" => { + "raise" > )?> => { ast::Stmt { custom: (), location, @@ -336,7 +329,7 @@ NonlocalStatement: ast::Stmt = { }; AssertStatement: ast::Stmt = { - "assert" => { + "assert" > )?> => { ast::Stmt { custom: (), location, @@ -473,7 +466,7 @@ TryStatement: ast::Stmt = { }; ExceptClause: ast::Excepthandler = { - "except" ":" => { + "except" ?> ":" => { let end_location = body.last().unwrap().end_location.unwrap(); ast::Excepthandler::new( location, @@ -485,7 +478,7 @@ ExceptClause: ast::Excepthandler = { }, ) }, - "except" ":" => { + "except" "as" Identifier)> ":" => { let end_location = body.last().unwrap().end_location.unwrap(); ast::Excepthandler::new( location, @@ -512,34 +505,34 @@ WithStatement: ast::Stmt = { }, }; -// These are only used for their types as macro parameters -ExprGoal: ast::Expr = {}; -ExprOrWithitemsGoal: ExprOrWithitems = {}; - WithItems: Vec = { - > =>? items.try_into(), - > "as" =>? { - let optional_vars = Some(Box::new(set_context(vars, ast::ExprContext::Store))); - let context_expr = first.try_into()?; - Ok(vec![ast::Withitem { context_expr, optional_vars }]) + "(" ","? ")", + "(" ",")?> > >)*> ","? ")" => { + left.into_iter().flatten().chain([mid]).chain(right).collect() }, - > "," > =>? { - let optional_vars = n.map(|val| Box::new(set_context(val.1, ast::ExprContext::Store))); - let context_expr = first.try_into()?; - items.insert(0, ast::Withitem { context_expr, optional_vars }); - Ok(items) + > => vec![<>], + > >)+> => { + [item].into_iter().chain(items).collect() } }; -WithItem: ast::Withitem = { - => { - let optional_vars = n.map(|val| Box::new(set_context(val.1, ast::ExprContext::Store))); +#[inline] +WithItemsNoAs: Vec = { + > => { + <>.into_iter().map(|context_expr| ast::Withitem { context_expr, optional_vars: None }).collect() + }, +} + +WithItem: ast::Withitem = { + > if Goal != "as" => ast::Withitem { context_expr: <>, optional_vars: None }, + > "as" > => { + let optional_vars = Some(Box::new(set_context(vars, ast::ExprContext::Store))); ast::Withitem { context_expr, optional_vars } }, }; FuncDef: ast::Stmt = { - "def" " Test)?> ":" => { + "def" " Test<"all">)?> ":" => { let args = Box::new(args); let returns = r.map(|x| Box::new(x.1)); let end_location = body.last().unwrap().end_location.unwrap(); @@ -643,7 +636,7 @@ ParameterDefs: (Vec<(ast::Arg, Option)>, Vec<(ast::Arg, Opti ParameterDef: (ast::Arg, Option) = { => (i, None), - "=" => (i, Some(e)), + "=" > => (i, Some(e)), }; UntypedParameter: ast::Arg = { @@ -655,7 +648,7 @@ UntypedParameter: ast::Arg = { }; TypedParameter: ast::Arg = { - => { + )?> => { let annotation = a.map(|x| Box::new(x.1)); ast::Arg::new(location, end_location, ast::ArgData { arg, annotation, type_comment: None }) }, @@ -729,7 +722,7 @@ YieldExpr: ast::Expr = { custom: (), node: ast::ExprKind::Yield { value: value.map(Box::new) } }, - "yield" "from" => ast::Expr { + "yield" "from" > => ast::Expr { location, end_location: Some(end_location), custom: (), @@ -737,9 +730,8 @@ YieldExpr: ast::Expr = { }, }; -Test = TestAs; -TestAs: Goal = { - "if" "else" => ast::Expr { +Test: ast::Expr = { + > "if" > "else" > => ast::Expr { location, end_location: Some(end_location), custom: (), @@ -748,13 +740,13 @@ TestAs: Goal = { body: Box::new(body), orelse: Box::new(orelse), } - }.into(), - OrTestAs, - => e.into(), + }, + OrTest, + LambdaDef, }; NamedExpressionTest: ast::Expr = { - > => { if let Some(l) = left { ast::Expr { location, @@ -776,7 +768,7 @@ NamedExpressionTest: ast::Expr = { } LambdaDef: ast::Expr = { - "lambda" ?> ":" => { + "lambda" ?> ":" > => { let p = p.unwrap_or_else(|| { ast::Arguments { posonlyargs: vec![], @@ -800,9 +792,8 @@ LambdaDef: ast::Expr = { } } -OrTest = OrTestAs; -OrTestAs: Goal = { - => { +OrTest: ast::Expr = { + > )+> => { let mut values = vec![e1]; values.extend(e2.into_iter().map(|e| e.1)); ast::Expr { @@ -810,14 +801,13 @@ OrTestAs: Goal = { end_location: Some(end_location), custom: (), node: ast::ExprKind::BoolOp { op: ast::Boolop::Or, values } - }.into() + } }, - AndTestAs, + AndTest, }; -AndTest = AndTestAs; -AndTestAs: Goal = { - => { +AndTest: ast::Expr = { + > )+> => { let mut values = vec![e1]; values.extend(e2.into_iter().map(|e| e.1)); ast::Expr { @@ -825,34 +815,32 @@ AndTestAs: Goal = { end_location: Some(end_location), custom: (), node: ast::ExprKind::BoolOp { op: ast::Boolop::And, values } - }.into() + } }, - NotTestAs, + NotTest, }; -NotTest = NotTestAs; -NotTestAs: Goal = { - "not" => ast::Expr { +NotTest: ast::Expr = { + "not" > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::UnaryOp { operand: Box::new(e), op: ast::Unaryop::Not } - }.into(), - ComparisonAs, + }, + Comparison, }; -Comparison = ComparisonAs; -ComparisonAs: Goal = { - => { +Comparison: ast::Expr = { + > )+> => { let (ops, comparators) = comparisons.into_iter().unzip(); ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Compare { left: Box::new(left), ops, comparators } - }.into() + } }, - ExpressionAs, + Expression, }; CompOp: ast::Cmpop = { @@ -868,48 +856,44 @@ CompOp: ast::Cmpop = { "is" "not" => ast::Cmpop::IsNot, }; -Expression = ExpressionAs; -ExpressionAs: Goal = { - "|" => ast::Expr { +Expression: ast::Expr = { + > "|" > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(e1), op: ast::Operator::BitOr, right: Box::new(e2) } - }.into(), - XorExpressionAs, + }, + XorExpression, }; -XorExpression = XorExpressionAs; -XorExpressionAs: Goal = { - "^" => ast::Expr { +XorExpression: ast::Expr = { + > "^" > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(e1), op: ast::Operator::BitXor, right: Box::new(e2) } - }.into(), - AndExpressionAs, + }, + AndExpression, }; -AndExpression = AndExpressionAs; -AndExpressionAs: Goal = { - "&" => ast::Expr { +AndExpression: ast::Expr = { + > "&" > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(e1), op: ast::Operator::BitAnd, right: Box::new(e2) } - }.into(), - ShiftExpressionAs, + }, + ShiftExpression, }; -ShiftExpression = ShiftExpressionAs; -ShiftExpressionAs: Goal = { - => ast::Expr { +ShiftExpression: ast::Expr = { + > > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(e1), op, right: Box::new(e2) } - }.into(), - ArithmeticExpressionAs, + }, + ArithmeticExpression, }; ShiftOp: ast::Operator = { @@ -917,15 +901,14 @@ ShiftOp: ast::Operator = { ">>" => ast::Operator::RShift, }; -ArithmeticExpression = ArithmeticExpressionAs; -ArithmeticExpressionAs: Goal = { - => ast::Expr { +ArithmeticExpression: ast::Expr = { + > > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(a), op, right: Box::new(b) } - }.into(), - TermAs, + }, + Term, }; AddOp: ast::Operator = { @@ -933,15 +916,14 @@ AddOp: ast::Operator = { "-" => ast::Operator::Sub, }; -Term = TermAs; -TermAs: Goal = { - => ast::Expr { +Term: ast::Expr = { + > > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(a), op, right: Box::new(b) } - }.into(), - FactorAs, + }, + Factor, }; MulOp: ast::Operator = { @@ -952,15 +934,14 @@ MulOp: ast::Operator = { "@" => ast::Operator::MatMult, }; -Factor = FactorAs; -FactorAs: Goal = { - => ast::Expr { +Factor: ast::Expr = { + > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::UnaryOp { operand: Box::new(e), op } - }.into(), - PowerAs, + }, + Power, }; UnaryOp: ast::Unaryop = { @@ -969,53 +950,50 @@ UnaryOp: ast::Unaryop = { "~" => ast::Unaryop::Invert, }; -Power = PowerAs; -PowerAs: Goal = { - "**" => ast::Expr { +Power: ast::Expr = { + > "**" > => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::BinOp { left: Box::new(e), op: ast::Operator::Pow, right: Box::new(b) } - }.into(), - AtomExprAs, + }, + AtomExpr, }; -AtomExpr = AtomExprAs; -AtomExprAs: Goal = { - "await" => { +AtomExpr: ast::Expr = { + "await" > => { ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Await { value: Box::new(atom) } - }.into() + } }, - AtomExpr2As, + AtomExpr2, } -AtomExpr2 = AtomExpr2As; -AtomExpr2As: Goal = { - AtomAs, - "(" ")" => { +AtomExpr2: ast::Expr = { + Atom, + > "(" ")" => { ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Call { func: Box::new(f), args: a.args, keywords: a.keywords } - }.into() + } }, - "[" "]" => ast::Expr { + > "[" "]" => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Subscript { value: Box::new(e), slice: Box::new(s), ctx: ast::ExprContext::Load } - }.into(), - "." => ast::Expr { + }, + > "." => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Attribute { value: Box::new(e), attr, ctx: ast::ExprContext::Load } - }.into(), + }, }; SubscriptList: ast::Expr = { @@ -1039,8 +1017,8 @@ SubscriptList: ast::Expr = { }; Subscript: ast::Expr = { - Test, - ":" => { + Test<"all">, + ?> ":" ?> => { let lower = e1.map(Box::new); let upper = e2.map(Box::new); let step = e3.flatten().map(Box::new); @@ -1054,24 +1032,23 @@ Subscript: ast::Expr = { }; SliceOp: Option = { - ":" => e, + ":" ?> => e, } -Atom = AtomAs; -AtomAs: Goal = { - =>? Ok(parse_strings(s)?.into()), +Atom: ast::Expr = { + =>? Ok(parse_strings(s)?), => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Constant { value, kind: None } - }.into(), + }, => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Name { id: name, ctx: ast::ExprContext::Load } - }.into(), + }, "[" "]" => { let elts = e.unwrap_or_default(); ast::Expr { @@ -1079,7 +1056,7 @@ AtomAs: Goal = { end_location: Some(end_location), custom: (), node: ast::ExprKind::List { elts, ctx: ast::ExprContext::Load } - }.into() + } }, "[" "]" => { ast::Expr { @@ -1087,40 +1064,48 @@ AtomAs: Goal = { end_location: Some(end_location), custom: (), node: ast::ExprKind::ListComp { elt: Box::new(elt), generators } - }.into() - }, - "(" > ")" =>? { - if items.len() == 1 && items[0].1.is_none() && trailing_comma.is_none() { - match items[0].0.node { - ast::ExprKind::Starred { .. } => { - Err(LexicalError{ - error: LexicalErrorType::OtherError("cannot use starred expression here".to_string()), - location: items[0].0.location, - }.into()) - } - _ => { - Ok(items.into_iter().next().unwrap().0.into()) - } - } - } else { - TupleOrWithitems { location, end_location, items }.try_into() } }, + "(" > ")" if Goal != "no-withitems" => { + if elts.len() == 1 && trailing_comma.is_none() { + elts.into_iter().next().unwrap() + } else { + ast::Expr::new( + location, + end_location, + ast::ExprKind::Tuple { elts, ctx: ast::ExprContext::Load }, + ) + } + }, + "(" > ",")?> )*> ")" =>? { + if left.is_none() && right.is_empty() && trailing_comma.is_none() { + Err(LexicalError{ + error: LexicalErrorType::OtherError("cannot use starred expression here".to_string()), + location: mid.location, + })? + } + let elts = left.into_iter().flatten().chain([mid]).chain(right).collect(); + Ok(ast::Expr::new( + location, + end_location, + ast::ExprKind::Tuple { elts, ctx: ast::ExprContext::Load }, + )) + }, "(" ")" => ast::Expr::new( location, end_location, ast::ExprKind::Tuple { elts: Vec::new(), ctx: ast::ExprContext::Load } - ).into(), - "(" ")" => e.into(), + ), + "(" ")" => e, "(" ")" => { ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::GeneratorExp { elt: Box::new(elt), generators } - }.into() + } }, - "(" "**" ")" =>? { + "(" "**" > ")" =>? { Err(LexicalError{ error : LexicalErrorType::OtherError("cannot use double starred expression here".to_string()), location: location, @@ -1170,7 +1155,7 @@ AtomAs: Goal = { end_location: Some(end_location), custom: (), node: ast::ExprKind::Dict { keys, values } - }.into() + } }, "{" "}" => { ast::Expr { @@ -1182,26 +1167,26 @@ AtomAs: Goal = { value: Box::new(e1.1), generators, } - }.into() + } }, "{" "}" => ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::Set { elts } - }.into(), - "{" "}" => { + }, + "{" > "}" => { ast::Expr { location, end_location: Some(end_location), custom: (), node: ast::ExprKind::SetComp { elt: Box::new(elt), generators } - }.into() + } }, - "True" => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: true.into(), kind: None }).into(), - "False" => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: false.into(), kind: None }).into(), - "None" => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: ast::Constant::None, kind: None }).into(), - "..." => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: ast::Constant::Ellipsis, kind: None }).into(), + "True" => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: true.into(), kind: None }), + "False" => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: false.into(), kind: None }), + "None" => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: ast::Constant::None, kind: None }), + "..." => ast::Expr::new(location, end_location, ast::ExprKind::Constant { value: ast::Constant::Ellipsis, kind: None }), }; ListLiteralValues: Vec = { @@ -1213,12 +1198,12 @@ DictLiteralValues: Vec<(Option>, ast::Expr)> = { }; DictEntry: (ast::Expr, ast::Expr) = { - ":" => (e1, e2), + > ":" > => (e1, e2), }; DictElement: (Option>, ast::Expr) = { => (Some(Box::new(e.0)), e.1), - "**" => (None, e), + "**" > => (None, e), }; SetLiteralValues: Vec = { @@ -1226,7 +1211,7 @@ SetLiteralValues: Vec = { }; ExpressionOrStarExpression = { - Expression, + Expression<"all">, StarExpr }; @@ -1264,7 +1249,7 @@ GenericList: ast::Expr = { // Test StarExpr: ast::Expr = { - "*" => ast::Expr { + "*" > => ast::Expr { location, end_location: Some(end_location), custom: (), @@ -1276,7 +1261,7 @@ StarExpr: ast::Expr = { CompFor: Vec = => c; SingleForComprehension: ast::Comprehension = { - "for" "in" => { + "for" "in" > => { let is_async = is_async.is_some(); ast::Comprehension { target: set_context(target, ast::ExprContext::Store), @@ -1287,7 +1272,7 @@ SingleForComprehension: ast::Comprehension = { } }; -ExpressionNoCond: ast::Expr = OrTest; +ExpressionNoCond: ast::Expr = OrTest<"all">; ComprehensionIf: ast::Expr = "if" => c; ArgumentList: ArgumentList = { @@ -1313,8 +1298,8 @@ FunctionArgument: (Option<(ast::Location, ast::Location, Option)>, ast:: }; (None, expr) }, - "=" => (Some((location, end_location, Some(i))), e), - "*" => { + "=" > => (Some((location, end_location, Some(i))), e), + "*" > => { let expr = ast::Expr::new( location, end_location, @@ -1322,7 +1307,7 @@ FunctionArgument: (Option<(ast::Location, ast::Location, Option)>, ast:: ); (None, expr) }, - "**" => (Some((location, end_location, None)), e), + "**" > => (Some((location, end_location, None)), e), }; #[inline] diff --git a/parser/src/with.rs b/parser/src/with.rs index 5f018c6..b05fb37 100644 --- a/parser/src/with.rs +++ b/parser/src/with.rs @@ -1,137 +1,3 @@ -//! Intermediate types for `with` statement cover grammar. -//! -//! When we start parsing a `with` statement, we don't initially know -//! whether we're looking at a tuple or a Python 3.9+ parenthesized -//! collection of contexts: -//! -//! ```python -//! with (a, b, c) as t: # tuple -//! with (a, b, c): # withitems -//! ``` -//! -//! Since LALRPOP requires us to commit to an output type before we -//! have enough information to decide, we build a cover grammar that's -//! convertible either way. This module contains the necessary -//! intermediate data types. - -use crate::ast::{self, Location}; -use crate::context; -use crate::error::{LexicalError, LexicalErrorType}; -use crate::token::Tok; -use lalrpop_util::ParseError as LalrpopError; - -/// Represents a parenthesized collection that we might later convert -/// to a tuple or to `with` items. -/// -/// It can be converted to either `Expr` or `ExprOrWithitems` with -/// `.try_into()`. The `Expr` conversion will fail if any `as` -/// variables are present. The `ExprOrWithitems` conversion cannot -/// fail (but we need it to have the same interface so we can use -/// LALRPOP macros to declare the cover grammar without much code -/// duplication). -pub struct TupleOrWithitems { - pub location: Location, - pub end_location: Location, - pub items: Vec<(ast::Expr, Option>)>, -} - -impl TryFrom for ast::Expr { - type Error = LalrpopError; - fn try_from(tuple_or_withitems: TupleOrWithitems) -> Result { - Ok(ast::Expr { - location: tuple_or_withitems.location, - end_location: Some(tuple_or_withitems.end_location), - custom: (), - node: ast::ExprKind::Tuple { - elts: tuple_or_withitems - .items - .into_iter() - .map(|(expr, optional_vars)| { - if let Some(vars) = optional_vars { - Err(LexicalError { - error: LexicalErrorType::OtherError( - "cannot use 'as' here".to_string(), - ), - location: vars.location, - })? - } - Ok(expr) - }) - .collect::, Self::Error>>()?, - ctx: ast::ExprContext::Load, - }, - }) - } -} - -impl TryFrom for ExprOrWithitems { - type Error = LalrpopError; - fn try_from(items: TupleOrWithitems) -> Result { - Ok(ExprOrWithitems::TupleOrWithitems(items)) - } -} - -/// Represents either a non-tuple expression, or a parenthesized -/// collection that we might later convert to a tuple or to `with` -/// items. -/// -/// It can be constructed from an `Expr` with `.into()`. (The same -/// interface can be used to convert an `Expr` into itself, which is -/// also important for our LALRPOP macro setup.) -/// -/// It can be converted to either `Expr` or `Vec` with -/// `.try_into()`. The `Expr` conversion will fail if any `as` -/// clauses are present. The `Vec` conversion will fail if -/// both `as` clauses and starred expressions are present. -pub enum ExprOrWithitems { - Expr(ast::Expr), - TupleOrWithitems(TupleOrWithitems), -} - -impl From for ExprOrWithitems { - fn from(expr: ast::Expr) -> ExprOrWithitems { - ExprOrWithitems::Expr(expr) - } -} - -impl TryFrom for ast::Expr { - type Error = LalrpopError; - fn try_from(expr_or_withitems: ExprOrWithitems) -> Result { - match expr_or_withitems { - ExprOrWithitems::Expr(expr) => Ok(expr), - ExprOrWithitems::TupleOrWithitems(items) => items.try_into(), - } - } -} - -impl TryFrom for Vec { - type Error = LalrpopError; - fn try_from(expr_or_withitems: ExprOrWithitems) -> Result, Self::Error> { - match expr_or_withitems { - ExprOrWithitems::TupleOrWithitems(tuple_or_withitems) - if !tuple_or_withitems.items.iter().any(|(context_expr, _)| { - matches!(context_expr.node, ast::ExprKind::Starred { .. }) - }) => - { - Ok(tuple_or_withitems - .items - .into_iter() - .map(|(context_expr, optional_vars)| ast::Withitem { - context_expr, - optional_vars: optional_vars.map(|expr| { - Box::new(context::set_context(*expr, ast::ExprContext::Store)) - }), - }) - .collect()) - } - _ => Ok(vec![ast::Withitem { - context_expr: expr_or_withitems.try_into()?, - optional_vars: None, - }]), - } - } -} - #[cfg(test)] mod tests { use crate::parser::parse_program;