fix!: parse block and if statements independently of expressions in statements

compiler/noirc_frontend/src/ast/expression.rs

@@ -263,29 +263,9 @@ impl Expression {
         arguments: Vec<Expression>,
         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)
     }
 }

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<StatementKind> + 'a
+where
+    P: ExprParser + 'a,
+    S: NoirParser<StatementKind> + '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<StatementKind> + 'a
+where
+    S: NoirParser<StatementKind> + '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<StatementKind> + '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);
+    }
 }

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. 

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();
 }

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();
 }