Ensure edition lints and internal lints are enabled with deny-warnings=false #64098

Mark-Simulacrum · 2019-09-02T18:27:40Z

Previously we only passed the deny command line flags if deny-warnings was enabled, but now we either pass -W... or -D... for each of the flags as appropriate.

This is also a breaking change to x.py as it changes --warnings=allow to --warnings=warn which is what that flag actually did; we don't have an allow warnings mode.

rust-highfive · 2019-09-02T18:27:50Z

r? @alexcrichton

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

alexcrichton · 2019-09-03T15:36:04Z

While you're touching this code, would you be up for tweaking how this is handled? Ideally this would be removed from bin/rustc.rs entirely and just moved to RUSTFLAGS when we invoke Cargo, and I think that would work?

Mark-Simulacrum · 2019-09-03T15:43:32Z

We don't set this for external tools (submodules) and I'd rather not thread that information through into Builder::cargo since it's a bit out there so it'd be somewhat hard I think to do that in a good way, but happy to implement something if you have good ideas.

Mark-Simulacrum · 2019-09-03T16:25:20Z

It also looks like RUSTFLAGS might be somewhat a poor fit for us since Cargo will AFAICT only pass that to non-build-scripts (and presumably non-proc-macros; though not sure there) due to us passing --target unless I've misinterpreted something.

src/bootstrap/bin/rustc.rs

alexcrichton · 2019-09-04T16:57:56Z

We already have a mode in Builder::cargo, and we can presumably only enable this for non-Tool builds?

Mark-Simulacrum · 2019-09-04T17:10:13Z

Yeah, I think it might be easiest to just add the tool type (submodule or not) to the Mode enum; I do think it makes some sense to try to deny warnings for the in-tree tools. I'll take a look at that in a bit.

We never allowed the warnings, only made them not denied.

Mark-Simulacrum · 2019-09-04T18:50:42Z

Okay, moved most of the warning code out. Some of it is dependent on the specific crate we're compiling which means we can't actually move it out unfortunately, not sure if we can do anything about that though :/

alexcrichton · 2019-09-04T23:49:54Z

@bors: r+

bors · 2019-09-04T23:49:55Z

📌 Commit a2384cb has been approved by alexcrichton

…crichton Ensure edition lints and internal lints are enabled with deny-warnings=false Previously we only passed the deny command line flags if deny-warnings was enabled, but now we either pass -W... or -D... for each of the flags as appropriate. This is also a breaking change to x.py as it changes `--warnings=allow` to `--warnings=warn` which is what that flag actually did; we don't have an allow warnings mode.

Centril · 2019-09-05T09:47:23Z

Failed in #64168 (comment), @bors r-

bors · 2019-09-05T14:40:21Z

📌 Commit c6f868f has been approved by alexcrichton

…crichton Ensure edition lints and internal lints are enabled with deny-warnings=false Previously we only passed the deny command line flags if deny-warnings was enabled, but now we either pass -W... or -D... for each of the flags as appropriate. This is also a breaking change to x.py as it changes `--warnings=allow` to `--warnings=warn` which is what that flag actually did; we don't have an allow warnings mode.

@ghost

Rollup of 9 pull requests Successful merges: - #64067 (Remove no-prefer-dynamic from valgrind tests) - #64078 (compiletest: disable -Aunused for run-pass tests) - #64096 (Fix regex replacement in theme detection) - #64098 (Ensure edition lints and internal lints are enabled with deny-warnings=false) - #64166 (Better way of conditioning the sanitizer builds) - #64189 (annotate-snippet emitter: Deal with multispans from macros, too) - #64202 (Fixed grammar/style in some error messages) - #64206 (annotate-snippet emitter: Update an issue number) - #64208 (it's more pythonic to use 'is not None' in python files) Failed merges: r? @ghost

Centril · 2019-09-06T17:07:28Z

@bors p=50

Want to eliminate this PR as the cause of failure in #64164 and #64216.

Centril · 2019-09-06T17:08:03Z

@bors rollup=never

@Centril

[WIP] Verify if #64098 causes link errors cc @Centril for `try` command

Centril · 2019-09-06T18:50:35Z

Failed in #64232 (comment), @bors r- p=0 rollup=maybe

Raw log: https://rust-lang.github.io/rust-log-analyzer/log-viewer/#https://dev.azure.com/rust-lang/e71b0ddf-dd27-435a-873c-e30f86eea377/_apis/build/builds/7533/logs/19

Mark-Simulacrum · 2019-09-06T21:05:09Z

Determined the cause of the problem. The dist-various-2 builder uses CARGO_TARGET_X86_64_FUCHSIA_RUSTFLAGS (among others) to pass some linker arguments into rustc for that target, which Cargo will ignore if RUSTFLAGS is set in the environment. I don't think this PR is the right place to try and refactor our handling of rustflags and how to correctly pull in, so I've dropped the last two commits.

r? @alexcrichton on this decision

alexcrichton · 2019-09-06T22:31:08Z

@bors: r+

