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

parser: reduce diversity in error handling mechanisms #67744

Merged
merged 11 commits into from
Dec 31, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Dec 31, 2019

Instead of having e.g. span_err, fatal, etc., we prefer to move towards uniformly using struct_span_err thus making it harder to emit fatal and/or unstructured diagnostics.

This PR also de-fatalizes some diagnostics.

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2019
| ^ unexpected `(` after qualified path
| ------------------^ unexpected `(` after qualified path
| |
| the qualified path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could make this recover, but then the vec![...] => recovery has to go away (and I think it should, because it is causing problems in #66364 as well).

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be a shame to lose it entirely, but if it improves this case it might be worth it. For the vec case if we lose the suggestion at least it should recover gracefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to have the vec![...] work with resolve instead of first checking if there is an error in expansion (i.e. check if the path is actually {core,std}::vec)? Perhaps we can use diagnostic_items? It feels like the current encoding of the error is problematic in terms of infrastructure. What do you think @petrochenkov?

Copy link
Contributor

Choose a reason for hiding this comment

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

The vec![...] hack is really bad, but I cannot answer whether it can be done it a better way without some investigation.

@petrochenkov
Copy link
Contributor

Finally!
The error reporting had a wrong default (fatal) for so long.
I wanted to switch the default to recoverable in #57540 where many diagnostics were de-fatalized, but we decided to defer it to some later PR, but that "later" never happened.

@petrochenkov petrochenkov self-assigned this Dec 31, 2019
@petrochenkov
Copy link
Contributor

@bors r+

Perhaps it's time to eliminate -Z continue-parse-after-error already.

@bors
Copy link
Contributor

bors commented Dec 31, 2019

📌 Commit 2e78061 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 Dec 31, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 31, 2019

Perhaps it's time to eliminate -Z continue-parse-after-error already.

Worth a try, I think, 👍.

Centril added a commit to Centril/rust that referenced this pull request Dec 31, 2019
…enkov

parser: reduce diversity in error handling mechanisms

Instead of having e.g. `span_err`, `fatal`, etc., we prefer to move towards uniformly using `struct_span_err` thus making it harder to emit fatal and/or unstructured diagnostics.

This PR also de-fatalizes some diagnostics.

r? @estebank
bors added a commit that referenced this pull request Dec 31, 2019
Rollup of 6 pull requests

Successful merges:

 - #67574 (Extract `rustc_ast_lowering` crate from `rustc`)
 - #67685 (Constify Result)
 - #67702 (Add symbol normalization for proc_macro_server.)
 - #67730 (Cleanup pattern type checking, fix diagnostics bugs (+ improvements))
 - #67744 (parser: reduce diversity in error handling mechanisms)
 - #67748 (Use function attribute "frame-pointer" instead of "no-frame-pointer-elim")

Failed merges:

r? @ghost
@bors bors merged commit 2e78061 into rust-lang:master Dec 31, 2019
@Centril Centril deleted the reduce-diversity branch December 31, 2019 23:00
Centril added a commit to Centril/rust that referenced this pull request Jan 3, 2020
…chenkov

More reductions in error handling diversity

In this follow up to rust-lang#67744, we:

- Remove all fatal / error / warning macros in `syntax` except for `struct_span_err`, which is moved to `rustc_errors`.

- Lintify some hard-coded warnings which used warning macros.

- Defatalize some errors.

In general, the goal here is to make it painful to use fatal or unstructured errors and so we hopefully won't see many of these creep in.
bors added a commit that referenced this pull request Jan 8, 2020
More reductions in error handling diversity

In this follow up to #67744, we:

- Remove all fatal / error / warning macros in `syntax` except for `struct_span_err`, which is moved to `rustc_errors`.

- Lintify some hard-coded warnings which used warning macros.

- Defatalize some errors.

In general, the goal here is to make it painful to use fatal or unstructured errors and so we hopefully won't see many of these creep in.

Fixes #67933.
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.

5 participants