Skip to content

Commit

Permalink
fix: Parse an if followed by a tuple as a block (#1924)
Browse files Browse the repository at this point in the history
* Fix parsing an if followed by a tuple as a function call

* Fix test
  • Loading branch information
jfecher authored Jul 13, 2023
1 parent a8ac338 commit 8df4f05
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
13 changes: 11 additions & 2 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,17 @@ impl Expression {
}

pub fn call(lhs: Expression, arguments: Vec<Expression>, 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)
}
}
Expand Down
21 changes: 21 additions & 0 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,19 @@ mod test {
fn parse_block() {
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()),
vec![
Expand All @@ -1544,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() {
Expand Down

0 comments on commit 8df4f05

Please sign in to comment.