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

Top level or-patterns are not allowed let binding #2569

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

MahadMuhammad
Copy link
Contributor

Top level or-patterns are not allowed let binding - E0658

  • This error code is emitted by rustc 1.49.0, but not emitted by rustc -nightly.
  • I have updated according to rustc 1.49.0, as we are targeting rustc 1.49.0 testsuite.
  • See the difference here godbolt

gcc/rust/ChangeLog:

* hir/rust-ast-lower-pattern.cc (ASTLoweringPattern::visit): Added richlocation & error code.

gcc/testsuite/ChangeLog:

* rust/compile/let_alt.rs: Updated comment.

richloc.add_fixit_replace ("use an outer grouped pattern");
rust_error_at (
richloc, ErrorCode::E0658,
"top level or-patterns are not allowed for %<let%> bindings");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This message and errorcode is according to rustc 1.49.0, rustc nighly not emit E0658 (An unstable feature was used) as this was unstable feature in rustc 1.49.0.

Should we add the latest verion message, or stick to rustc 1.49.0

Copy link
Member

Choose a reason for hiding this comment

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

We can't use E0658 here, as E0658 refers to the use of unstable features. In 1.49, top level or patterns were considered an unstable feature, which is why the compiler emitted E0658. It's now been stabilized. So the handling here needs to be a little different.

The error in itself does not have an error code. So we cannot have one either.

We then have to decide if we want to implement top level or-patterns (if they are needed by core or std, we'll need to implement them, but otherwise no) and go from there.

Furthermore, in Rust 1.49 with nightly features let a | a = 15 is valid, but not in Rust 1.53 when toplevel or-patterns were stabilized. You need to use let (a | a) = 15.

Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

I think improving the error message is nice, and we should remove the error code from the error message. Looks good to me apart from this :)

richloc.add_fixit_replace ("use an outer grouped pattern");
rust_error_at (
richloc, ErrorCode::E0658,
"top level or-patterns are not allowed for %<let%> bindings");
Copy link
Member

Choose a reason for hiding this comment

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

We can't use E0658 here, as E0658 refers to the use of unstable features. In 1.49, top level or patterns were considered an unstable feature, which is why the compiler emitted E0658. It's now been stabilized. So the handling here needs to be a little different.

The error in itself does not have an error code. So we cannot have one either.

We then have to decide if we want to implement top level or-patterns (if they are needed by core or std, we'll need to implement them, but otherwise no) and go from there.

Furthermore, in Rust 1.49 with nightly features let a | a = 15 is valid, but not in Rust 1.53 when toplevel or-patterns were stabilized. You need to use let (a | a) = 15.

@MahadMuhammad MahadMuhammad changed the title gccrs: [E0658] top level or-patterns are not allowed let binding Top level or-patterns are not allowed let binding Aug 18, 2023
gcc/rust/hir/rust-ast-lower-pattern.cc Outdated Show resolved Hide resolved
gcc/rust/ChangeLog:

	* hir/rust-ast-lower-pattern.cc (ASTLoweringPattern::visit):
	Added richlocation & error code.

gcc/testsuite/ChangeLog:

	* rust/compile/let_alt.rs: Updated comment.

Signed-off-by: Muhammad Mahad <mahadtxt@gmail.com>
@CohenArthur CohenArthur added the diagnostic diagnostic static analysis label Aug 21, 2023
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

Looks great!

@CohenArthur CohenArthur added this pull request to the merge queue Aug 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Aug 21, 2023
@P-E-P P-E-P added this pull request to the merge queue Sep 1, 2023
Merged via the queue into Rust-GCC:master with commit 47d8541 Sep 1, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostic diagnostic static analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants