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

Clarify what -D warnings or -F warnings does #46136

Merged
merged 2 commits into from
Dec 6, 2017
Merged

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Nov 20, 2017

They set all lints currently on the warning level to deny or forbid,
respectively.

They set all lints currently on the warning level to `deny` or `forbid`,
respectively.
@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@tbu-
Copy link
Contributor Author

tbu- commented Nov 20, 2017

Tested what it actually does by running rustc on the following file

pub fn main() {
    Box::new(());
}

with the lint box-pointers.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 21, 2017
@shepmaster
Copy link
Member

Ping from triage @pnkfelix — will you have some time to review this?

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.

I think we can do a bit better.

@@ -980,7 +980,7 @@ Available lint options:
println!("Lint groups provided by rustc:\n");
println!(" {} {}", padded("name"), "sub-lints");
println!(" {} {}", padded("----"), "---------");
println!(" {} {}", padded("warnings"), "all built-in lints");
println!(" {} {}", padded("warnings"), "all lints activated to the warning level");
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I don't find this phrasing very clear. "The warning level" sounds to me like something that one can control independently. Perhaps "all lints set to warn", or "all lints that are set to issue warnings"?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have a lint that defaults to allow, but you set it explicitly to warn, and then you do #[deny(warnings)], is that lint then denied?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I don't find this phrasing very clear.

Yea, I struggled to find a good wording. Thanks for the suggestions, I used the second one for now.

If you have a lint that defaults to allow, but you set it explicitly to warn, and then you do #[deny(warnings)], is that lint then denied?

Yes.

#![deny(warnings)]
fn main() {
    Box::new(());
}
$ rustc -W box-pointers a.rs
error: type uses owned (Box type) pointers: std::boxed::Box<()>
 --> box.rs:3:5
  |
3 |     Box::new(());
  |     ^^^^^^^^^^^^
  |
note: lint level defined here
 --> box.rs:1:9
  |
1 | #![deny(warnings)]
  |         ^^^^^^^^
  = note: #[deny(box_pointers)] implied by #[deny(warnings)]

error: aborting due to previous error

@nikomatsakis
Copy link
Contributor

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 5, 2017

📌 Commit e1e1dcc has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 6, 2017

⌛ Testing commit e1e1dcc with merge 3de30dd00edbc89613e93900a48ddfa4f8eb06a7...

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Dec 6, 2017
Clarify what `-D warnings` or `-F warnings` does

They set all lints currently on the warning level to `deny` or `forbid`,
respectively.
@kennytm
Copy link
Member

kennytm commented Dec 6, 2017

@bors retry — Prioritize for rollup (which included this PR...)

bors added a commit that referenced this pull request Dec 6, 2017
Rollup of 7 pull requests

- Successful merges: #46136, #46378, #46431, #46483, #46495, #46502, #46512
- Failed merges:
@bors bors merged commit e1e1dcc into rust-lang:master Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants