Auto merge of #137899 - notriddle:merged-doctests-stable, r=fmease,GuillaumeGomez

doctests: fix merging on stable

Fixes #137898

The generated multi-test harness relies on nightly-only APIs, so the only way to run it on stable is to enable them.

To prevent the executing test case from getting at any of the stuff that the harness uses, they're built as two separate crates. The test bundle isn't built with RUSTC_BOOTSTRAP, while the runner harness is.

Author: bors

src/librustdoc/doctest.rs

@@ -96,7 +96,7 @@ pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) ->
         .map_err(|error| format!("failed to create args file: {error:?}"))?;
 
     // We now put the common arguments into the file we created.
-    let mut content = vec!["--crate-type=bin".to_string()];
+    let mut content = vec![];
 
     for cfg in &options.cfgs {
         content.push(format!("--cfg={cfg}"));
@@ -513,12 +513,18 @@ pub(crate) struct RunnableDocTest {
     line: usize,
     edition: Edition,
     no_run: bool,
-    is_multiple_tests: bool,
+    merged_test_code: Option<String>,
 }
 
 impl RunnableDocTest {
-    fn path_for_merged_doctest(&self) -> PathBuf {
-        self.test_opts.outdir.path().join(format!("doctest_{}.rs", self.edition))
+    fn path_for_merged_doctest_bundle(&self) -> PathBuf {
+        self.test_opts.outdir.path().join(format!("doctest_bundle_{}.rs", self.edition))
+    }
+    fn path_for_merged_doctest_runner(&self) -> PathBuf {
+        self.test_opts.outdir.path().join(format!("doctest_runner_{}.rs", self.edition))
+    }
+    fn is_multiple_tests(&self) -> bool {
+        self.merged_test_code.is_some()
     }
 }
 
@@ -537,91 +543,108 @@ fn run_test(
     let rust_out = add_exe_suffix("rust_out".to_owned(), &rustdoc_options.target);
     let output_file = doctest.test_opts.outdir.path().join(rust_out);
 
-    let rustc_binary = rustdoc_options
-        .test_builder
-        .as_deref()
-        .unwrap_or_else(|| rustc_interface::util::rustc_path().expect("found rustc"));
-    let mut compiler = wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary);
+    // Common arguments used for compiling the doctest runner.
+    // On merged doctests, the compiler is invoked twice: once for the test code itself,
+    // and once for the runner wrapper (which needs to use `#![feature]` on stable).
+    let mut compiler_args = vec![];
 
-    compiler.arg(format!("@{}", doctest.global_opts.args_file.display()));
+    compiler_args.push(format!("@{}", doctest.global_opts.args_file.display()));
 
     if let Some(sysroot) = &rustdoc_options.maybe_sysroot {
-        compiler.arg(format!("--sysroot={}", sysroot.display()));
+        compiler_args.push(format!("--sysroot={}", sysroot.display()));
     }
 
-    compiler.arg("--edition").arg(doctest.edition.to_string());
-    if !doctest.is_multiple_tests {
-        // Setting these environment variables is unneeded if this is a merged doctest.
-        compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &doctest.test_opts.path);
-        compiler.env(
-            "UNSTABLE_RUSTDOC_TEST_LINE",
-            format!("{}", doctest.line as isize - doctest.full_test_line_offset as isize),
-        );
-    }
-    compiler.arg("-o").arg(&output_file);
+    compiler_args.extend_from_slice(&["--edition".to_owned(), doctest.edition.to_string()]);
     if langstr.test_harness {
-        compiler.arg("--test");
+        compiler_args.push("--test".to_owned());
     }
     if rustdoc_options.json_unused_externs.is_enabled() && !langstr.compile_fail {
-        compiler.arg("--error-format=json");
-        compiler.arg("--json").arg("unused-externs");
-        compiler.arg("-W").arg("unused_crate_dependencies");
-        compiler.arg("-Z").arg("unstable-options");
+        compiler_args.push("--error-format=json".to_owned());
+        compiler_args.extend_from_slice(&["--json".to_owned(), "unused-externs".to_owned()]);
+        compiler_args.extend_from_slice(&["-W".to_owned(), "unused_crate_dependencies".to_owned()]);
+        compiler_args.extend_from_slice(&["-Z".to_owned(), "unstable-options".to_owned()]);
     }
 
     if doctest.no_run && !langstr.compile_fail && rustdoc_options.persist_doctests.is_none() {
         // FIXME: why does this code check if it *shouldn't* persist doctests
         //        -- shouldn't it be the negation?
-        compiler.arg("--emit=metadata");
+        compiler_args.push("--emit=metadata".to_owned());
     }
-    compiler.arg("--target").arg(match &rustdoc_options.target {
-        TargetTuple::TargetTuple(s) => s,
-        TargetTuple::TargetJson { path_for_rustdoc, .. } => {
-            path_for_rustdoc.to_str().expect("target path must be valid unicode")
-        }
-    });
+    compiler_args.extend_from_slice(&[
+        "--target".to_owned(),
+        match &rustdoc_options.target {
+            TargetTuple::TargetTuple(s) => s.clone(),
+            TargetTuple::TargetJson { path_for_rustdoc, .. } => {
+                path_for_rustdoc.to_str().expect("target path must be valid unicode").to_owned()
+            }
+        },
+    ]);
     if let ErrorOutputType::HumanReadable { kind, color_config } = rustdoc_options.error_format {
         let short = kind.short();
         let unicode = kind == HumanReadableErrorType::Unicode;
 
         if short {
-            compiler.arg("--error-format").arg("short");
+            compiler_args.extend_from_slice(&["--error-format".to_owned(), "short".to_owned()]);
         }
         if unicode {
-            compiler.arg("--error-format").arg("human-unicode");
+            compiler_args
+                .extend_from_slice(&["--error-format".to_owned(), "human-unicode".to_owned()]);
         }
 
         match color_config {
             ColorConfig::Never => {
-                compiler.arg("--color").arg("never");
+                compiler_args.extend_from_slice(&["--color".to_owned(), "never".to_owned()]);
             }
             ColorConfig::Always => {
-                compiler.arg("--color").arg("always");
+                compiler_args.extend_from_slice(&["--color".to_owned(), "always".to_owned()]);
             }
             ColorConfig::Auto => {
-                compiler.arg("--color").arg(if supports_color { "always" } else { "never" });
+                compiler_args.extend_from_slice(&[
+                    "--color".to_owned(),
+                    if supports_color { "always" } else { "never" }.to_owned(),
+                ]);
             }
         }
     }
 
+    let rustc_binary = rustdoc_options
+        .test_builder
+        .as_deref()
+        .unwrap_or_else(|| rustc_interface::util::rustc_path().expect("found rustc"));
+    let mut compiler = wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary);
+
+    compiler.args(&compiler_args);
+
     // If this is a merged doctest, we need to write it into a file instead of using stdin
     // because if the size of the merged doctests is too big, it'll simply break stdin.
