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

Always emit unclosed delimiter diagnostics #58903

Merged
merged 11 commits into from
Mar 8, 2019

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 3, 2019

Fix #58886.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 3, 2019
@Centril
Copy link
Contributor

Centril commented Mar 3, 2019

@estebank Does this regression exist on the current beta compiler? If so we should beta backport.

@estebank estebank added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 4, 2019
@estebank
Copy link
Contributor Author

estebank commented Mar 4, 2019

@Centril I thought beta was spared, but alas, it wasn't.

@Centril
Copy link
Contributor

Centril commented Mar 4, 2019

@estebank Aw; crap... 😭; At least we caught it before it went to stable 🎉!

@rust-highfive

This comment has been minimized.

@estebank estebank force-pushed the forgetful-delims branch 2 times, most recently from 5d4334c to 163191a Compare March 4, 2019 03:15
@rust-highfive

This comment has been minimized.

@estebank estebank changed the title Emit unclosed delimiter diagnostics or panic if this hasn't happened Emit unclosed delimiter diagnostics Mar 4, 2019
@estebank estebank changed the title Emit unclosed delimiter diagnostics Always emit unclosed delimiter diagnostics Mar 4, 2019
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2019
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 4, 2019
@rust-highfive

This comment has been minimized.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 5, 2019

📌 Commit cc077bcd2f625101eee450c0e0083210d2e51af1 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2019
@estebank
Copy link
Contributor Author

estebank commented Mar 7, 2019

@bors p=52

@estebank
Copy link
Contributor Author

estebank commented Mar 7, 2019

@petrochenkov subsuming #58863 into this PR to avoid merge conflict.

@estebank
Copy link
Contributor Author

estebank commented Mar 7, 2019

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Mar 7, 2019

📌 Commit 551ea65 has been approved by petrochenkov

@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 7, 2019
LL | }
| - expected one of 11 possible tokens here
LL | }
| ^ unexpected token
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally identified what is causing these: when reaching the end of the current TokenTree, self.bump() will advance to a close token corresponding to the opening delimiter. There are two options: 1) attempt to rewrite the TokenTree representation that the Parser has once we suggest a close delim (high potential for bad lurking bugs), or 2) catch this specific case in expect_one_of, but that might be too magical.

@pietroalbini
Copy link
Member

@bors p=2

@bors
Copy link
Contributor

bors commented Mar 8, 2019

⌛ Testing commit 551ea65 with merge b58a006...

bors added a commit that referenced this pull request Mar 8, 2019
Always emit unclosed delimiter diagnostics

Fix #58886.
@bors
Copy link
Contributor

bors commented Mar 8, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing b58a006 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 8, 2019
@bors bors merged commit 551ea65 into rust-lang:master Mar 8, 2019
@pnkfelix
Copy link
Member

discussed at T-compiler meeting. beta-accepted.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Mar 14, 2019
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 16, 2019
@estebank estebank deleted the forgetful-delims branch November 9, 2023 05:20
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 23, 2024
…i-obk

Don't fatal when calling `expect_one_of` when recovering arg in `parse_seq`

In `parse_seq`, when parsing a sequence of token-separated items, if we don't see a separator, we try to parse another item eagerly in order to give a good diagnostic and recover from a missing separator:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/mod.rs#L900-L901

If parsing the item itself calls `expect_one_of`, then we will fatal because of rust-lang#58903:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/mod.rs#L513-L516

For `precise_capturing` feature I implemented, we do end up calling `expected_one_of`:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/ty.rs#L712-L714

This leads the compiler to fatal *before* having emitted the first error, leading to absolutely no useful information for the user about what happened in the parser.

This PR makes it so that we stop doing that.

Fixes rust-lang#124195
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 23, 2024
…i-obk

Don't fatal when calling `expect_one_of` when recovering arg in `parse_seq`

In `parse_seq`, when parsing a sequence of token-separated items, if we don't see a separator, we try to parse another item eagerly in order to give a good diagnostic and recover from a missing separator:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/mod.rs#L900-L901

If parsing the item itself calls `expect_one_of`, then we will fatal because of rust-lang#58903:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/mod.rs#L513-L516

For `precise_capturing` feature I implemented, we do end up calling `expected_one_of`:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/ty.rs#L712-L714

This leads the compiler to fatal *before* having emitted the first error, leading to absolutely no useful information for the user about what happened in the parser.

This PR makes it so that we stop doing that.

Fixes rust-lang#124195
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 23, 2024
Rollup merge of rust-lang#124169 - compiler-errors:parser-fatal, r=oli-obk

Don't fatal when calling `expect_one_of` when recovering arg in `parse_seq`

In `parse_seq`, when parsing a sequence of token-separated items, if we don't see a separator, we try to parse another item eagerly in order to give a good diagnostic and recover from a missing separator:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/mod.rs#L900-L901

If parsing the item itself calls `expect_one_of`, then we will fatal because of rust-lang#58903:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/mod.rs#L513-L516

For `precise_capturing` feature I implemented, we do end up calling `expected_one_of`:
https://github.com/rust-lang/rust/blob/d1a0fa5ed3ffe52d72f761d3c95cbeb0a9cdfe66/compiler/rustc_parse/src/parser/ty.rs#L712-L714

This leads the compiler to fatal *before* having emitted the first error, leading to absolutely no useful information for the user about what happened in the parser.

This PR makes it so that we stop doing that.

Fixes rust-lang#124195
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants