From b6f486049015e282f8aea98a470179d6e1b905e8 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 13 Jul 2023 10:35:13 -0500 Subject: [PATCH 1/2] Fix parsing an if followed by a tuple as a function call --- crates/noirc_frontend/src/ast/expression.rs | 13 +++++++++++-- crates/noirc_frontend/src/parser/parser.rs | 11 +++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 0b8cc9b575f..b1cd30a6777 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -158,8 +158,17 @@ impl Expression { } pub fn call(lhs: Expression, arguments: Vec, span: Span) -> Expression { - let func = Box::new(lhs); - let kind = ExpressionKind::Call(Box::new(CallExpression { func, arguments })); + // Need to check if lhs is an if expression since users can sequence if expressions + // with tuples without calling them. E.g. `if c { t } else { e }(a, b)` is interpreted + // as a sequence of { if, tuple } rather than a function call. This behavior matches rust. + let kind = if matches!(&lhs.kind, ExpressionKind::If(..)) { + ExpressionKind::Block(BlockExpression(vec![ + Statement::Expression(lhs), + Statement::Expression(Expression::new(ExpressionKind::Tuple(arguments), span)), + ])) + } else { + ExpressionKind::Call(Box::new(CallExpression { func: Box::new(lhs), arguments })) + }; Expression::new(kind, span) } } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 85260cdb202..3bed6568da5 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -188,7 +188,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser { }) } -/// function_modifiers: 'unconstrained'? 'open'? 'internal'? +/// function_modifiers: 'unconstrained'? 'open'? 'internal'? /// /// returns (is_unconstrained, is_open, is_internal) for whether each keyword was present fn function_modifiers() -> impl NoirParser<(bool, bool, bool)> { @@ -1529,7 +1529,14 @@ mod test { #[test] fn parse_block() { - parse_with(block(expression()), "{ [0,1,2,3,4] }").unwrap(); + parse_all( + block(expression()), + vec![ + "{ [0,1,2,3,4] }", + // Regression for #1310: this should be parsed as a block and not a function call + "{ if true { 1 } else { 2 } (3, 4) }", + ], + ); parse_all_failing( block(expression()), From 99d9233876d88c867312cf504dc20c7663aeefdc Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 13 Jul 2023 11:02:23 -0500 Subject: [PATCH 2/2] Fix test --- crates/noirc_frontend/src/parser/parser.rs | 30 ++++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 3bed6568da5..3f9fe31f414 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -1529,14 +1529,20 @@ mod test { #[test] fn parse_block() { - parse_all( - block(expression()), - vec![ - "{ [0,1,2,3,4] }", - // Regression for #1310: this should be parsed as a block and not a function call - "{ if true { 1 } else { 2 } (3, 4) }", - ], - ); + parse_with(block(expression()), "{ [0,1,2,3,4] }").unwrap(); + + // Regression for #1310: this should be parsed as a block and not a function call + let res = parse_with(block(expression()), "{ if true { 1 } else { 2 } (3, 4) }").unwrap(); + match unwrap_expr(res.0.last().unwrap()) { + // The `if` followed by a tuple is currently creates a block around both in case + // there was none to start with, so there is an extra block here. + ExpressionKind::Block(block) => { + assert_eq!(block.0.len(), 2); + assert!(matches!(unwrap_expr(&block.0[0]), ExpressionKind::If(_))); + assert!(matches!(unwrap_expr(&block.0[1]), ExpressionKind::Tuple(_))); + } + _ => unreachable!(), + } parse_all_failing( block(expression()), @@ -1551,6 +1557,14 @@ mod test { ); } + /// Extract an Statement::Expression from a statement or panic + fn unwrap_expr(stmt: &Statement) -> &ExpressionKind { + match stmt { + Statement::Expression(expr) => &expr.kind, + _ => unreachable!(), + } + } + /// Deprecated constrain usage test #[test] fn parse_constrain() {