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.

Sign up for GitHub

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

Jump to bottom

Parser incorrectly suggests adding semicolon after function in impl #87647

Closed
camelid opened this issue Jul 30, 2021 · 6 comments · Fixed by #91531
Closed

Parser incorrectly suggests adding semicolon after function in impl #87647

camelid opened this issue Jul 30, 2021 · 6 comments · Fixed by #91531
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone
1.56.0

Comments

@camelid
Copy link
Member

camelid commented Jul 30, 2021

This also ICEs, but that's a separate issue: #87635. The MCVE here is also from that issue.

I think this issue is also caused by #87436, based on looking at the changes in the test output from that PR. That PR seems to have changed the parser to emit an expected `;`, found ... error regardless of what token was actually expected.

Code

Playground

struct Foo {}

impl Foo {
    pub fn bar()
}

fn main() {}

Nightly error

error: expected `;`, found `}`
 --> src/main.rs:4:17
  |
4 |     pub fn bar()
  |                 ^ help: add `;` here
5 | }
  | - unexpected token

thread 'rustc' panicked at 'internal error: entered unreachable code', compiler/rustc_parse/src/parser/item.rs:1723:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.56.0-nightly (492723897 2021-07-29) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C codegen-units=1 -C debuginfo=2 --crate-type bin

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
error: could not compile `playground` due to previous error

Beta error

error: expected one of `->`, `;`, `where`, or `{`, found `}`
 --> src/main.rs:5:1
  |
4 |     pub fn bar()
  |            ---  - expected one of `->`, `;`, `where`, or `{`
  |            |
  |            while parsing this `fn`
5 | }
  | ^ unexpected token

error: associated function in `impl` without body
 --> src/main.rs:4:5
  |
4 |     pub fn bar()
  |     ^^^^^^^^^^^-
  |                |
  |                help: provide a definition for the function: `{ <body> }`

error: could not compile `playground` due to 2 previous errors

Meta

Regressed in nightly 1.56.0.
@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Jul 30, 2021
@camelid camelid added this to the 1.56.0 milestone Jul 30, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 30, 2021
@apiraino
Copy link
Contributor

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

Bisection seems to confirm that #87436 may have a role here (specifically 8bebfe5), @ebobrow could perhaps have more info here? Thanks!

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 31, 2021
@apiraino apiraino mentioned this issue Jul 31, 2021
Closed
@ebobrow
Copy link
Contributor

ebobrow commented Aug 1, 2021

Uh oh, sorry about this. I don't have any good ideas on how to fix it but I'll take a look. Alternatively, since none of the changes in #87436 are necessary, we could technically roll back the whole PR. It's definitely the cause of this issue.

@apiraino
Copy link
Contributor

apiraino commented Aug 1, 2021

no worries @ebobrow and thanks for investigating

@ebobrow
Copy link
Contributor

ebobrow commented Aug 2, 2021

If we can be sure that this is the only case where the semicolon check creates unwanted suggestions, we could make it optional, like with a helper function or an extra boolean parameter. That would prevent the ICE and give the expected error message, but I'm not sure how elegant it is. Thoughts?

@JohnTitor JohnTitor mentioned this issue Aug 2, 2021
Merged
@Mark-Simulacrum Mark-Simulacrum added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Sep 15, 2021
@Mark-Simulacrum
Copy link
Member

This is likely going to slip into the release, but it's just a diagnostics regression I think (the ICE fix landed), so seems OK.

@camelid
Copy link
Member Author

camelid commented Oct 15, 2021

Yeah, this is the current playground output on beta 1.56.0:

error: expected `;`, found `}`
 --> src/main.rs:4:17
  |
4 |     pub fn bar()
  |                 ^ help: add `;` here
5 | }
  | - unexpected token

error: associated function in `impl` without body
 --> src/main.rs:4:5
  |
4 |     pub fn bar()
  |     ^^^^^^^^^^^-
  |                |
  |                help: provide a definition for the function: `{ <body> }`

error: could not compile `playground` due to 2 previous errors

@notriddle notriddle mentioned this issue Dec 4, 2021
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 7, 2021
@matthiaskrgr
Rollup merge of rust-lang#91531 - notriddle:notriddle/issue-87647-exp…
915fb2a 
…ected-semicolon, r=estebank

Do not add `;` to expected tokens list when it's wrong

There's a few spots where semicolons are checked for to do error recovery, and should not be suggested (or checked for other stuff).

Fixes rust-lang#87647
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 8, 2021
@matthiaskrgr
Rollup merge of rust-lang#91531 - notriddle:notriddle/issue-87647-exp…
0e8c1b8 
…ected-semicolon, r=estebank

Do not add `;` to expected tokens list when it's wrong

There's a few spots where semicolons are checked for to do error recovery, and should not be suggested (or checked for other stuff).

Fixes rust-lang#87647
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 8, 2021
@matthiaskrgr
Rollup merge of rust-lang#91531 - notriddle:notriddle/issue-87647-exp…
87f2c51 
…ected-semicolon, r=estebank

Do not add `;` to expected tokens list when it's wrong

There's a few spots where semicolons are checked for to do error recovery, and should not be suggested (or checked for other stuff).

Fixes rust-lang#87647
@bors bors closed this as completed in 74437e4 Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.56.0
Development

Successfully merging a pull request may close this issue.

Do not add ; to expected tokens list when it's wrong notriddle/rust
5 participants
@Mark-Simulacrum @apiraino @camelid @rustbot @ebobrow