nightly regression?: `dead_code` detection with `-Zfuture-incompat-test`

[weihanglo]

Code

Test against rust-lang/cargo@7e9de3f with the current nightly.

cargo test --test testsuite future_incompat_report::test_multi_crate

Expected warning from Cargo v.s Actual

- warning: the following packages contain code that will be rejected by a future version of Rust: first-dep v0.0.1, second-dep v0.0.2
+ warning: the following packages contain code that will be rejected by a future version of Rust: first-dep v0.0.1, foo v0.0.0 (/home/runner/work/cargo/cargo/target/tmp/cit/t1414/foo), second-dep v0.0.2

Note that the primary package foo is included because nightly compiler starts reporting main() as a dead code in conjunction with -Zfuture-incompat-test, which it used to display dependencies only.

And yes in Cargo we use -Zfuture-incompat-test to fire a future-incompat warning for tests. This doesn't work anymore. See https://github.com/rust-lang/cargo/actions/runs/5850845391/job/15860819020?pr=12489

Version it worked on

searched nightlies: from nightly-2023-08-01 to nightly-2023-08-14
regressed nightly: nightly-2023-08-13
searched commit range: a6f8aa5...28eb857
regressed commit: 1e836d1
regressed PR: #114710

bisected with cargo-bisect-rustc v0.6.6

Host triple: aarch64-apple-darwin
Reproduce with:

cargo bisect-rustc --start 2023-08-01 --end 2023-08-14 -- test --test testsuite test_multi_crate

The versions of Cargo in nightly-2023-08-12 and nightly-2023-08-13 are the same, but only 08-13 regressed. Hence, I excluded Cargo as a source of this regression. I'll continue investigating but any help is pretty much welcome!

Not a proposed solution

Taking out the insertion from the if block works, though I believe it is not the correct way to solve this.

diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
index 2936c8fac7d..33d3921f926 100644
--- a/compiler/rustc_passes/src/dead.rs
+++ b/compiler/rustc_passes/src/dead.rs
@@ -318,9 +318,7 @@ fn mark_live_symbols(&mut self) {
                 // this "duplication" is essential as otherwise a function with `#[expect]`
                 // called from a `pub fn` may be falsely reported as not live, falsely
                 // triggering the `unfulfilled_lint_expectations` lint.
-                if comes_from_allow_expect != ComesFromAllowExpect::Yes {
-                    self.live_symbols.insert(id);
-                }
+                self.live_symbols.insert(id);
                 self.visit_node(node);
             }
         }

[weihanglo]

cc @Urgau, as it looks like you made the change. Sorry I don't really understand rustc internals. Can't help here.

[weihanglo]

To reproduce:

cargo new pkg
cd pkg
RUSTFLAGS="-Zfuture-incompat-test" cargo +nightly t

Output:

    Finished test [unoptimized + debuginfo] target(s) in 0.00s
warning: the following packages contain code that will be rejected by a future version of Rust: pkg v0.1.0 (/projects/pkg)
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
     Running unittests src/main.rs (target/debug/deps/pkg-c6163123c7b0e1b6)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

It only happend with cargo test. More specifically, cargo b --bins doesn't have that warnings, whereas cargo b --tests contains warnings from the root binary package. Might be something related to rustc --test flag?

[Urgau]

Thanks you for the ping and the reproducer. ❤️

---

Before #114710, the dead_code lint took advantage of the presence of #[allow(dead_code)]s to not emit itself, my PR changed that, the lint is now emitted regardless of the presence of the attribute. This shouldn't change anything for the user as the lint would still not being displayed. But when using the --test flag for rustc it creates a second main shadowing the previous one and marking as #[allow(dead_code)]:

#[prelude_import]
use ::std::prelude::rust_2015::*;
#[macro_use]
extern crate std;
#[allow(dead_code)]
fn main() { }
#[rustc_main]
#[no_coverage]
fn main()
    ->
        () {
        extern crate test;
        test::test_main_static(&[])
    }

but -Zfuture-incompat-test:

forces all lints to be future incompatible

so dead_code now appears in the future incompatible report, as it probably should always have been.

For the purpose of cargo changing the test to not be depend on the internal details of rustc --test would be good.

@rustbot label +requires-nightly -needs-triage