bors · 2019-09-06T22:31:09Z

📌 Commit fda251b has been approved by alexcrichton

…crichton Ensure edition lints and internal lints are enabled with deny-warnings=false Previously we only passed the deny command line flags if deny-warnings was enabled, but now we either pass -W... or -D... for each of the flags as appropriate. This is also a breaking change to x.py as it changes `--warnings=allow` to `--warnings=warn` which is what that flag actually did; we don't have an allow warnings mode.

@ghost

Rollup of 7 pull requests Successful merges: - #64023 (libstd fuchsia fixes) - #64098 (Ensure edition lints and internal lints are enabled with deny-warnings=false) - #64139 (Migrate internal diagnostic registration to macro_rules) - #64226 (Aggregation of cosmetic changes made during work on REPL PRs: libsyntax) - #64227 (Aggregation of cosmetic changes made during work on REPL PRs: librustc) - #64235 (Upgrade env_logger to 0.6) - #64258 (compiletest: Match suffixed environments) Failed merges: r? @ghost

rust-highfive assigned alexcrichton Sep 2, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2019

ollie27 reviewed Sep 4, 2019

View reviewed changes

src/bootstrap/bin/rustc.rs Outdated Show resolved Hide resolved

Mark-Simulacrum force-pushed the always-warn branch 2 times, most recently from 1dbc772 to 6f959fa Compare September 4, 2019 13:09

Mark-Simulacrum 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2019

Mark-Simulacrum added 2 commits September 4, 2019 14:21

Ensure all warnings are emitted even on warnings=warn

c86ea34

Rename --warnings=allow to --warnings=warn

fda251b

We never allowed the warnings, only made them not denied.

Mark-Simulacrum force-pushed the always-warn branch from 6f959fa to a2384cb Compare September 4, 2019 18:50

Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2019

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 Sep 4, 2019

Centril mentioned this pull request Sep 5, 2019

Rollup of 11 pull requests #64164

Closed

Centril mentioned this pull request Sep 5, 2019

Rollup of 11 pull requests #64168

Closed

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 Sep 5, 2019

Centril mentioned this pull request Sep 6, 2019

Rollup of 9 pull requests #64216

Closed

bors added a commit that referenced this pull request Sep 6, 2019

Auto merge of #64232 - mati865:tmp-error-test, r=<try>

e942285

[WIP] Verify if #64098 causes link errors cc @Centril for `try` command

mati865 mentioned this pull request Sep 6, 2019

[WIP] Verify if #64098 causes link errors #64232

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 Sep 6, 2019

Mark-Simulacrum force-pushed the always-warn branch from c6f868f to fda251b Compare September 6, 2019 21:00

Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 6, 2019

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 Sep 6, 2019

Centril mentioned this pull request Sep 7, 2019

Rollup of 4 pull requests #64253

Closed

Centril mentioned this pull request Sep 7, 2019

Rollup of 6 pull requests #64257

Closed

Centril mentioned this pull request Sep 7, 2019

Rollup of 7 pull requests #64264

Merged

bors merged commit fda251b into rust-lang:master Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure edition lints and internal lints are enabled with deny-warnings=false #64098

Ensure edition lints and internal lints are enabled with deny-warnings=false #64098

Mark-Simulacrum commented Sep 2, 2019

rust-highfive commented Sep 2, 2019

alexcrichton commented Sep 3, 2019

Mark-Simulacrum commented Sep 3, 2019

Mark-Simulacrum commented Sep 3, 2019

alexcrichton commented Sep 4, 2019

Mark-Simulacrum commented Sep 4, 2019

Mark-Simulacrum commented Sep 4, 2019

alexcrichton commented Sep 4, 2019

bors commented Sep 4, 2019

Centril commented Sep 5, 2019

bors commented Sep 5, 2019

Centril commented Sep 6, 2019

Centril commented Sep 6, 2019

Centril commented Sep 6, 2019

Mark-Simulacrum commented Sep 6, 2019

alexcrichton commented Sep 6, 2019

bors commented Sep 6, 2019

Ensure edition lints and internal lints are enabled with deny-warnings=false #64098

Ensure edition lints and internal lints are enabled with deny-warnings=false #64098

Conversation

Mark-Simulacrum commented Sep 2, 2019

rust-highfive commented Sep 2, 2019

alexcrichton commented Sep 3, 2019

Mark-Simulacrum commented Sep 3, 2019

Mark-Simulacrum commented Sep 3, 2019

alexcrichton commented Sep 4, 2019

Mark-Simulacrum commented Sep 4, 2019

Mark-Simulacrum commented Sep 4, 2019

alexcrichton commented Sep 4, 2019

bors commented Sep 4, 2019

Centril commented Sep 5, 2019

bors commented Sep 5, 2019

Centril commented Sep 6, 2019

Centril commented Sep 6, 2019

Centril commented Sep 6, 2019

Mark-Simulacrum commented Sep 6, 2019

alexcrichton commented Sep 6, 2019

bors commented Sep 6, 2019