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

feat: Issue an error when attempting to use a return expression #1330

Merged
merged 6 commits into from
May 11, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,12 @@ impl Recoverable for Expression {
}
}

impl Recoverable for Option<Expression> {
fn error(span: Span) -> Self {
Some(Expression::new(ExpressionKind::Error, span))
}
}

#[derive(Debug, Eq, Clone)]
pub struct Expression {
pub kind: ExpressionKind,
Expand Down
12 changes: 4 additions & 8 deletions crates/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ impl Statement {
last_statement_in_block: bool,
emit_error: &mut dyn FnMut(ParserError),
) -> Statement {
let missing_semicolon =
ParserError::with_reason(ParserErrorReason::MissingSeparatingSemi, span);
match self {
Statement::Let(_)
| Statement::Constrain(_)
Expand All @@ -59,10 +61,7 @@ impl Statement {
| Statement::Error => {
// To match rust, statements always require a semicolon, even at the end of a block
if semi.is_none() {
emit_error(ParserError::with_reason(
ParserErrorReason::MissingSeparatingSemi,
span,
));
emit_error(missing_semicolon);
}
self
}
Expand All @@ -85,10 +84,7 @@ impl Statement {
// for unneeded expressions like { 1 + 2; 3 }
(_, Some(_), false) => Statement::Expression(expr),
(_, None, false) => {
emit_error(ParserError::with_reason(
ParserErrorReason::MissingSeparatingSemi,
span,
));
emit_error(missing_semicolon);
Statement::Expression(expr)
}

Expand Down
2 changes: 2 additions & 0 deletions crates/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub enum ParserErrorReason {
ConstrainDeprecated,
#[error("Expression is invalid in an array-length type: '{0}'. Only unsigned integer constants, globals, generics, +, -, *, /, and % may be used in this context.")]
InvalidArrayLengthExpression(Expression),
#[error("Early 'return' is unsupported")]
EarlyReturn,
}

/// Represents a parsing error, or a parsing error in the making.
Expand Down
49 changes: 49 additions & 0 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ where
assertion(expr_parser.clone()),
declaration(expr_parser.clone()),
assignment(expr_parser.clone()),
return_statement(expr_parser.clone()),
expr_parser.map(Statement::Expression),
))
}
Expand Down Expand Up @@ -714,6 +715,18 @@ fn expression() -> impl ExprParser {
.labelled(ParsingRuleLabel::Expression)
}

fn return_statement<'a, P>(expr_parser: P) -> impl NoirParser<Statement> + 'a
where
P: ExprParser + 'a,
{
ignore_then_commit(keyword(Keyword::Return), expr_parser.or_not())
.validate(|_, span, emit| {
emit(ParserError::with_reason(ParserErrorReason::EarlyReturn, span));
Statement::Error
})
.labelled(ParsingRuleLabel::Statement)
}

// An expression is a single term followed by 0 or more (OP subexpression)*
// where OP is an operator at the given precedence level and subexpression
// is an expression at the current precedence level plus one.
Expand Down Expand Up @@ -1599,4 +1612,40 @@ mod test {
);
}
}

#[test]
fn return_validation() {
let cases = vec![
("{ return 42; }", 1, "{\n Error\n}"),
("{ return 1; return 2; }", 2, "{\n Error\n Error\n}"),
(
"{ return 123; let foo = 4 + 3; }",
1,
"{\n Error\n let foo: unspecified = (4 + 3)\n}",
),
("{ return 1 + 2 }", 2, "{\n Error\n}"),
("{ return; }", 1, "{\n Error\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(expression()), 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 {} error(s) and got {}:\n\n{}\n\nFrom input: {}\nExpected AST: {}\nActual AST: {}\n",
expected_errors, num_errors, shown_errors, src, expected_result, actual);
}
result
});

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