Make `--force-warns` a normal lint level option #86970

inquisitivecrystal · 2021-07-08T08:59:27Z

Now that ForceWarn is a lint level, there's no reason --force-warns should be treated differently from other options that set lint levels. This merges the ForceWarn handling in with the other lint level command line options. It also unifies all of the relevant selection logic in compiler/rustc_lint/src/levels.rs, rather than having some of it weirdly elsewhere.

Fixes #86958, which arose from the special-cased handling of ForceWarn having had an error in it.

rust-highfive · 2021-07-08T08:59:29Z

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

rylev

Awesome! We should add some tests. Specifically:

--force-warns works with a bogus lint name
--force-warns and -F are not overwritten by later lint options

compiler/rustc_lint/src/levels.rs

rylev · 2021-07-08T09:25:39Z

Now that force-warns is processed with other lints, you'll need to make sure that --cap-lints doesn't cap force-warns. This is somewhat duplicate logic found here.

Also, this PR should partially fix #70819 (the command line part of it) and thus it partially supersedes #85298.

inquisitivecrystal · 2021-07-09T07:01:43Z

I think I've made all the requested changes. Let me know if I missed anything!

varkor · 2021-07-11T13:52:42Z

r? @rylev

rylev · 2021-07-12T13:12:28Z

Unfortunately, I don't have review rights. I'll do a review though, but someone else will have to invoke bors.

r? @davidtwco

rylev

Looking good! The issue with the E0602 being emitted multiple times is strange, but I don't think it's a blocker.

rylev · 2021-07-12T13:13:18Z

src/test/ui/lint/cli-unknown-force-warn.stderr

+   |
+   = note: requested on the command line with `--force-warns foo_qux`
+
+error[E0602]: unknown lint: `foo_qux`


Any idea why this error gets emitted multiple times?

This happens for some other cases as well. I think it's a product of the way the tests are run, though I'm not totally sure?

rylev · 2021-07-12T13:16:19Z

