Implement rustc side of report-future-incompat #75534

Aaron1011 · 2020-08-14T18:34:53Z

This is an alternative to @pnkfelix's initial implementation in https://github.com/pnkfelix/rust/commits/prototype-rustc-side-of-report-future-incompat (mainly because I started working before seeing that branch 😄 ).

My approach outputs the entire original Diagnostic, in a way that is compatible with incremental compilation. This is not yet integrated with compiletest, but can be used manually by passing -Z emit-future-incompat-report to rustc.

Several changes are made to support this feature:

The librustc_session/lint module is moved to a new crate librustc_lint_defs (name bikesheddable). This allows accessing lint definitions from librustc_errors.
The Lint struct is extended with an Option<FutureBreakage>. When present, it indicates that we should display a lint in the future-compat report. FutureBreakage contains additional information that we may want to display in the report (currently, a date field indicating when the crate will stop compiling).
A new variant rustc_error::Level::Allow is added. This is used when constructing a diagnostic for a future-breakage lint that is marked as allowed (via #[allow] or --cap-lints). This allows us to capture any future-breakage diagnostics in one place, while still discarding them before they are passed to the Emitter.
DiagnosticId::Lint is extended with a has_future_breakage field, indicating whether or not the Lint has future breakage information (and should therefore show up in the report).
Session is given access to the LintStore via a new SessionLintStore trait (since librustc_session cannot directly reference LintStore without a cyclic dependency). We use this to turn a string DiagnosticId::Lint back into a Lint, to retrieve the FutureBreakage data.

Currently, FutureBreakage.date is always set to None. However, this could potentially be interpreted by Cargo in the future.

I've enabled the future-breakage report for the ARRAY_INTO_ITER lint, which can be used to test out this PR. The intent is to use the field to allow Cargo to determine the date of future breakage (as described in RFC 2834) without needing to parse the diagnostic itself.

cc @pnkfelix

rust-highfive · 2020-08-14T18:34:57Z

r? @ecstatic-morse

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

ecstatic-morse · 2020-08-15T23:22:47Z

Not familiar with this part of the code.

r? @pnkfelix maybe?

Aaron1011 · 2020-09-14T02:43:23Z

@pnkfelix: This is now ready for review

Aaron1011 · 2020-09-24T22:16:54Z

@pnkfelix: Are there any changes that you'd like me to make?

bors · 2020-09-26T08:53:18Z

☔ The latest upstream changes (presumably #70743) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Aaron1011 · 2020-10-02T16:00:16Z

@pnkfelix: Do you think you'll be able to review this sometime soon? There are several issues that would benefit from having this available.

bors · 2020-10-05T05:07:55Z

☔ The latest upstream changes (presumably #77557) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

bors · 2020-11-01T03:47:57Z

⌛ Testing commit 6db00a2 with merge 7bcd889ebca66a5d5e6224220bc58bca2a240ddf...

bors · 2020-11-01T05:19:53Z

💔 Test failed - checks-actions

Aaron1011 · 2020-11-01T16:50:12Z

@bors retry

bors · 2020-11-01T16:52:34Z

⌛ Testing commit 6db00a2 with merge b202532...

bors · 2020-11-01T19:16:43Z

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing b202532 to master...

Fixes rust-lang#78660 With PR rust-lang#75534 merged, we now run more lint-related code for future-incompat-report, even when their final level is Allow. Some lint-related code was not expecting `Level::Allow`, and had an explicit panic. This PR explicitly tracks the lint level set on the command line before `--cap-lints` is applied. This is used to emit a more precise error note (e.g. we don't say that `-W lint-name` was specified on the command line just because a lint was capped to Warn). As a result, we can now correctly emit a note that `-A` was used if we got `Level::Allow` from the command line (before the cap is applied).

…tmandry Fix ICE when a future-incompat-report has its command-line level capped Fixes rust-lang#78660 With PR rust-lang#75534 merged, we now run more lint-related code for future-incompat-report, even when their final level is Allow. Some lint-related code was not expecting `Level::Allow`, and had an explicit panic. This PR explicitly tracks the lint level set on the command line before `--cap-lints` is applied. This is used to emit a more precise error note (e.g. we don't say that `-W lint-name` was specified on the command line just because a lint was capped to Warn). As a result, we can now correctly emit a note that `-A` was used if we got `Level::Allow` from the command line (before the cap is applied).

…, r=pnkfelix Implement rustc side of report-future-incompat cc rust-lang#71249 This is an alternative to `@pnkfelix's` initial implementation in https://github.com/pnkfelix/rust/commits/prototype-rustc-side-of-report-future-incompat (mainly because I started working before seeing that branch 😄 ). My approach outputs the entire original `Diagnostic`, in a way that is compatible with incremental compilation. This is not yet integrated with compiletest, but can be used manually by passing `-Z emit-future-incompat-report` to `rustc`. Several changes are made to support this feature: * The `librustc_session/lint` module is moved to a new crate `librustc_lint_defs` (name bikesheddable). This allows accessing lint definitions from `librustc_errors`. * The `Lint` struct is extended with an `Option<FutureBreakage>`. When present, it indicates that we should display a lint in the future-compat report. `FutureBreakage` contains additional information that we may want to display in the report (currently, a `date` field indicating when the crate will stop compiling). * A new variant `rustc_error::Level::Allow` is added. This is used when constructing a diagnostic for a future-breakage lint that is marked as allowed (via `#[allow]` or `--cap-lints`). This allows us to capture any future-breakage diagnostics in one place, while still discarding them before they are passed to the `Emitter`. * `DiagnosticId::Lint` is extended with a `has_future_breakage` field, indicating whether or not the `Lint` has future breakage information (and should therefore show up in the report). * `Session` is given access to the `LintStore` via a new `SessionLintStore` trait (since `librustc_session` cannot directly reference `LintStore` without a cyclic dependency). We use this to turn a string `DiagnosticId::Lint` back into a `Lint`, to retrieve the `FutureBreakage` data. Currently, `FutureBreakage.date` is always set to `None`. However, this could potentially be interpreted by Cargo in the future. I've enabled the future-breakage report for the `ARRAY_INTO_ITER` lint, which can be used to test out this PR. The intent is to use the field to allow Cargo to determine the date of future breakage (as described in [RFC 2834](https://github.com/rust-lang/rfcs/blob/master/text/2834-cargo-report-future-incompat.md)) without needing to parse the diagnostic itself. cc `@pnkfelix`

rustc_session: Remove lint store from `Session` It was added in rust-lang#75534, but after the cleanup in rust-lang#87070 it's no longer necessary.

rust-highfive assigned ecstatic-morse Aug 14, 2020

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