Skip to content

Commit

Permalink
fix: Allow constructors in parentheses in if conditions
Browse files Browse the repository at this point in the history 
and `for` ranges

Fixes #2996
  • Loading branch information
@pczarn
pczarn committed Oct 18, 2023
1 parent 4cb2024 commit d41c6db
Showing 1 changed file with 70 additions and 19 deletions.
89 changes: 70 additions & 19 deletions compiler/noirc_frontend/src/parser/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ where
}

fn fresh_statement() -> impl NoirParser<StatementKind> {
statement(expression(), expression_no_constructors())
statement(expression(), expression_no_constructors(expression()))
}

fn constrain<'a, P>(expr_parser: P) -> impl NoirParser<StatementKind> + 'a
Expand Down Expand Up @@ -1189,22 +1189,25 @@ fn expression() -> impl ExprParser {
expression_with_precedence(
Precedence::Lowest,
expr.clone(),
expression_no_constructors(),
statement(expr, expression_no_constructors()),
expression_no_constructors(expr.clone()),
statement(expr.clone(), expression_no_constructors(expr)),
false,
true,
)
})
.labelled(ParsingRuleLabel::Expression)
}

fn expression_no_constructors() -> impl ExprParser {
recursive(|expr| {
fn expression_no_constructors<'a, P>(expr_parser: P) -> impl ExprParser + 'a
where
P: ExprParser + 'a,
{
recursive(|expr_no_constructors| {
expression_with_precedence(
Precedence::Lowest,
expr.clone(),
expr.clone(),
statement(expr.clone(), expr),
expr_parser.clone(),
expr_no_constructors.clone(),
statement(expr_parser, expr_no_constructors),
false,
false,
)
Expand Down Expand Up @@ -1772,7 +1775,7 @@ mod test {
parse_all(
atom_or_right_unary(
expression(),
expression_no_constructors(),
expression_no_constructors(expression()),
fresh_statement(),
true,
),
Expand All @@ -1781,7 +1784,7 @@ mod test {
parse_all_failing(
atom_or_right_unary(
expression(),
expression_no_constructors(),
expression_no_constructors(expression()),
fresh_statement(),
true,
),
Expand All @@ -1801,7 +1804,7 @@ mod test {
parse_all(
atom_or_right_unary(
expression(),
expression_no_constructors(),
expression_no_constructors(expression()),
fresh_statement(),
true,
),
Expand Down Expand Up @@ -2040,12 +2043,12 @@ mod test {
#[test]
fn parse_for_loop() {
parse_all(
for_loop(expression_no_constructors(), fresh_statement()),
for_loop(expression_no_constructors(expression()), fresh_statement()),
vec!["for i in x+y..z {}", "for i in 0..100 { foo; bar }"],
);

parse_all_failing(
for_loop(expression_no_constructors(), fresh_statement()),
for_loop(expression_no_constructors(expression()), fresh_statement()),
vec![
"for 1 in x+y..z {}", // Cannot have a literal as the loop identifier
"for i in 0...100 {}", // Only '..' is supported, there are no inclusive ranges yet
Expand Down Expand Up @@ -2140,11 +2143,11 @@ mod test {
#[test]
fn parse_parenthesized_expression() {
parse_all(
atom(expression(), expression_no_constructors(), fresh_statement(), true),
atom(expression(), expression_no_constructors(expression()), fresh_statement(), true),
vec!["(0)", "(x+a)", "({(({{({(nested)})}}))})"],
);
parse_all_failing(
atom(expression(), expression_no_constructors(), fresh_statement(), true),
atom(expression(), expression_no_constructors(expression()), fresh_statement(), true),
vec!["(x+a", "((x+a)", "(,)"],
);
}
Expand All @@ -2157,12 +2160,12 @@ mod test {
#[test]
fn parse_if_expr() {
parse_all(
if_expr(expression_no_constructors(), fresh_statement()),
if_expr(expression_no_constructors(expression()), fresh_statement()),
vec!["if x + a { } else { }", "if x {}", "if x {} else if y {} else {}"],
);

parse_all_failing(
if_expr(expression_no_constructors(), fresh_statement()),
if_expr(expression_no_constructors(expression()), fresh_statement()),
vec!["if (x / a) + 1 {} else", "if foo then 1 else 2", "if true { 1 }else 3"],
);
}
Expand Down Expand Up @@ -2256,11 +2259,11 @@ mod test {
#[test]
fn parse_unary() {
parse_all(
term(expression(), expression_no_constructors(), fresh_statement(), true),
term(expression(), expression_no_constructors(expression()), fresh_statement(), true),
vec!["!hello", "-hello", "--hello", "-!hello", "!-hello"],
);
parse_all_failing(
term(expression(), expression_no_constructors(), fresh_statement(), true),
term(expression(), expression_no_constructors(expression()), fresh_statement(), true),
vec!["+hello", "/hello"],
);
}
Expand Down Expand Up @@ -2438,4 +2441,52 @@ mod test {

assert_eq!(vecmap(&results, |t| t.0.clone()), vecmap(&results, |t| t.1.clone()),);
}

#[test]
fn expr_no_constructors() {
let cases = vec![
(
"{ if structure { a: 1 } {} }",
1,
"{\n if plain::structure {\n Error\n }\n {\n }\n}",
),
(
"{ if ( structure { a: 1 } ) {} }",
0,
"{\n if ((plain::structure { a: 1 })) {\n }\n}",
),
("{ if ( structure {} ) {} }", 0, "{\n if ((plain::structure { })) {\n }\n}"),
(
"{ if (a { x: 1 }, b { y: 2 }) {} }",
0,
"{\n if ((plain::a { x: 1 }), (plain::b { y: 2 })) {\n }\n}",
),
(
"{ if ({ let foo = bar { baz: 42 }; foo == bar { baz: 42 }}) {} }",
0,
"{\n if ({\n let foo: unspecified = (plain::bar { baz: 42 })\
\n (plain::foo == (plain::bar { baz: 42 }))\n }) {\n }\n}",
),
];

let show_errors = |v| vecmap(&v, ToString::to_string).join("\n");

let results = vecmap(&cases, |&(src, expected_errors, expected_result)| {
let (opt, errors) = parse_recover(block(fresh_statement()), src);
let actual = opt.map(|ast| ast.to_string());
let actual = if let Some(s) = &actual { s.to_string() } else { "(none)".to_string() };

let result =
((errors.len(), actual.clone()), (expected_errors, expected_result.to_string()));
if result.0 != result.1 {
let num_errors = errors.len();
let shown_errors = show_errors(errors);
eprintln!(
"\nExpected {expected_errors} error(s) and got {num_errors}:\n\n{shown_errors}\n\nFrom input: {src}\nExpected AST: {expected_result}\nActual AST: {actual}\n");
}
result
});

assert_eq!(vecmap(&results, |t| t.0.clone()), vecmap(&results, |t| t.1.clone()),);
}
}

0 comments on commit d41c6db

Please sign in to comment.