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

Fix ICE when misplaced visibility cannot be properly parsed #86932

Merged
merged 1 commit into from
Jul 8, 2021

Conversation

rylev
Copy link
Member

@rylev rylev commented Jul 7, 2021

Fixes #86895

The issue was that a failure to parse the visibility was causing the original error to be dropped before being emitted.

The resulting error isn't quite as nice as when the visibility is parsed properly, but I'm not sure which error to prioritize here. Displaying both errors might be too confusing.

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2021
@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

estebank commented Jul 7, 2021

@bors r+

I want to rework this part of the parser to handle incorrect code more gracefully, but for now this is fine.

@bors
Copy link
Contributor

bors commented Jul 7, 2021

📌 Commit 04a9c10 has been approved by estebank

@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 Jul 7, 2021
Comment on lines +1794 to +1800
let vis = match self.parse_visibility(FollowedByType::No) {
Ok(v) => v,
Err(mut d) => {
d.cancel();
return Err(err);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of code is quite common, it'd be nice if we had some helper for it 🤔

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 7, 2021
Fix ICE when misplaced visibility cannot be properly parsed

Fixes rust-lang#86895

The issue was that a failure to parse the visibility was causing the original error to be dropped before being emitted.

The resulting error isn't quite as nice as when the visibility is parsed properly, but I'm not sure which error to prioritize here. Displaying both errors might be too confusing.

r? `@estebank`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 7, 2021
Fix ICE when misplaced visibility cannot be properly parsed

Fixes rust-lang#86895

The issue was that a failure to parse the visibility was causing the original error to be dropped before being emitted.

The resulting error isn't quite as nice as when the visibility is parsed properly, but I'm not sure which error to prioritize here. Displaying both errors might be too confusing.

r? ``@estebank``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2021
Rollup of 9 pull requests

Successful merges:

 - rust-lang#86639 (Support lint tool names in rustc command line options)
 - rust-lang#86812 (Recover from `&dyn mut ...` parse errors)
 - rust-lang#86917 (Add doc comment for `impl From<LayoutError> for TryReserveError`)
 - rust-lang#86925 (Add self to mailmap)
 - rust-lang#86927 (Sync rustc_codegen_cranelift)
 - rust-lang#86932 (Fix ICE when misplaced visibility cannot be properly parsed)
 - rust-lang#86933 (Clean up rustdoc static files)
 - rust-lang#86955 (Fix typo in `ops::Drop` docs)
 - rust-lang#86956 (Revert "Add "every" as a doc alias for "all".")

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 463301a into rust-lang:master Jul 8, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

internal compiler error: the following error was constructed but not emitted
6 participants