From a70171b111ccbc26f7d3c0033907b372640ddb9f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 16 Jul 2025 16:25:29 +0200 Subject: [PATCH] clippy: make tests work in stage 1 --- src/bootstrap/src/core/build_steps/test.rs | 44 +++++++++---------- src/tools/clippy/tests/compile-test.rs | 29 +++++++++++- .../tests/ui/auxiliary/proc_macro_derive.rs | 2 +- .../clippy/tests/ui/iter_over_hash_type.rs | 11 +++-- .../tests/ui/iter_over_hash_type.stderr | 26 +++++------ .../clippy/tests/ui/useless_attribute.fixed | 2 +- .../clippy/tests/ui/useless_attribute.rs | 2 +- 7 files changed, 71 insertions(+), 45 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9e7ea5c115f19..cc2c88f512cbf 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -739,7 +739,6 @@ impl Step for CompiletestTest { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Clippy { - stage: u32, host: TargetSelection, } @@ -753,33 +752,23 @@ impl Step for Clippy { } fn make_run(run: RunConfig<'_>) { - // If stage is explicitly set or not lower than 2, keep it. Otherwise, make sure it's at least 2 - // as tests for this step don't work with a lower stage. - let stage = if run.builder.config.is_explicit_stage() || run.builder.top_stage >= 2 { - run.builder.top_stage - } else { - 2 - }; - - run.builder.ensure(Clippy { stage, host: run.target }); + run.builder.ensure(Clippy { host: run.target }); } /// Runs `cargo test` for clippy. fn run(self, builder: &Builder<'_>) { - let stage = self.stage; + let stage = builder.top_stage; let host = self.host; - let compiler = builder.compiler(stage, host); - - if stage < 2 { - eprintln!("WARNING: clippy tests on stage {stage} may not behave well."); - eprintln!("HELP: consider using stage 2"); - } + // We need to carefully distinguish the compiler that builds clippy, and the compiler + // that is linked into the clippy being tested. `target_compiler` is the latter, + // and it must also be used by clippy's test runner to build tests and their dependencies. + let target_compiler = builder.compiler(stage, host); - let tool_result = builder.ensure(tool::Clippy { compiler, target: self.host }); - let compiler = tool_result.build_compiler; + let tool_result = builder.ensure(tool::Clippy { compiler: target_compiler, target: host }); + let tool_compiler = tool_result.build_compiler; let mut cargo = tool::prepare_tool_cargo( builder, - compiler, + tool_compiler, Mode::ToolRustc, host, Kind::Test, @@ -788,11 +777,17 @@ impl Step for Clippy { &[], ); - cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler)); - cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler)); - let host_libs = builder.stage_out(compiler, Mode::ToolRustc).join(builder.cargo_dir()); + cargo.env("RUSTC_TEST_SUITE", builder.rustc(tool_compiler)); + cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(tool_compiler)); + let host_libs = builder.stage_out(tool_compiler, Mode::ToolRustc).join(builder.cargo_dir()); cargo.env("HOST_LIBS", host_libs); + // Build the standard library that the tests can use. + builder.std(target_compiler, host); + cargo.env("TEST_SYSROOT", builder.sysroot(target_compiler)); + cargo.env("TEST_RUSTC", builder.rustc(target_compiler)); + cargo.env("TEST_RUSTC_LIB", builder.rustc_libdir(target_compiler)); + // Collect paths of tests to run 'partially_test: { let paths = &builder.config.paths[..]; @@ -813,7 +808,8 @@ impl Step for Clippy { cargo.add_rustc_lib_path(builder); let cargo = prepare_cargo_test(cargo, &[], &[], host, builder); - let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); + let _guard = + builder.msg_sysroot_tool(Kind::Test, tool_compiler.stage, "clippy", host, host); // Clippy reports errors if it blessed the outputs if cargo.allow_failure().run(builder) { diff --git a/src/tools/clippy/tests/compile-test.rs b/src/tools/clippy/tests/compile-test.rs index 57d623b2cfc8b..83f91ccaa7b4b 100644 --- a/src/tools/clippy/tests/compile-test.rs +++ b/src/tools/clippy/tests/compile-test.rs @@ -151,7 +151,31 @@ impl TestContext { defaults.set_custom( "dependencies", DependencyBuilder { - program: CommandBuilder::cargo(), + program: { + let mut p = CommandBuilder::cargo(); + // If we run in bootstrap, we need to use the right compiler for building the + // tests -- not the compiler that built clippy, but the compiler that got linked + // into clippy. Just invoking TEST_RUSTC does not work because LD_LIBRARY_PATH + // is set in a way that makes it pick the wrong sysroot. Sadly due to + // we cannot use RUSTFLAGS to + // set `--sysroot`, so we need to use bootstrap's rustc wrapper. That wrapper + // however has some staging logic that is hurting us here, so to work around + // that we set both the "real" and "staging" rustc to TEST_RUSTC, including the + // associated library paths. + if let Some(rustc) = option_env!("TEST_RUSTC") { + let libdir = option_env!("TEST_RUSTC_LIB").unwrap(); + let sysroot = option_env!("TEST_SYSROOT").unwrap(); + p.envs.push(("RUSTC_REAL".into(), Some(rustc.into()))); + p.envs.push(("RUSTC_REAL_LIBDIR".into(), Some(libdir.into()))); + p.envs.push(("RUSTC_SNAPSHOT".into(), Some(rustc.into()))); + p.envs.push(("RUSTC_SNAPSHOT_LIBDIR".into(), Some(libdir.into()))); + p.envs.push(( + "RUSTC_SYSROOT".into(), + Some(sysroot.into()), + )); + } + p + }, crate_manifest_path: Path::new("clippy_test_deps").join("Cargo.toml"), build_std: None, bless_lockfile: self.args.bless, @@ -192,6 +216,9 @@ impl TestContext { let dep = format!("-Ldependency={}", Path::new(host_libs).join("deps").display()); config.program.args.push(dep.into()); } + if let Some(sysroot) = option_env!("TEST_SYSROOT") { + config.program.args.push(format!("--sysroot={sysroot}").into()); + } config.program.program = profile_path.join(if cfg!(windows) { "clippy-driver.exe" diff --git a/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs b/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs index 5992d15935d5b..5465092287179 100644 --- a/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs +++ b/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs @@ -16,7 +16,7 @@ pub fn derive(_: TokenStream) -> TokenStream { let output = quote! { // Should not trigger `useless_attribute` #[allow(dead_code)] - extern crate rustc_middle; + extern crate core; }; output } diff --git a/src/tools/clippy/tests/ui/iter_over_hash_type.rs b/src/tools/clippy/tests/ui/iter_over_hash_type.rs index 914cc9df0de99..9a3e7033cd879 100644 --- a/src/tools/clippy/tests/ui/iter_over_hash_type.rs +++ b/src/tools/clippy/tests/ui/iter_over_hash_type.rs @@ -3,15 +3,18 @@ #![warn(clippy::iter_over_hash_type)] use std::collections::{HashMap, HashSet}; -extern crate rustc_data_structures; - extern crate proc_macros; +// Ensure it also works via type aliases (this isn't really the Fx hasher but that does not matter). +type FxBuildHasher = std::collections::hash_map::RandomState; +type FxHashMap = HashMap; +type FxHashSet = HashSet; + fn main() { let mut hash_set = HashSet::::new(); let mut hash_map = HashMap::::new(); - let mut fx_hash_map = rustc_data_structures::fx::FxHashMap::::default(); - let mut fx_hash_set = rustc_data_structures::fx::FxHashMap::::default(); + let mut fx_hash_map = FxHashMap::::default(); + let mut fx_hash_set = FxHashSet::::default(); let vec = Vec::::new(); // test hashset diff --git a/src/tools/clippy/tests/ui/iter_over_hash_type.stderr b/src/tools/clippy/tests/ui/iter_over_hash_type.stderr index 1bc6f4588d44b..3356186547db7 100644 --- a/src/tools/clippy/tests/ui/iter_over_hash_type.stderr +++ b/src/tools/clippy/tests/ui/iter_over_hash_type.stderr @@ -1,5 +1,5 @@ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:18:5 + --> tests/ui/iter_over_hash_type.rs:21:5 | LL | / for x in &hash_set { LL | | @@ -11,7 +11,7 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::iter_over_hash_type)]` error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:22:5 + --> tests/ui/iter_over_hash_type.rs:25:5 | LL | / for x in hash_set.iter() { LL | | @@ -20,7 +20,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:26:5 + --> tests/ui/iter_over_hash_type.rs:29:5 | LL | / for x in hash_set.clone() { LL | | @@ -29,7 +29,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:30:5 + --> tests/ui/iter_over_hash_type.rs:33:5 | LL | / for x in hash_set.drain() { LL | | @@ -38,7 +38,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:36:5 + --> tests/ui/iter_over_hash_type.rs:39:5 | LL | / for (x, y) in &hash_map { LL | | @@ -47,7 +47,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:40:5 + --> tests/ui/iter_over_hash_type.rs:43:5 | LL | / for x in hash_map.keys() { LL | | @@ -56,7 +56,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:44:5 + --> tests/ui/iter_over_hash_type.rs:47:5 | LL | / for x in hash_map.values() { LL | | @@ -65,7 +65,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:48:5 + --> tests/ui/iter_over_hash_type.rs:51:5 | LL | / for x in hash_map.values_mut() { LL | | @@ -74,7 +74,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:52:5 + --> tests/ui/iter_over_hash_type.rs:55:5 | LL | / for x in hash_map.iter() { LL | | @@ -83,7 +83,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:56:5 + --> tests/ui/iter_over_hash_type.rs:59:5 | LL | / for x in hash_map.clone() { LL | | @@ -92,7 +92,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:60:5 + --> tests/ui/iter_over_hash_type.rs:63:5 | LL | / for x in hash_map.drain() { LL | | @@ -101,7 +101,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:66:5 + --> tests/ui/iter_over_hash_type.rs:69:5 | LL | / for x in fx_hash_set { LL | | @@ -110,7 +110,7 @@ LL | | } | |_____^ error: iteration over unordered hash-based type - --> tests/ui/iter_over_hash_type.rs:70:5 + --> tests/ui/iter_over_hash_type.rs:73:5 | LL | / for x in fx_hash_map { LL | | diff --git a/src/tools/clippy/tests/ui/useless_attribute.fixed b/src/tools/clippy/tests/ui/useless_attribute.fixed index a96c8f46f551f..930bc1eaecf9c 100644 --- a/src/tools/clippy/tests/ui/useless_attribute.fixed +++ b/src/tools/clippy/tests/ui/useless_attribute.fixed @@ -13,7 +13,7 @@ #[allow(unused_imports)] #[allow(unused_extern_crates)] #[macro_use] -extern crate rustc_middle; +extern crate regex as regex_crate; #[macro_use] extern crate proc_macro_derive; diff --git a/src/tools/clippy/tests/ui/useless_attribute.rs b/src/tools/clippy/tests/ui/useless_attribute.rs index b26410134bbb5..50fafd478e51c 100644 --- a/src/tools/clippy/tests/ui/useless_attribute.rs +++ b/src/tools/clippy/tests/ui/useless_attribute.rs @@ -13,7 +13,7 @@ #[allow(unused_imports)] #[allow(unused_extern_crates)] #[macro_use] -extern crate rustc_middle; +extern crate regex as regex_crate; #[macro_use] extern crate proc_macro_derive;