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.

Sign up for GitHub

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

Jump to bottom

Support for force-warns #85788

Merged
merged 9 commits into from
Jun 4, 2021
Merged

Support for force-warns #85788

merged 9 commits into from
Jun 4, 2021

Conversation

rylev
Copy link
Member

@rylev rylev commented May 28, 2021 •
edited
Loading

Implements #85512.

This PR adds a new command line option force-warns which will force the provided lints to warn even if they are allowed by some other mechanism such as #![allow(warnings)].

Some remaining issues:

  • force-warn for edition lints #85512 mentions that force-warns should also be capable of taking lint groups instead of individual lints. This is not implemented.
  • If a lint has a higher warning level than warn, this will cause that lint to warn instead. We probably want to allow the lint to error if it is set to a higher lint and is not allowed somewhere else.
  • One test is currently ignored because it's not working - when a deny-by-default lint is allowed, it does not currently warn under force-warns. I'm not sure why, but I wanted to get this in before the weekend.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 28, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
nikomatsakis
nikomatsakis requested changes Jun 2, 2021
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Left some notes on how to make this support lint groups (I think)

compiler/rustc_lint/src/levels.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/lint.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/lint.rs Outdated Show resolved Hide resolved
nikomatsakis
nikomatsakis requested changes Jun 2, 2021
View reviewed changes
compiler/rustc_middle/src/lint.rs Outdated Show resolved Hide resolved
nikomatsakis
nikomatsakis reviewed Jun 2, 2021
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Can you add an entry to the unstable book compiler flags listing:

https://github.com/rust-lang/rust/tree/master/src/doc/unstable-book/src/compiler-flags

I'm not sure if there is a SUMMARY.md to edit

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot
Copy link
Contributor

cjgillot commented Jun 2, 2021

Can this feature be implemented more simply as an additional lint level, similar to "forbid" but for warnings?

@rylev
Copy link
Member Author

rylev commented Jun 3, 2021

@cjgillot I think it would be possible, but I'm not convinced it would be more simple. We want these lints to behave as they currently do in all situations except in the one case where we want to force them to warn. Having another lint level would force more of a cognitive burden on users and documentation maintenance.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 3, 2021

📌 Commit 896898e has been approved by nikomatsakis

@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 Jun 3, 2021
@rylev rylev changed the title Initial support for force-warns Support for force-warns Jun 4, 2021
@bors
Copy link
Contributor

bors commented Jun 4, 2021

⌛ Testing commit 896898e with merge 595088d...

@bors
Copy link
Contributor

bors commented Jun 4, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 595088d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 4, 2021
@bors bors merged commit 595088d into rust-lang:master Jun 4, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 4, 2021
@bors bors mentioned this pull request Jun 4, 2021
Merged
@rylev rylev deleted the force-warns branch June 4, 2021 16:32
This was referenced Jun 4, 2021
Closed
Merged
@cjgillot cjgillot mentioned this pull request Jun 4, 2021
Merged
@m-ou-se m-ou-se mentioned this pull request Jun 7, 2021
Closed
4 tasks
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2021
@bors
Auto merge of rust-lang#86009 - cjgillot:fwarn, r=davidtwco
8971fff 
Make ForceWarn a lint level.

Follow-up to rust-lang#85788
cc `@rylev`
@m-ou-se m-ou-se mentioned this pull request Sep 3, 2021
Closed
@ehuss ehuss mentioned this pull request Nov 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
7 participants
@rylev @rust-log-analyzer @cjgillot @nikomatsakis @bors @rust-highfive @rustbot