Rollup merge of #134950 - Zalathar:tool-check-step, r=jieyouxu

bootstrap: Overhaul and simplify the `tool_check_step!` macro

Main changes:

- Pull most of `run` out of the macro and into a regular helper function
- Reduce the number of redundant/unnecessary macro arguments
- Switch to struct-like syntax so that optional arguments are clearer, and so that rustfmt is happy

~~The one “functional” change is that the `-check.stamp` files now get their name from the final path segment, instead of the struct name; in practice this means that they now contain more hyphens in some cases. As far as I'm aware, the exact filename doesn't matter so this should be fine.~~ (that change has been removed from this PR)

Author: Zalathar

src/bootstrap/src/core/build_steps/check.rs

@@ -401,12 +401,13 @@ impl Step for RustAnalyzer {
 
 macro_rules! tool_check_step {
     (
-        $name:ident,
-        $display_name:literal,
-        $path:literal,
-        $($alias:literal, )*
-        $source_type:path
-        $(, $default:literal )?
+        $name:ident {
+            // The part of this path after the final '/' is also used as a display name.
+            path: $path:literal
+            $(, alt_path: $alt_path:literal )*
+            $(, default: $default:literal )?
+            $( , )?
+        }
     ) => {
         #[derive(Debug, Clone, PartialEq, Eq, Hash)]
         pub struct $name {
@@ -416,94 +417,83 @@ macro_rules! tool_check_step {
         impl Step for $name {
             type Output = ();
             const ONLY_HOSTS: bool = true;
-            /// don't ever check out-of-tree tools by default, they'll fail when toolstate is broken
-            const DEFAULT: bool = matches!($source_type, SourceType::InTree) $( && $default )?;
+            /// Most of the tool-checks using this macro are run by default.
+            const DEFAULT: bool = true $( && $default )?;
 
             fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-                run.paths(&[ $path, $($alias),* ])
+                run.paths(&[ $path, $( $alt_path ),* ])
             }
 
             fn make_run(run: RunConfig<'_>) {
                 run.builder.ensure($name { target: run.target });
             }
 
             fn run(self, builder: &Builder<'_>) {
-                let compiler = builder.compiler(builder.top_stage, builder.config.build);
-                let target = self.target;
-
-                builder.ensure(Rustc::new(target, builder));
-
-                let mut cargo = prepare_tool_cargo(
-                    builder,
-                    compiler,
-                    Mode::ToolRustc,
-                    target,
-                    builder.kind,
-                    $path,
-                    $source_type,
-                    &[],
-                );
-
-                // For ./x.py clippy, don't run with --all-targets because
-                // linting tests and benchmarks can produce very noisy results
-                if builder.kind != Kind::Clippy {
-                    cargo.arg("--all-targets");
-                }
-
-                let _guard = builder.msg_check(&format!("{} artifacts", $display_name), target);
-                run_cargo(
-                    builder,
-                    cargo,
-                    builder.config.free_args.clone(),
-                    &stamp(builder, compiler, target),
-                    vec![],
-                    true,
-                    false,
-                );
-
-                /// Cargo's output path in a given stage, compiled by a particular
-                /// compiler for the specified target.
-                fn stamp(
-                    builder: &Builder<'_>,
-                    compiler: Compiler,
-                    target: TargetSelection,
-                ) -> PathBuf {
-                    builder
-                        .cargo_out(compiler, Mode::ToolRustc, target)
-                        .join(format!(".{}-check.stamp", stringify!($name).to_lowercase()))
-                }
+                let Self { target } = self;
+                run_tool_check_step(builder, target, stringify!($name), $path);
             }
         }
-    };
+    }
+}
+
+/// Used by the implementation of `Step::run` in `tool_check_step!`.
+fn run_tool_check_step(
+    builder: &Builder<'_>,
+    target: TargetSelection,
+    step_type_name: &str,
+    path: &str,
+) {
+    let display_name = path.rsplit('/').next().unwrap();
+    let compiler = builder.compiler(builder.top_stage, builder.config.build);
+
+    builder.ensure(Rustc::new(target, builder));
+
+    let mut cargo = prepare_tool_cargo(
+        builder,
+        compiler,
+        Mode::ToolRustc,
+        target,
+        builder.kind,
+        path,
+        // Currently, all of the tools that use this macro/function are in-tree.
+        // If support for out-of-tree tools is re-added in the future, those
+        // steps should probably be marked non-default so that the default
+        // checks aren't affected by toolstate being broken.
+        SourceType::InTree,
+        &[],
+    );
+
+    // For ./x.py clippy, don't run with --all-targets because
+    // linting tests and benchmarks can produce very noisy results
+    if builder.kind != Kind::Clippy {
+        cargo.arg("--all-targets");
+    }
+
+    let stamp = builder
+        .cargo_out(compiler, Mode::ToolRustc, target)
+        .join(format!(".{}-check.stamp", step_type_name.to_lowercase()));
+
+    let _guard = builder.msg_check(format!("{display_name} artifacts"), target);
+    run_cargo(builder, cargo, builder.config.free_args.clone(), &stamp, vec![], true, false);
 }
 
-tool_check_step!(Rustdoc, "rustdoc", "src/tools/rustdoc", "src/librustdoc", SourceType::InTree);
+tool_check_step!(Rustdoc { path: "src/tools/rustdoc", alt_path: "src/librustdoc" });
 // Clippy, miri and Rustfmt are hybrids. They are external tools, but use a git subtree instead
 // of a submodule. Since the SourceType only drives the deny-warnings
 // behavior, treat it as in-tree so that any new warnings in clippy will be
 // rejected.
-tool_check_step!(Clippy, "clippy", "src/tools/clippy", SourceType::InTree);
-tool_check_step!(Miri, "miri", "src/tools/miri", SourceType::InTree);
-tool_check_step!(CargoMiri, "cargo-miri", "src/tools/miri/cargo-miri", SourceType::InTree);
-tool_check_step!(Rls, "rls", "src/tools/rls", SourceType::InTree);
-tool_check_step!(Rustfmt, "rustfmt", "src/tools/rustfmt", SourceType::InTree);
-tool_check_step!(
-    MiroptTestTools,
-    "miropt-test-tools",
-    "src/tools/miropt-test-tools",
-    SourceType::InTree
-);
-tool_check_step!(
-    TestFloatParse,
-    "test-float-parse",
-    "src/etc/test-float-parse",
-    SourceType::InTree
-);
-
-tool_check_step!(Bootstrap, "bootstrap", "src/bootstrap", SourceType::InTree, false);
+tool_check_step!(Clippy { path: "src/tools/clippy" });
+tool_check_step!(Miri { path: "src/tools/miri" });
+tool_check_step!(CargoMiri { path: "src/tools/miri/cargo-miri" });
+tool_check_step!(Rls { path: "src/tools/rls" });
+tool_check_step!(Rustfmt { path: "src/tools/rustfmt" });
+tool_check_step!(MiroptTestTools { path: "src/tools/miropt-test-tools" });
+tool_check_step!(TestFloatParse { path: "src/etc/test-float-parse" });
+
+tool_check_step!(Bootstrap { path: "src/bootstrap", default: false });
 // Compiletest is implicitly "checked" when it gets built in order to run tests,
 // so this is mainly for people working on compiletest to run locally.
-tool_check_step!(Compiletest, "compiletest", "src/tools/compiletest", SourceType::InTree, false);
+tool_check_step!(Compiletest { path: "src/tools/compiletest", default: false });
 
 /// Cargo's output path for the standard library in a given stage, compiled
 /// by a particular compiler for the specified target.