Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Parse an if followed by a tuple as a block #1924

Merged
merged 2 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
23 changes: 22 additions & 1 deletion crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
})
}

/// 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)> {
Expand Down 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