-    if doctest.is_multiple_tests {
+    if doctest.is_multiple_tests() {
         // It makes the compilation failure much faster if it is for a combined doctest.
         compiler.arg("--error-format=short");
-        let input_file = doctest.path_for_merged_doctest();
+        let input_file = doctest.path_for_merged_doctest_bundle();
         if std::fs::write(&input_file, &doctest.full_test_code).is_err() {
             // If we cannot write this file for any reason, we leave. All combined tests will be
             // tested as standalone tests.
             return Err(TestFailure::CompileError);
         }
-        compiler.arg(input_file);
         if !rustdoc_options.nocapture {
             // If `nocapture` is disabled, then we don't display rustc's output when compiling
             // the merged doctests.
             compiler.stderr(Stdio::null());
         }
+        // bundled tests are an rlib, loaded by a separate runner executable
+        compiler
+            .arg("--crate-type=lib")
+            .arg("--out-dir")
+            .arg(doctest.test_opts.outdir.path())
+            .arg(input_file);
     } else {
+        compiler.arg("--crate-type=bin").arg("-o").arg(&output_file);
+        // Setting these environment variables is unneeded if this is a merged doctest.
+        compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &doctest.test_opts.path);
+        compiler.env(
+            "UNSTABLE_RUSTDOC_TEST_LINE",
+            format!("{}", doctest.line as isize - doctest.full_test_line_offset as isize),
+        );
         compiler.arg("-");
         compiler.stdin(Stdio::piped());
         compiler.stderr(Stdio::piped());
@@ -630,8 +653,65 @@ fn run_test(
     debug!("compiler invocation for doctest: {compiler:?}");
 
     let mut child = compiler.spawn().expect("Failed to spawn rustc process");
-    let output = if doctest.is_multiple_tests {
+    let output = if let Some(merged_test_code) = &doctest.merged_test_code {
+        // compile-fail tests never get merged, so this should always pass
         let status = child.wait().expect("Failed to wait");
+
+        // the actual test runner is a separate component, built with nightly-only features;
+        // build it now
+        let runner_input_file = doctest.path_for_merged_doctest_runner();
+
+        let mut runner_compiler =
+            wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary);
+        // the test runner does not contain any user-written code, so this doesn't allow
+        // the user to exploit nightly-only features on stable
+        runner_compiler.env("RUSTC_BOOTSTRAP", "1");
+        runner_compiler.args(compiler_args);
+        runner_compiler.args(&["--crate-type=bin", "-o"]).arg(&output_file);
+        let mut extern_path = std::ffi::OsString::from(format!(
+            "--extern=doctest_bundle_{edition}=",
+            edition = doctest.edition
+        ));
+        for extern_str in &rustdoc_options.extern_strs {
+            if let Some((_cratename, path)) = extern_str.split_once('=') {
+                // Direct dependencies of the tests themselves are
+                // indirect dependencies of the test runner.
+                // They need to be in the library search path.
+                let dir = Path::new(path)
+                    .parent()
+                    .filter(|x| x.components().count() > 0)
+                    .unwrap_or(Path::new("."));
+                runner_compiler.arg("-L").arg(dir);
+            }
+        }
+        let output_bundle_file = doctest
+            .test_opts
+            .outdir
+            .path()
+            .join(format!("libdoctest_bundle_{edition}.rlib", edition = doctest.edition));
+        extern_path.push(&output_bundle_file);
+        runner_compiler.arg(extern_path);
+        runner_compiler.arg(&runner_input_file);
+        if std::fs::write(&runner_input_file, &merged_test_code).is_err() {
+            // If we cannot write this file for any reason, we leave. All combined tests will be
+            // tested as standalone tests.
+            return Err(TestFailure::CompileError);
+        }
+        if !rustdoc_options.nocapture {
+            // If `nocapture` is disabled, then we don't display rustc's output when compiling
+            // the merged doctests.
+            runner_compiler.stderr(Stdio::null());
+        }
+        runner_compiler.arg("--error-format=short");
+        debug!("compiler invocation for doctest runner: {runner_compiler:?}");
+
+        let status = if !status.success() {
+            status
+        } else {
+            let mut child_runner = runner_compiler.spawn().expect("Failed to spawn rustc process");
+            child_runner.wait().expect("Failed to wait")
+        };
+
         process::Output { status, stdout: Vec::new(), stderr: Vec::new() }
     } else {
         let stdin = child.stdin.as_mut().expect("Failed to open stdin");
@@ -708,15 +788,15 @@ fn run_test(
         cmd.arg(&output_file);
     } else {
         cmd = Command::new(&output_file);
-        if doctest.is_multiple_tests {
+        if doctest.is_multiple_tests() {
             cmd.env("RUSTDOC_DOCTEST_BIN_PATH", &output_file);
         }
     }
     if let Some(run_directory) = &rustdoc_options.test_run_directory {
         cmd.current_dir(run_directory);
     }
 
-    let result = if doctest.is_multiple_tests || rustdoc_options.nocapture {
+    let result = if doctest.is_multiple_tests() || rustdoc_options.nocapture {
         cmd.status().map(|status| process::Output {
             status,
             stdout: Vec::new(),
@@ -1003,7 +1083,7 @@ fn doctest_run_fn(
         line: scraped_test.line,
         edition: scraped_test.edition(&rustdoc_options),
         no_run: scraped_test.no_run(&rustdoc_options),
-        is_multiple_tests: false,
+        merged_test_code: None,
     };
     let res =
         run_test(runnable_test, &rustdoc_options, doctest.supports_color, report_unused_externs);

src/librustdoc/doctest/runner.rs

@@ -14,6 +14,7 @@ pub(crate) struct DocTestRunner {
     crate_attrs: FxIndexSet<String>,
     ids: String,
     output: String,
+    output_merged_tests: String,
     supports_color: bool,
     nb_tests: usize,
 }
@@ -24,6 +25,7 @@ impl DocTestRunner {
             crate_attrs: FxIndexSet::default(),
             ids: String::new(),
             output: String::new(),
+            output_merged_tests: String::new(),
             supports_color: true,
             nb_tests: 0,
         }
@@ -55,7 +57,8 @@ impl DocTestRunner {
                 scraped_test,
                 ignore,
                 self.nb_tests,
-                &mut self.output
+                &mut self.output,
+                &mut self.output_merged_tests,
             ),
         ));
         self.supports_color &= doctest.supports_color;
@@ -78,25 +81,28 @@ impl DocTestRunner {
 "
         .to_string();
 
+        let mut code_prefix = String::new();
+
         for crate_attr in &self.crate_attrs {
-            code.push_str(crate_attr);
-            code.push('\n');
+            code_prefix.push_str(crate_attr);
+            code_prefix.push('\n');
         }
 
         if opts.attrs.is_empty() {
             // If there aren't any attributes supplied by #![doc(test(attr(...)))], then allow some
             // lints that are commonly triggered in doctests. The crate-level test attributes are
             // commonly used to make tests fail in case they trigger warnings, so having this there in
             // that case may cause some tests to pass when they shouldn't have.
-            code.push_str("#![allow(unused)]\n");
+            code_prefix.push_str("#![allow(unused)]\n");
         }
 
         // Next, any attributes that came from the crate root via #![doc(test(attr(...)))].
         for attr in &opts.attrs {
-            code.push_str(&format!("#![{attr}]\n"));
+            code_prefix.push_str(&format!("#![{attr}]\n"));
         }
 
         code.push_str("extern crate test;\n");
+        writeln!(code, "extern crate doctest_bundle_{edition} as doctest_bundle;").unwrap();
 
         let test_args = test_args.iter().fold(String::new(), |mut x, arg| {
             write!(x, "{arg:?}.to_string(),").unwrap();
@@ -161,20 +167,20 @@ the same process\");
 std::process::Termination::report(test::test_main(test_args, Vec::from(TESTS), None))
 }}",
             nb_tests = self.nb_tests,
-            output = self.output,
+            output = self.output_merged_tests,
             ids = self.ids,
         )
         .expect("failed to generate test code");
         let runnable_test = RunnableDocTest {
-            full_test_code: code,
+            full_test_code: format!("{code_prefix}{code}", code = self.output),
             full_test_line_offset: 0,
             test_opts: test_options,
             global_opts: opts.clone(),
             langstr: LangString::default(),
             line: 0,
             edition,
             no_run: false,
-            is_multiple_tests: true,
+            merged_test_code: Some(code),
         };
         let ret =
             run_test(runnable_test, rustdoc_options, self.supports_color, |_: UnusedExterns| {});
@@ -189,14 +195,15 @@ fn generate_mergeable_doctest(
     ignore: bool,
     id: usize,
     output: &mut String,
+    output_merged_tests: &mut String,
 ) -> String {
     let test_id = format!("__doctest_{id}");
 
     if ignore {
         // We generate nothing else.
-        writeln!(output, "mod {test_id} {{\n").unwrap();
+        writeln!(output, "pub mod {test_id} {{}}\n").unwrap();
     } else {
-        writeln!(output, "mod {test_id} {{\n{}{}", doctest.crates, doctest.maybe_crate_attrs)
+        writeln!(output, "pub mod {test_id} {{\n{}{}", doctest.crates, doctest.maybe_crate_attrs)
             .unwrap();
         if doctest.has_main_fn {
             output.push_str(&doctest.everything_else);
@@ -216,11 +223,17 @@ fn main() {returns_result} {{
             )
             .unwrap();
         }
+        writeln!(
+            output,
+            "\npub fn __main_fn() -> impl std::process::Termination {{ main() }} \n}}\n"
+        )
+        .unwrap();
     }
     let not_running = ignore || scraped_test.langstr.no_run;
     writeln!(
-        output,
+        output_merged_tests,
         "
+mod {test_id} {{
 pub const TEST: test::TestDescAndFn = test::TestDescAndFn::new_doctest(
 {test_name:?}, {ignore}, {file:?}, {line}, {no_run}, {should_panic},
 test::StaticTestFn(
@@ -242,7 +255,7 @@ test::StaticTestFn(
 if let Some(bin_path) = crate::__doctest_mod::doctest_path() {{
     test::assert_test_result(crate::__doctest_mod::doctest_runner(bin_path, {id}))
 }} else {{
-    test::assert_test_result(self::main())
+    test::assert_test_result(doctest_bundle::{test_id}::__main_fn())
 }}
 ",
             )

tests/run-make/doctests-merge/rmake.rs

@@ -8,7 +8,6 @@ fn test_and_compare(input_file: &str, stdout_file: &str, edition: &str, dep: &Pa
     let output = cmd
         .input(input_file)
         .arg("--test")
-        .arg("-Zunstable-options")
         .edition(edition)
         .arg("--test-args=--test-threads=1")
         .extern_("foo", dep.display().to_string())

tests/rustdoc-ui/doctest/doctest-output.rs

@@ -2,7 +2,7 @@
 //@[edition2015]edition:2015
 //@[edition2015]aux-build:extern_macros.rs
 //@[edition2015]compile-flags:--test --test-args=--test-threads=1
-//@[edition2024]edition:2015
+//@[edition2024]edition:2024
 //@[edition2024]aux-build:extern_macros.rs
 //@[edition2024]compile-flags:--test --test-args=--test-threads=1
 //@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR"

tests/rustdoc-ui/doctest/failed-doctest-test-crate.edition2015.stdout

@@ -0,0 +1,28 @@
+
+running 1 test
+test $DIR/failed-doctest-test-crate.rs - m (line 14) ... FAILED
+
+failures:
+
+---- $DIR/failed-doctest-test-crate.rs - m (line 14) stdout ----
+error[E0432]: unresolved import `test`
+  --> $DIR/failed-doctest-test-crate.rs:15:5
+   |
+LL | use test::*;
+   |     ^^^^ use of unresolved module or unlinked crate `test`
+   |
+help: you might be missing a crate named `test`, add it to your project and import it in your code
+   |
+LL + extern crate test;
+   |
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0432`.
+Couldn't compile the test.
+
+failures:
+    $DIR/failed-doctest-test-crate.rs - m (line 14)
+
+test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+

tests/rustdoc-ui/doctest/failed-doctest-test-crate.edition2024.stdout

@@ -0,0 +1,25 @@
+
+running 1 test
+test $DIR/failed-doctest-test-crate.rs - m (line 14) ... FAILED
+
+failures:
+
+---- $DIR/failed-doctest-test-crate.rs - m (line 14) stdout ----
+error[E0432]: unresolved import `test`
+  --> $DIR/failed-doctest-test-crate.rs:15:5
+   |
+LL | use test::*;
+   |     ^^^^ use of unresolved module or unlinked crate `test`
+   |
+   = help: you might be missing a crate named `test`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0432`.
+Couldn't compile the test.
+
+failures:
+    $DIR/failed-doctest-test-crate.rs - m (line 14)
+
+test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+

tests/rustdoc-ui/doctest/failed-doctest-test-crate.rs

@@ -0,0 +1,17 @@
+// FIXME: if/when the output of the test harness can be tested on its own, this test should be
+// adapted to use that, and that normalize line can go away
+
+//@ revisions: edition2015 edition2024
+//@[edition2015]edition:2015
+//@[edition2024]edition:2024
+//@ compile-flags:--test
+//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR"
+//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME"
+//@ failure-status: 101
+
+/// <https://github.com/rust-lang/rust/pull/137899#discussion_r1976743383>
+///
+/// ```rust
+/// use test::*;
+/// ```
+pub mod m {}