Move deny(warnings) into rustbuild

[Mark-Simulacrum]

This permits easier iteration without having to worry about warnings
being denied.

Fixes #49517

[Mark-Simulacrum]

r? @alexcrichton

cc @eddyb @RalfJung

[RalfJung]

Just to be sure, the ./configure-based build that runs on CI still has deny(warnings), right? I can't see any changes to the configure script.

[Mark-Simulacrum]

The default behavior is to deny warnings, so yes.

[steveklabnik]

is this change intentional? seems odd

[alexcrichton]

Sounds like a great idea to me! I wouldn't personally feel the need to have the command line flag as well, but now that it's done there's certainly no harm.

I wonder if it'd be best now though to disable warnings by default and update src/ci/run.sh so all CI builders deny warnings?

[alexcrichton]

er, disable deny(warnings) by default*

[RalfJung]

@cramertj expressed at the all-hands that he preferred deny(warnings) on in the compiler. Also, it seems it took 3 years for someone to take enough offense with the default to actually raise the point in the issue tracker, so maybe it's not such a bad default? Either way, that seems like a separate discussion form just providing the option.

I am perfectly fine changing my cargo.toml, so this patch already makes me happy :)

[Mark-Simulacrum]

@alexcrichton I think feedback above indicates we may not want to make this a default, and either way that seems like something we should do down the line. I fixed the nit that @steveklabnik noticed (thanks!) so I believe this is good to go.

[alexcrichton]

@bors: r+

Seems reasonable to me!

[bors]

📌 Commit b5a3b29 has been approved by alexcrichton

[bors]

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

[Mark-Simulacrum]

@bors r=alexcrichton

[bors]

📌 Commit c115cc6 has been approved by alexcrichton

[bors]

⌛ Testing commit c115cc6 with merge 8b36e1691a8403b545655840b3eb2e1530327ede...

[bors]

💔 Test failed - status-appveyor

[RalfJung]

Seems like we now run deny(warnings) on more code than before? The windows build says

   Compiling rustc-main v0.0.0 (file:///C:/projects/rust/src/rustc)
error: unused attribute
  --> rustc\rustc.rs:16:47
   |
16 | #[cfg_attr(all(windows, target_env = "msvc"), link_args = "/STACK:16777216")]
   |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `-D unused-attributes` implied by `-D warnings`
error: aborting due to previous error

[Mark-Simulacrum]

@ishitatsuyuki Any thoughts on that error? It looks like this code was added by you in #48575; I'm wondering if we should ignore that warning or if it's just that we don't properly consider link_args...

[alexcrichton]

I believe I tested that out locally a few days ago and rustc is basically incorrectly reporting that it's an unused attribute, you'll probably want to attach an #[allow] annotation there for now

[Mark-Simulacrum]

Okay, added a commit to allow that warning.

@bors r=alexcrichton

[bors]

📌 Commit 46aad25 has been approved by alexcrichton

[bors]

⌛ Testing commit 46aad25ab96aa239ccbc0b23f3f11520b2252376 with merge 5c943cf5a213407d51eba89edab56065b8df5696...

[bors]

💔 Test failed - status-appveyor

[RalfJung]

And another one of the same:

   Compiling rustdoc-tool v0.0.0 (file:///C:/projects/rust/src/tools/rustdoc)
error: unused attribute
  --> tools\rustdoc\main.rs:14:47
   |
14 | #[cfg_attr(all(windows, target_env = "msvc"), link_args = "/STACK:16777216")]
   |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `-D unused-attributes` implied by `-D warnings`
error: aborting due to previous error

[Mark-Simulacrum]

@bors r=alexcrichton

[bors]

📌 Commit 53718d2 has been approved by alexcrichton

[bors]

⌛ Testing commit 53718d2 with merge 43e994c...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 43e994c to master...