Refactor `Rustdoc`

3 files changed, 90 insertions, 96 deletions

diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs
index b3e054a9d8b..9ee83075ea0 100644
--- a/src/bootstrap/src/core/build_steps/tool.rs
+++ b/src/bootstrap/src/core/build_steps/tool.rs
@@ -681,15 +681,21 @@ impl Step for RemoteTestServer {
     }
 }
 
-#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
+/// Represents `Rustdoc` that either comes from the external stage0 sysroot or that is built
+/// locally.
+/// Rustdoc is special, because it both essentially corresponds to a `Compiler` (that can be
+/// externally provided), but also to a `ToolRustc` tool.
+#[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub struct Rustdoc {
-    /// This should only ever be 0 or 2.
-    /// We sometimes want to reference the "bootstrap" rustdoc, which is why this option is here.
+    /// If the stage of `target_compiler` is `0`, then rustdoc is externally provided.
+    /// Otherwise it is built locally.
     pub target_compiler: Compiler,
 }
 
 impl Step for Rustdoc {
-    type Output = ToolBuildResult;
+    /// Path to the built rustdoc binary.
+    type Output = PathBuf;
+
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
@@ -703,21 +709,20 @@ impl Step for Rustdoc {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> ToolBuildResult {
+    fn run(self, builder: &Builder<'_>) -> Self::Output {
         let target_compiler = self.target_compiler;
         let target = target_compiler.host;
 
+        // If stage is 0, we use a prebuilt rustdoc from stage0
         if target_compiler.stage == 0 {
             if !target_compiler.is_snapshot(builder) {
                 panic!("rustdoc in stage 0 must be snapshot rustdoc");
             }
 
-            return ToolBuildResult {
-                tool_path: builder.initial_rustdoc.clone(),
-                build_compiler: target_compiler,
-            };
+            return builder.initial_rustdoc.clone();
         }
 
+        // If stage is higher, we build rustdoc instead
         let bin_rustdoc = || {
             let sysroot = builder.sysroot(target_compiler);
             let bindir = sysroot.join("bin");
@@ -729,10 +734,7 @@ impl Step for Rustdoc {
 
         // If CI rustc is enabled and we haven't modified the rustdoc sources,
         // use the precompiled rustdoc from CI rustc's sysroot to speed up bootstrapping.
-        if builder.download_rustc()
-            && target_compiler.stage > 0
-            && builder.rust_info().is_managed_git_subrepository()
-        {
+        if builder.download_rustc() && builder.rust_info().is_managed_git_subrepository() {
             let files_to_track = &["src/librustdoc", "src/tools/rustdoc", "src/rustdoc-json-types"];
 
             // Check if unchanged
@@ -745,8 +747,7 @@ impl Step for Rustdoc {
 
                 let bin_rustdoc = bin_rustdoc();
                 builder.copy_link(&precompiled_rustdoc, &bin_rustdoc, FileType::Executable);
-
-                return ToolBuildResult { tool_path: bin_rustdoc, build_compiler: target_compiler };
+                return bin_rustdoc;
             }
         }
 
@@ -762,45 +763,39 @@ impl Step for Rustdoc {
             extra_features.push("jemalloc".to_string());
         }
 
-        let ToolBuildResult { tool_path, build_compiler } = builder.ensure(ToolBuild {
-            build_compiler: target_compiler,
-            target,
-            // Cargo adds a number of paths to the dylib search path on windows, which results in
-            // the wrong rustdoc being executed. To avoid the conflicting rustdocs, we name the "tool"
-            // rustdoc a different name.
-            tool: "rustdoc_tool_binary",
-            mode: Mode::ToolRustc,
-            path: "src/tools/rustdoc",
-            source_type: SourceType::InTree,
-            extra_features,
-            allow_features: "",
-            cargo_args: Vec::new(),
-            artifact_kind: ToolArtifactKind::Binary,
-        });
-
-        // FIXME: handle the build/target compiler split here somehow
-        // don't create a stage0-sysroot/bin directory.
-        if target_compiler.stage > 0 {
-            if builder.config.rust_debuginfo_level_tools == DebuginfoLevel::None {
-                // Due to LTO a lot of debug info from C++ dependencies such as jemalloc can make it into
-                // our final binaries
-                compile::strip_debug(builder, target, &tool_path);
-            }
-            let bin_rustdoc = bin_rustdoc();
-            builder.copy_link(&tool_path, &bin_rustdoc, FileType::Executable);
-            ToolBuildResult { tool_path: bin_rustdoc, build_compiler }
-        } else {
-            ToolBuildResult { tool_path, build_compiler }
+        let compilers = RustcPrivateCompilers::from_link_compiler(builder, target_compiler);
+        let tool_path = builder
+            .ensure(ToolBuild {
+                build_compiler: compilers.build_compiler,
+                target,
+                // Cargo adds a number of paths to the dylib search path on windows, which results in
+                // the wrong rustdoc being executed. To avoid the conflicting rustdocs, we name the "tool"
+                // rustdoc a different name.
+                tool: "rustdoc_tool_binary",
+                mode: Mode::ToolRustc,
+                path: "src/tools/rustdoc",
+                source_type: SourceType::InTree,
+                extra_features,
+                allow_features: "",
+                cargo_args: Vec::new(),
+                artifact_kind: ToolArtifactKind::Binary,
+            })
+            .tool_path;
+
+        if builder.config.rust_debuginfo_level_tools == DebuginfoLevel::None {
+            // Due to LTO a lot of debug info from C++ dependencies such as jemalloc can make it into
+            // our final binaries
+            compile::strip_debug(builder, target, &tool_path);
         }
+        let bin_rustdoc = bin_rustdoc();
+        builder.copy_link(&tool_path, &bin_rustdoc, FileType::Executable);
+        bin_rustdoc
     }
 
     fn metadata(&self) -> Option<StepMetadata> {
         Some(
             StepMetadata::build("rustdoc", self.target_compiler.host)
-                // rustdoc is ToolRustc, so stage N rustdoc is built by stage N-1 rustc
-                // FIXME: make this stage deduction automatic somehow
-                // FIXME: log the compiler that actually built ToolRustc steps
-                .stage(self.target_compiler.stage.saturating_sub(1)),
+                .stage(self.target_compiler.stage),
         )
     }
 }
@@ -1317,18 +1312,11 @@ impl RustcPrivateCompilers {
     /// Create compilers for a `rustc_private` tool with the given `stage` and for the given
     /// `target`.
     pub fn new(builder: &Builder<'_>, stage: u32, target: TargetSelection) -> Self {
-        assert!(stage > 0);
-
-        let build_compiler = if builder.download_rustc() && stage == 1 {
-            // We shouldn't drop to stage0 compiler when using CI rustc.
-            builder.compiler(1, builder.config.host_target)
-        } else {
-            builder.compiler(stage - 1, builder.config.host_target)
-        };
+        let build_compiler = Self::build_compiler_from_stage(builder, stage);
 
         // This is the compiler we'll link to
         // FIXME: make 100% sure that `link_compiler` was indeed built with `build_compiler`...
-        let link_compiler = builder.compiler(stage, target);
+        let link_compiler = builder.compiler(build_compiler.stage + 1, target);
 
         Self { build_compiler, link_compiler }
     }
@@ -1343,6 +1331,25 @@ impl RustcPrivateCompilers {
         Self { build_compiler, link_compiler }
     }
 
+    /// Create rustc tool compilers from the link compiler.
+    pub fn from_link_compiler(builder: &Builder<'_>, link_compiler: Compiler) -> Self {
+        Self {
+            build_compiler: Self::build_compiler_from_stage(builder, link_compiler.stage),
+            link_compiler,
+        }
+    }
+
+    fn build_compiler_from_stage(builder: &Builder<'_>, stage: u32) -> Compiler {
+        assert!(stage > 0);
+
+        if builder.download_rustc() && stage == 1 {
+            // We shouldn't drop to stage0 compiler when using CI rustc.
+            builder.compiler(1, builder.config.host_target)
+        } else {
+            builder.compiler(stage - 1, builder.config.host_target)
+        }
+    }
+
     pub fn build_compiler(&self) -> Compiler {
         self.build_compiler
     }
diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs
index b30c460d5f1..660c869b67e 100644
--- a/src/bootstrap/src/core/builder/mod.rs
+++ b/src/bootstrap/src/core/builder/mod.rs
@@ -1540,7 +1540,7 @@ You have to build a stage1 compiler for `{}` first, and then use it to build a s
     /// It can be either a stage0 rustdoc or a locally built rustdoc that *links* to
     /// `target_compiler`.
     pub fn rustdoc_for_compiler(&self, target_compiler: Compiler) -> PathBuf {
-        self.ensure(tool::Rustdoc { target_compiler }).tool_path
+        self.ensure(tool::Rustdoc { target_compiler })
     }
 
     pub fn cargo_clippy_cmd(&self, run_compiler: Compiler) -> BootstrapCommand {
diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs
index 06ae73a6ad3..26158abf4bd 100644
--- a/src/bootstrap/src/core/builder/tests.rs
+++ b/src/bootstrap/src/core/builder/tests.rs
@@ -279,13 +279,6 @@ mod defaults {
             first(cache.all::<tool::ErrorIndex>()),
             &[tool::ErrorIndex { compiler: Compiler::new(1, a) }]
         );
-        // docs should be built with the stage0 compiler, not with the stage0 artifacts.
-        // recall that rustdoc is off-by-one: `stage` is the compiler rustdoc is _linked_ to,
-        // not the one it was built by.
-        assert_eq!(
-            first(cache.all::<tool::Rustdoc>()),
-            &[tool::Rustdoc { target_compiler: Compiler::new(1, a) },]
-        );
     }
 }
 
@@ -337,12 +330,6 @@ mod dist {
             first(builder.cache.all::<tool::ErrorIndex>()),
             &[tool::ErrorIndex { compiler: Compiler::new(1, a) }]
         );
-        // This is actually stage 1, but Rustdoc::run swaps out the compiler with
-        // stage minus 1 if --stage is not 0. Very confusing!
-        assert_eq!(
-            first(builder.cache.all::<tool::Rustdoc>()),
-            &[tool::Rustdoc { target_compiler: Compiler::new(2, a) },]
-        );
     }
 }
 
@@ -627,7 +614,7 @@ mod snapshot {
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         ");
     }
 
@@ -650,10 +637,10 @@ mod snapshot {
         [build] rustc 2 <host> -> std 2 <host>
         [build] rustc 1 <host> -> std 1 <target1>
         [build] rustc 2 <host> -> std 2 <target1>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [build] llvm <target1>
         [build] rustc 1 <host> -> rustc 2 <target1>
-        [build] rustdoc 1 <target1>
+        [build] rustdoc 2 <target1>
         ");
     }
 
@@ -750,7 +737,7 @@ mod snapshot {
         [build] rustc 1 <host> -> LldWrapper 2 <host>
         [build] rustc 1 <host> -> LlvmBitcodeLinker 2 <host>
         [build] rustc 2 <host> -> std 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         "
         );
     }
@@ -779,7 +766,7 @@ mod snapshot {
         [build] rustc 1 <host> -> rustc 2 <target1>
         [build] rustc 1 <host> -> LldWrapper 2 <target1>
         [build] rustc 1 <host> -> LlvmBitcodeLinker 2 <target1>
-        [build] rustdoc 1 <target1>
+        [build] rustdoc 2 <target1>
         "
         );
     }
@@ -968,7 +955,7 @@ mod snapshot {
             .render_steps(), @r"
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] std 1 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         ");
     }
@@ -1017,7 +1004,7 @@ mod snapshot {
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
         [build] rustc 0 <host> -> LintDocs 1 <host>
@@ -1059,7 +1046,7 @@ mod snapshot {
         [build] rustc 1 <host> -> LldWrapper 2 <host>
         [build] rustc 1 <host> -> WasmComponentLd 2 <host>
         [build] rustc 1 <host> -> LlvmBitcodeLinker 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
         [build] rustc 0 <host> -> LintDocs 1 <host>
@@ -1098,7 +1085,7 @@ mod snapshot {
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [doc] std 2 <target1> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
@@ -1135,7 +1122,7 @@ mod snapshot {
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
         [build] rustc 0 <host> -> LintDocs 1 <host>
@@ -1149,7 +1136,7 @@ mod snapshot {
         [dist] rustc <host>
         [build] llvm <target1>
         [build] rustc 1 <host> -> rustc 2 <target1>
-        [build] rustdoc 1 <target1>
+        [build] rustdoc 2 <target1>
         [dist] rustc <target1>
         [dist] rustc 1 <host> -> std 1 <host>
         [dist] src <>
@@ -1172,7 +1159,7 @@ mod snapshot {
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [doc] std 2 <target1> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
@@ -1190,7 +1177,7 @@ mod snapshot {
         [dist] rustc <host>
         [build] llvm <target1>
         [build] rustc 1 <host> -> rustc 2 <target1>
-        [build] rustdoc 1 <target1>
+        [build] rustdoc 2 <target1>
         [dist] rustc <target1>
         [dist] rustc 1 <host> -> std 1 <host>
         [dist] rustc 1 <host> -> std 1 <target1>
@@ -1214,7 +1201,7 @@ mod snapshot {
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <target1> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
         [build] rustc 0 <host> -> RustInstaller 1 <host>
@@ -1246,7 +1233,7 @@ mod snapshot {
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
         [build] rustc 1 <host> -> WasmComponentLd 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] std 2 <target1> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         [build] rustc 2 <host> -> std 2 <host>
         [build] rustc 1 <host> -> std 1 <target1>
@@ -1259,7 +1246,7 @@ mod snapshot {
         [build] llvm <target1>
         [build] rustc 1 <host> -> rustc 2 <target1>
         [build] rustc 1 <host> -> WasmComponentLd 2 <target1>
-        [build] rustdoc 1 <target1>
+        [build] rustdoc 2 <target1>
         [build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <target1>
         [build] rustc 0 <host> -> GenerateCopyright 1 <host>
         [dist] rustc <target1>
@@ -1595,7 +1582,7 @@ mod snapshot {
                 .render_steps(), @r"
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] std 1 <host> crates=[alloc,compiler_builtins,core,panic_abort,panic_unwind,proc_macro,rustc-std-workspace-core,std,std_detect,sysroot,test,unwind]
         ");
     }
@@ -1609,7 +1596,7 @@ mod snapshot {
                 .render_steps(), @r"
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] std 1 <host> crates=[core]
         ");
     }
@@ -1624,7 +1611,7 @@ mod snapshot {
                 .render_steps(), @r"
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] std 1 <host> crates=[core]
         ");
     }
@@ -1654,7 +1641,7 @@ mod snapshot {
                 .render_steps(), @r"
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] std 1 <host> crates=[alloc,core]
         ");
     }
@@ -1670,7 +1657,7 @@ mod snapshot {
                 .render_steps(), @r"
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] std 1 <target1> crates=[alloc,core]
         ");
     }
@@ -1700,7 +1687,7 @@ mod snapshot {
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] rustc 1 <host>
         ");
     }
@@ -1718,7 +1705,7 @@ mod snapshot {
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
         [build] rustc 2 <host> -> std 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] rustc 2 <host>
         ");
     }
@@ -1747,7 +1734,7 @@ mod snapshot {
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
         [build] rustc 1 <host> -> std 1 <host>
-        [build] rustdoc 0 <host>
+        [build] rustdoc 1 <host>
         [doc] Compiletest <host>
         ");
     }
@@ -1765,7 +1752,7 @@ mod snapshot {
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
         [build] rustc 2 <host> -> std 2 <host>
-        [build] rustdoc 1 <host>
+        [build] rustdoc 2 <host>
         [doc] Compiletest <host>
         ");
     }