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

mk: Move from -D warnings to #![deny(warnings)] #31120

Merged
merged 3 commits into from
Jan 27, 2016

Conversation

alexcrichton
Copy link
Member

This commit removes the -D warnings flag being passed through the makefiles to
all crates to instead be a crate attribute. We want these attributes always
applied for all our standard builds, and this is more amenable to Cargo-based
builds as well.

Note that all deny(warnings) attributes are gated with a cfg(stage0)
attribute currently to match the same semantics we have today

@rust-highfive
Copy link
Collaborator

r? @brson

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

@alexcrichton
Copy link
Member Author

One change I also noticed was that the rustc_mir crate did not previously have an #[unstable] tag, which means that this compiles in stable Rust:

extern crate rustc_mir;

fn main() {}

I added the #[unstable] tag here, but I would be quite surprised if that actually caused breakage. @brson do you think it's worth adding to relnotes?

@brson brson added relnotes Marks issues that should be documented in the release notes of the next release. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jan 22, 2016
@brson
Copy link
Contributor

brson commented Jan 22, 2016

@alexcrichton It may not be worth mentioning but I'll tag it and think about it later. Also nominating for beta, maybe just the rustc_mir crate fix.

Here's an issue to lint for this problem: #31122

@alexcrichton
Copy link
Member Author

Oh let me rearrange the commits for the new unstable attribute to be in its own commit to be easily cherry-picked in that case.

@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from 85ab967 to 3a107c5 Compare January 22, 2016 18:17
@alexcrichton
Copy link
Member Author

rearranged

@brson
Copy link
Contributor

brson commented Jan 22, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Jan 22, 2016

📌 Commit 3a107c5 has been approved by brson

@bors
Copy link
Contributor

bors commented Jan 23, 2016

⌛ Testing commit 3a107c5 with merge 234e3dd...

@bors
Copy link
Contributor

bors commented Jan 23, 2016

💔 Test failed - auto-mac-64-opt

@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from 3a107c5 to ae8f816 Compare January 23, 2016 07:51
@alexcrichton
Copy link
Member Author

@bors: r=brson ae8f816

@bors
Copy link
Contributor

bors commented Jan 23, 2016

⌛ Testing commit ae8f816 with merge 96b734f...

@bors
Copy link
Contributor

bors commented Jan 23, 2016

💔 Test failed - auto-linux-64-x-android-t

@bors
Copy link
Contributor

bors commented Jan 23, 2016

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

@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from ae8f816 to fc8d780 Compare January 24, 2016 18:43
@alexcrichton
Copy link
Member Author

@bors: r=brson fc8d780

@bors
Copy link
Contributor

bors commented Jan 24, 2016

⌛ Testing commit fc8d780 with merge 5ead8cf...

@bors
Copy link
Contributor

bors commented Jan 24, 2016

💔 Test failed - auto-linux-64-x-android-t

This commit removes the `-D warnings` flag being passed through the makefiles to
all crates to instead be a crate attribute. We want these attributes always
applied for all our standard builds, and this is more amenable to Cargo-based
builds as well.

Note that all `deny(warnings)` attributes are gated with a `cfg(stage0)`
attribute currently to match the same semantics we have today
Wouldn't want to be able to link to this on stable Rust!
@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from fc8d780 to cc4db8b Compare January 25, 2016 04:37
@alexcrichton
Copy link
Member Author

@bors: r=brson cc4db8b

@alexcrichton
Copy link
Member Author

@bors: r=brson 790b862

@tamird
Copy link
Contributor

tamird commented Jan 25, 2016

travis failed btw

src/libstd/process.rs:849:5: 866:6 error: duplicate definition of value `test_inherit_env` [E0428]
src/libstd/process.rs:849     fn test_inherit_env() {
src/libstd/process.rs:850         use env;
src/libstd/process.rs:851 
src/libstd/process.rs:852         let mut result = env_cmd().output().unwrap();
src/libstd/process.rs:853         let output = String::from_utf8(result.stdout).unwrap();
src/libstd/process.rs:854 
                          ...
src/libstd/process.rs:849:5: 866:6 help: run `rustc --explain E0428` to see a detailed explanation
src/libstd/process.rs:829:5: 845:6 note: first definition of value `test_inherit_env` here
src/libstd/process.rs:829     fn test_inherit_env() {
src/libstd/process.rs:830         use env;
src/libstd/process.rs:831 
src/libstd/process.rs:832         let result = env_cmd().output().unwrap();
src/libstd/process.rs:833         let output = String::from_utf8(result.stdout).unwrap();
src/libstd/process.rs:834 

#[test]
#[cfg_attr(target_os = "android", ignore)]
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this not be ignored on android?

@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from 790b862 to 9d95a22 Compare January 26, 2016 00:57
@alexcrichton
Copy link
Member Author

@bors: r=brson 9d95a22

@bors
Copy link
Contributor

bors commented Jan 26, 2016

⌛ Testing commit 9d95a22 with merge 2518a71...

@bors
Copy link
Contributor

bors commented Jan 26, 2016

💔 Test failed - auto-win-msvc-64-opt

@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from 9d95a22 to cdc1c2c Compare January 26, 2016 16:29
@alexcrichton
Copy link
Member Author

@bors: r=brson cdc1c2c

@bors
Copy link
Contributor

bors commented Jan 26, 2016

⌛ Testing commit cdc1c2c with merge 669130e...

@bors
Copy link
Contributor

bors commented Jan 26, 2016

💔 Test failed - auto-mac-64-nopt-t

The deny(warnings) attribute is now enabled for tests so we need to weed out
these warnings as well.
@alexcrichton alexcrichton force-pushed the attribute-deny-warnings branch from cdc1c2c to cb343c3 Compare January 26, 2016 17:29
@alexcrichton
Copy link
Member Author

@bors: r=brson cb343c3

bors added a commit that referenced this pull request Jan 26, 2016
This commit removes the `-D warnings` flag being passed through the makefiles to
all crates to instead be a crate attribute. We want these attributes always
applied for all our standard builds, and this is more amenable to Cargo-based
builds as well.

Note that all `deny(warnings)` attributes are gated with a `cfg(stage0)`
attribute currently to match the same semantics we have today
@bors
Copy link
Contributor

bors commented Jan 26, 2016

⌛ Testing commit cb343c3 with merge 4b61585...

@alexcrichton
Copy link
Member Author

flagging as beta-accepted, but only the commit alexcrichton@4b3c355

@alexcrichton alexcrichton added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jan 27, 2016
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants