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

chore: Emit an error for the 'return' expression #1214

Closed
wants to merge 4 commits into from

Conversation

pczarn
Copy link
Contributor

@pczarn pczarn commented Apr 25, 2023

Related issue(s)

Resolves #1190

Description

Emit the Early 'return' is unsupported message for the 'return' expression

Dependency additions / changes

N/A

Test additions / changes

Added return_validation

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Documentation needs

  • This PR requires documentation updates when merged.

Additional context

N/A

@pczarn
Copy link
Contributor Author

pczarn commented Apr 25, 2023

My guess is that return should be under atom instead.

@kevaundray kevaundray changed the title Emit an error for the 'return' expression chore: Emit an error for the 'return' expression Apr 25, 2023
crates/noirc_frontend/src/parser/parser.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/parser/parser.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/parser/parser.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/parser/parser.rs Outdated Show resolved Hide resolved
@pczarn
Copy link
Contributor Author

pczarn commented Apr 30, 2023

@jfecher A 'return' statement fails with an additional error when not followed by a semicolon, but I guess it's not an issue to allow that.

@jfecher
Copy link
Contributor

jfecher commented May 1, 2023

@pczarn you should be able to remove the semicolon error by adding an appropriate case for Return in Statement::add_semicolon which controls whether a semicolon is required or optional.

@pczarn
Copy link
Contributor Author

pczarn commented May 7, 2023

@jfecher Fixed according to your advice.

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@jfecher jfecher enabled auto-merge May 8, 2023 19:05
Statement::Return { expr, semi: false } => {
if !last_statement_in_block && semi.is_none() {
let reason = "Expected a ; separating these two statements".to_string();
emit_error(ParserError::with_reason(reason, span));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of #1286, ParserError::with_reason takes an enum (hence the failing tests). You can add a variant in parser/errors.rs.

@kevaundray
Copy link
Contributor

Closing as #1330 was merged

@kevaundray kevaundray closed this Jul 13, 2023
auto-merge was automatically disabled July 13, 2023 12:52

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue an error when return is used
4 participants