compiler/rustc_lint/src/levels.rs

            let lint_flag_val = Symbol::intern(lint_name);

            let ids = match store.find_lints(&lint_name) {
                Ok(ids) => ids,
                Err(_) => continue, // errors handled in check_lint_name_cmdline above
            };
            for id in ids {
+                // ForceWarn and Forbid cannot be overriden
+                if let Some((Level::ForceWarn | Level::Forbid, _)) = specs.get(&id) {


Is this behavior documented anywhere?

Sort of. However, the fact that forbid currently overrides the others isn't specified, and force-warns isn't documented at all because it's unstable. Given that the behavior is intrinsic to the definition of these lint levels, I don't think it's too surprising. It might be worth documenting it in the long run though?

davidtwco

LGTM too, I'd like to see @rylev's comments resolved but I don't think those comments ought to block this landing.

davidtwco · 2021-07-15T10:50:40Z

@bors r+

bors · 2021-07-15T10:50:42Z

📌 Commit dbdce6e has been approved by davidtwco

…vidtwco Make `--force-warns` a normal lint level option Now that `ForceWarn` is a lint level, there's no reason `--force-warns` should be treated differently from other options that set lint levels. This merges the `ForceWarn` handling in with the other lint level command line options. It also unifies all of the relevant selection logic in `compiler/rustc_lint/src/levels.rs`, rather than having some of it weirdly elsewhere. Fixes rust-lang#86958, which arose from the special-cased handling of `ForceWarn` having had an error in it.

bors · 2021-07-16T12:09:24Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout force-warn (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self force-warn --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging compiler/rustc_session/src/options.rs
CONFLICT (content): Merge conflict in compiler/rustc_session/src/options.rs
Auto-merging compiler/rustc_session/src/config.rs
Automatic merge failed; fix conflicts and then commit the result.

bors · 2021-07-16T12:35:59Z

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

inquisitivecrystal · 2021-07-18T07:25:48Z

Rebased!

davidtwco · 2021-07-19T11:44:37Z

@bors r+

bors · 2021-07-19T11:44:38Z

📌 Commit 10cb380 has been approved by davidtwco

davidtwco · 2021-07-19T11:45:06Z

@bors delegate=rylev

bors · 2021-07-19T11:45:08Z

✌️ @rylev can now approve this pull request

bors · 2021-07-19T13:18:09Z

⌛ Testing commit 10cb380 with merge b543e0d...

bors · 2021-07-19T16:03:29Z

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing b543e0d to master...

rust-highfive assigned varkor Jul 8, 2021

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

rylev suggested changes Jul 8, 2021

View reviewed changes

compiler/rustc_lint/src/levels.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rust-highfive assigned rylev and unassigned varkor Jul 11, 2021

rust-highfive assigned davidtwco and unassigned rylev Jul 12, 2021

rylev approved these changes Jul 12, 2021

View reviewed changes

rylev mentioned this pull request Jul 13, 2021

Stabilize "force warn" option #86516

Closed

davidtwco approved these changes Jul 15, 2021

View reviewed changes

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 15, 2021

GuillaumeGomez mentioned this pull request Jul 15, 2021

Rollup of 6 pull requests #87164

Closed

GuillaumeGomez mentioned this pull request Jul 15, 2021

Rollup of 4 pull requests #87165

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 16, 2021

Make --force-warns a normal lint level option

2f2db99

Add tests for command line lint control

10cb380

inquisitivecrystal force-pushed the force-warn branch from dbdce6e to 10cb380 Compare July 18, 2021 06:14

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 Jul 19, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 19, 2021

bors merged commit b543e0d into rust-lang:master Jul 19, 2021

rustbot added this to the 1.55.0 milestone Jul 19, 2021

dtolnay mentioned this pull request Feb 29, 2024

ICE: must_produce_diag: trimmed_def_paths called but no diagnostics emitted #121774

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `--force-warns` a normal lint level option #86970

Make `--force-warns` a normal lint level option #86970

inquisitivecrystal commented Jul 8, 2021

rust-highfive commented Jul 8, 2021

rylev left a comment

This comment has been minimized.

rylev commented Jul 8, 2021

inquisitivecrystal commented Jul 9, 2021

varkor commented Jul 11, 2021

rylev commented Jul 12, 2021

rylev left a comment

rylev Jul 12, 2021

inquisitivecrystal Jul 15, 2021 •

edited

Loading

rylev Jul 12, 2021

inquisitivecrystal Jul 15, 2021

davidtwco left a comment

davidtwco commented Jul 15, 2021

bors commented Jul 15, 2021

bors commented Jul 16, 2021

bors commented Jul 16, 2021

inquisitivecrystal commented Jul 18, 2021

davidtwco commented Jul 19, 2021

bors commented Jul 19, 2021

davidtwco commented Jul 19, 2021

bors commented Jul 19, 2021

bors commented Jul 19, 2021

bors commented Jul 19, 2021

Make --force-warns a normal lint level option #86970

Make --force-warns a normal lint level option #86970

Conversation

inquisitivecrystal commented Jul 8, 2021

rust-highfive commented Jul 8, 2021

rylev left a comment

Choose a reason for hiding this comment

This comment has been minimized.

rylev commented Jul 8, 2021

inquisitivecrystal commented Jul 9, 2021

varkor commented Jul 11, 2021

rylev commented Jul 12, 2021

rylev left a comment

Choose a reason for hiding this comment

rylev Jul 12, 2021

Choose a reason for hiding this comment

inquisitivecrystal Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

rylev Jul 12, 2021

Choose a reason for hiding this comment

inquisitivecrystal Jul 15, 2021

Choose a reason for hiding this comment

davidtwco left a comment

Choose a reason for hiding this comment

davidtwco commented Jul 15, 2021

bors commented Jul 15, 2021

bors commented Jul 16, 2021

bors commented Jul 16, 2021

inquisitivecrystal commented Jul 18, 2021

davidtwco commented Jul 19, 2021

bors commented Jul 19, 2021

davidtwco commented Jul 19, 2021

bors commented Jul 19, 2021

bors commented Jul 19, 2021

bors commented Jul 19, 2021

Make `--force-warns` a normal lint level option #86970

Make `--force-warns` a normal lint level option #86970

inquisitivecrystal Jul 15, 2021 •

edited

Loading