diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 633e38a0e0a..e176c7fc8b4 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -263,29 +263,9 @@ impl Expression { arguments: Vec, span: Span, ) -> Expression { - // 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 { - statements: vec![ - Statement { kind: StatementKind::Expression(lhs), span }, - Statement { - kind: StatementKind::Expression(Expression::new( - ExpressionKind::Tuple(arguments), - span, - )), - span, - }, - ], - }) - } else { - ExpressionKind::Call(Box::new(CallExpression { - func: Box::new(lhs), - is_macro_call, - arguments, - })) - }; + let func = Box::new(lhs); + let kind = + ExpressionKind::Call(Box::new(CallExpression { func, is_macro_call, arguments })); Expression::new(kind, span) } } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index a7c62048283..9772814027f 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -466,6 +466,8 @@ where assertion::assertion_eq(expr_parser.clone()), declaration(expr_parser.clone()), assignment(expr_parser.clone()), + if_statement(expr_no_constructors.clone(), statement.clone()), + block_statement(statement.clone()), for_loop(expr_no_constructors.clone(), statement.clone()), break_statement(), continue_statement(), @@ -932,6 +934,28 @@ where }) } +fn if_statement<'a, P, S>( + expr_no_constructors: P, + statement: S, +) -> impl NoirParser + 'a +where + P: ExprParser + 'a, + S: NoirParser + 'a, +{ + if_expr(expr_no_constructors, statement).map_with_span(|expression_kind, span| { + StatementKind::Expression(Expression::new(expression_kind, span)) + }) +} + +fn block_statement<'a, S>(statement: S) -> impl NoirParser + 'a +where + S: NoirParser + 'a, +{ + block(statement).map_with_span(|block, span| { + StatementKind::Expression(Expression::new(ExpressionKind::Block(block), span)) + }) +} + fn for_loop<'a, P, S>(expr_no_constructors: P, statement: S) -> impl NoirParser + 'a where P: ExprParser + 'a, @@ -1298,20 +1322,6 @@ mod test { fn parse_block() { parse_with(block(fresh_statement()), "{ [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(fresh_statement()), "{ if true { 1 } else { 2 } (3, 4) }").unwrap(); - match unwrap_expr(&res.statements.last().unwrap().kind) { - // 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.statements.len(), 2); - assert!(matches!(unwrap_expr(&block.statements[0].kind), ExpressionKind::If(_))); - assert!(matches!(unwrap_expr(&block.statements[1].kind), ExpressionKind::Tuple(_))); - } - _ => unreachable!(), - } - parse_all_failing( block(fresh_statement()), vec![ @@ -1325,14 +1335,6 @@ mod test { ); } - /// Extract an Statement::Expression from a statement or panic - fn unwrap_expr(stmt: &StatementKind) -> &ExpressionKind { - match stmt { - StatementKind::Expression(expr) => &expr.kind, - _ => unreachable!(), - } - } - #[test] fn parse_let() { // Why is it valid to specify a let declaration as having type u8? @@ -1629,4 +1631,40 @@ mod test { let failing = vec!["quote {}}", "quote a", "quote { { { } } } }"]; parse_all_failing(quote(), failing); } + + #[test] + fn test_parses_block_statement_not_infix_expression() { + let src = r#" + { + {} + -1 + }"#; + let (block_expr, _) = parse_recover(block(fresh_statement()), src); + let block_expr = block_expr.expect("Failed to parse module"); + assert_eq!(block_expr.statements.len(), 2); + } + + #[test] + fn test_parses_if_statement_not_infix_expression() { + let src = r#" + { + if 1 { 2 } else { 3 } + -1 + }"#; + let (block_expr, _) = parse_recover(block(fresh_statement()), src); + let block_expr = block_expr.expect("Failed to parse module"); + assert_eq!(block_expr.statements.len(), 2); + } + + #[test] + fn test_parses_if_statement_followed_by_tuple_as_two_separate_statements() { + // Regression for #1310: this should not be parsed as a function call + let src = r#" + { + if 1 { 2 } else { 3 } (1, 2) + }"#; + let (block_expr, _) = parse_recover(block(fresh_statement()), src); + let block_expr = block_expr.expect("Failed to parse module"); + assert_eq!(block_expr.statements.len(), 2); + } } diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index e99818bafa0..7b75cf4cae4 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -100,14 +100,14 @@ impl U128 { } fn decode_ascii(ascii: u8) -> Field { - if ascii < 58 { + (if ascii < 58 { ascii - 48 } else { let ascii = ascii + 32 * (U128::uconstrained_check_is_upper_ascii(ascii) as u8); assert(ascii >= 97); // enforce >= 'a' assert(ascii <= 102); // enforce <= 'f' ascii - 87 - } as Field + }) as Field } // TODO: Replace with a faster version. diff --git a/tooling/nargo_fmt/tests/expected/expr.nr b/tooling/nargo_fmt/tests/expected/expr.nr index 03a26835ee3..babaf5b356e 100644 --- a/tooling/nargo_fmt/tests/expected/expr.nr +++ b/tooling/nargo_fmt/tests/expected/expr.nr @@ -129,9 +129,9 @@ fn return_if_expr() { } fn if_if() { - if cond { + (if cond { some(); } else { none(); - }.bar().baz(); + }).bar().baz(); } diff --git a/tooling/nargo_fmt/tests/input/expr.nr b/tooling/nargo_fmt/tests/input/expr.nr index b4edcbbed5f..9ecefad7dfd 100644 --- a/tooling/nargo_fmt/tests/input/expr.nr +++ b/tooling/nargo_fmt/tests/input/expr.nr @@ -147,7 +147,7 @@ fn return_if_expr() { } fn if_if() { -if cond { some(); } else { none(); } +(if cond { some(); } else { none(); }) .bar() .baz(); } \ No newline at end of file