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 a future-incompat-report has its command-line level capped #78663

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

Aaron1011
Copy link
Member

Fixes #78660

With PR #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).

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).
LL | ["hi"].into_iter();
| ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
|
= note: `-D array-into-iter` implied by `-D warnings`
Copy link
Member Author

Choose a reason for hiding this comment

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

We should suppress this message when a lint only shows up in the future incompat report. However, this only affects nightly, so I'll address it in a follow up PR.

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2020
@JohnTitor
Copy link
Member

Seems highfive ignored this PR :/ Let's r? @pnkfelix as they reviewed #75534.

@jyn514 jyn514 added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 2, 2020
Level::Warn => "-W",
Level::Deny => "-D",
Level::Forbid => "-F",
Level::Allow => panic!(),
Level::Allow => "-A",
Copy link
Member

@tmandry tmandry Nov 3, 2020

Choose a reason for hiding this comment

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

This would still be unexpected, no?

(feel free to ignore if I'm wrong or this is otherwise intentional)

Copy link
Member Author

Choose a reason for hiding this comment

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

You can pass -A warnings on the command line. Previously, we would never show a specific help message, since we would bail out when the lint is allowed. Now, we may emit a future-incompat report, where we will want to specify the command-line flag.

@tmandry
Copy link
Member

tmandry commented Nov 3, 2020

r? @tmandry

Looks good, r=me after comment

@Aaron1011
Copy link
Member Author

@bors r=tmandry

@bors
Copy link
Contributor

bors commented Nov 3, 2020

📌 Commit 6c1f15f has been approved by tmandry

@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 Nov 3, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2020
Rollup of 8 pull requests

Successful merges:

 - rust-lang#78376 (Treat trailing semicolon as a statement in macro call)
 - rust-lang#78400 (Fix unindent in doc comments)
 - rust-lang#78575 (Add a test for compiletest rustc-env & unset-rustc-env directives)
 - rust-lang#78616 (Document -Zinstrument-coverage)
 - rust-lang#78663 (Fix ICE when a future-incompat-report has its command-line level capped)
 - rust-lang#78664 (Fix intrinsic size_of stable link)
 - rust-lang#78668 (inliner: Remove redundant loop)
 - rust-lang#78676 (add mipsel-unknown-none target)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8894c90 into rust-lang:master Nov 3, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. 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.

ICE in 1.49.0 nightly 2020-11-01 on windows
7 participants