More reductions in error handling diversity #67770

Centril · 2020-01-01T03:51:35Z

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.

Centril · 2020-01-01T03:51:50Z

r? @Mark-Simulacrum

Mark-Simulacrum

Some of my comments I think get obsoleted by future commits, but along with them, two higher-level comments:

no real point to moving span_err and friends to struct_span_err(...).emit()
it is not clear that removing the macros, rather than (e.g.) moving to rustc_errors is helpful

I'm also not super happy with moving things to lints in this PR; feels like maybe there's some discussion to be had, and it's not clear that it brings us benefits from the PR goals perspective. Maybe we could split those commits out? I don't know if we need (e.g.) sign off or so. Seems like probably no, and indeed moving to lints is good.

Would also be good to do some rebase magic and cleanup commits (a few are misspelled or essential duplicates of each other, e.g., I would roll in all the "unused" macro removal into the ~last commit which removes the syntax::diagnostics::macros module.

src/test/ui/lint/lint-uppercase-variables.stderr

src/librustc_metadata/locator.rs

src/librustc_typeck/structured_errors.rs

src/librustc_typeck/check/mod.rs

Centril · 2020-01-01T22:10:55Z

(For the benefit of everyone reading, here's a summary of notes I left on Discord...)

no real point to moving span_err and friends to struct_span_err(...).emit()

The point is to encourage adding .span_suggestion(...), .span_label(...) and whatnot by not immediately emitting the error ("the right thing should be more convenient"). Eventually we can maybe rename to error!(...).emit(); once diversity is further reduced.

it is not clear that removing the macros, rather than (e.g.) moving to rustc_errors is helpful

I think we should make it harder to emit unstructured / fatal errors as well as hard-coded warnings, and those macros are in the way of that. Also, the diagnostics builder API is good enough that help!(...) et. al feels unnecessary.

I'm also not super happy with moving things to lints in this PR; feels like maybe there's some discussion to be had, and it's not clear that it brings us benefits from the PR goals perspective. Maybe we could split those commits out? I don't know if we need (e.g.) sign off or so. Seems like probably no, and indeed moving to lints is good.

So lintifying these enabled nixing some of the macros. Beyond that, hard-coded warnings are not something we want outside of things like the NLL transition (which are super rare and don't deserve sugar).

Would also be good to do some rebase magic and cleanup commits (a few are misspelled or essential duplicates of each other, e.g., I would roll in all the "unused" macro removal into the ~last commit which removes the syntax::diagnostics::macros module.

Squashed all into one commit. :)

Mark-Simulacrum · 2020-01-02T00:44:37Z

Changes look fine to me overall; probably makes sense to wait for @petrochenkov here though.

src/librustc_errors/diagnostic_builder.rs

src/librustc/hir/check_attr.rs

src/librustc/lint/builtin.rs

petrochenkov · 2020-01-02T11:07:59Z

Eventually we can maybe rename to error!(...).emit();

+1, if the errors are spanned and structural by default, then it doesn't make sense to write struct_span.

petrochenkov · 2020-01-02T11:12:54Z

We also probably need a variation of struct_span_err without an error code though, errors with codes are not an overwhelming majority right now, unlike e.g. spanned errors.
(But that kind of depends on the decision about error codes in general.)

Centril · 2020-01-02T11:14:20Z

The variant without a code is just self.struct_span_err(...) (not a macro) though? (It works well enough in the parser... At least I've never wanted for more.)

petrochenkov · 2020-01-02T11:16:42Z

@Centril
You have to write &format!(...) with self.struct_span_err, which is kind of annoying.

Centril · 2020-01-02T12:51:30Z

Review comments should be addressed now (except for the new variation on the macro, which I think could be done some other time).

- remove syntax::{help!, span_help!, span_note!} - remove unused syntax::{struct_span_fatal, struct_span_err_or_warn!, span_err_or_warn!} - lintify check_for_bindings_named_same_as_variants + conflicting_repr_hints - inline syntax::{struct_span_warn!, diagnostic_used!} - stringify_error_code! -> error_code! & use it more. - find_plugin_registrar: de-fatalize an error - de-fatalize metadata errors - move type_error_struct! to rustc_typeck - struct_span_err! -> rustc_errors

Centril · 2020-01-08T03:27:23Z

@bors r=petrochenkov

bors · 2020-01-08T03:27:24Z

📌 Commit 20ebb80 has been approved by petrochenkov

bors · 2020-01-08T12:12:15Z

⌛ Testing commit 20ebb80 with merge ed6468d...

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.

bors · 2020-01-08T15:25:32Z

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing ed6468d to master...

rust-highfive · 2020-01-08T15:27:22Z

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Extract `rustc_ast_passes`, move gating, & refactor linting Based on rust-lang#67770. This PR extracts a crate `rustc_ast_passes`: - `ast_validation.rs`, which is contributed by `rustc_passes` (now only has HIR based passes) -- the goal here is to improve recompilation of the parser, - `feature_gate.rs`, which is contributed by `syntax` and performs post-expansion-gating & final erroring for pre-expansion gating, - `show_span`, which is contributed by `syntax`. To facilitate this, we first have to also: - Move `{leveled_}feature_err{_err}` from `syntax::feature_gate::check` into `rustc_session::parse`. - Move `get_features` into `rustc_parse::config`, the only place it is used. - Move some some lint datatypes and traits, e.g. `LintBuffer`, `BufferedEarlyLint`, `BuiltinLintDiagnostics`, `LintPass`, and `LintArray` into `rustc_session::lint`. - Move all the hard-wired lint `static`s into `rustc_session::lint::builtin`.

rust-highfive assigned eddyb Jan 1, 2020

This comment has been minimized.

Sign in to view

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 1, 2020

rust-highfive assigned Mark-Simulacrum and unassigned eddyb Jan 1, 2020

Mark-Simulacrum reviewed Jan 1, 2020

View reviewed changes

src/test/ui/lint/lint-uppercase-variables.stderr Outdated Show resolved Hide resolved

src/librustc_metadata/locator.rs Show resolved Hide resolved

src/librustc_typeck/structured_errors.rs Outdated Show resolved Hide resolved

src/librustc_typeck/check/mod.rs Show resolved Hide resolved

petrochenkov self-assigned this Jan 1, 2020

This comment has been minimized.

Sign in to view

Centril force-pushed the reduce-diversity-2 branch 5 times, most recently from c34ca3b to be8f29e Compare January 1, 2020 21:59

petrochenkov reviewed Jan 2, 2020

View reviewed changes

src/librustc_errors/diagnostic_builder.rs Show resolved Hide resolved

petrochenkov reviewed Jan 2, 2020

View reviewed changes

src/librustc/hir/check_attr.rs Show resolved Hide resolved

petrochenkov reviewed Jan 2, 2020

View reviewed changes

src/librustc/lint/builtin.rs Outdated Show resolved Hide resolved

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 Jan 2, 2020

Centril mentioned this pull request Jan 2, 2020

Extract rustc_ast_passes, move gating, & refactor linting #67806

Merged

Centril force-pushed the reduce-diversity-2 branch from be8f29e to 562389d Compare January 2, 2020 12:25

This comment has been minimized.

Sign in to view

Centril force-pushed the reduce-diversity-2 branch from 5728919 to 185f28e Compare January 2, 2020 14:48

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2020

Centril added 2 commits January 8, 2020 04:25

span_to_lines: account for DUMMY_SP

20ebb80

Centril force-pushed the reduce-diversity-2 branch from effdacf to 20ebb80 Compare January 8, 2020 03:26

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 8, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 8, 2020

bors merged commit 20ebb80 into rust-lang:master Jan 8, 2020

bors mentioned this pull request Jan 8, 2020

Introduce X.., ..X, and ..=X range patterns #67258

Merged

Centril deleted the reduce-diversity-2 branch January 8, 2020 18:36

Centril mentioned this pull request Jan 21, 2020

"Conflicting representations" warning became an error #68428

Closed

ehuss mentioned this pull request Feb 3, 2020

cargo check --message-format json regression of error span data on empty main.rs file #68808

Closed

est31 mentioned this pull request Nov 8, 2022

Matching field-less enum variants that aren't imported should deny-lint, not just give lots of warnings #103442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More reductions in error handling diversity #67770

More reductions in error handling diversity #67770

Centril commented Jan 1, 2020 •

edited

Loading

This comment has been minimized.

Centril commented Jan 1, 2020

Mark-Simulacrum left a comment

This comment has been minimized.

Centril commented Jan 1, 2020

Mark-Simulacrum commented Jan 2, 2020

petrochenkov commented Jan 2, 2020

petrochenkov commented Jan 2, 2020

Centril commented Jan 2, 2020 •

edited

Loading

petrochenkov commented Jan 2, 2020

Centril commented Jan 2, 2020

This comment has been minimized.

Centril commented Jan 8, 2020

bors commented Jan 8, 2020

bors commented Jan 8, 2020

bors commented Jan 8, 2020

rust-highfive commented Jan 8, 2020

More reductions in error handling diversity #67770

More reductions in error handling diversity #67770

Conversation

Centril commented Jan 1, 2020 • edited Loading

This comment has been minimized.

Centril commented Jan 1, 2020

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

This comment has been minimized.

Centril commented Jan 1, 2020

Mark-Simulacrum commented Jan 2, 2020

petrochenkov commented Jan 2, 2020

petrochenkov commented Jan 2, 2020

Centril commented Jan 2, 2020 • edited Loading

petrochenkov commented Jan 2, 2020

Centril commented Jan 2, 2020

This comment has been minimized.

Centril commented Jan 8, 2020

bors commented Jan 8, 2020

bors commented Jan 8, 2020

bors commented Jan 8, 2020

rust-highfive commented Jan 8, 2020

Centril commented Jan 1, 2020 •

edited

Loading

Centril commented Jan 2, 2020 •

edited

Loading