Rollup merge of rust-lang#149565 - jyn514:force-doctest-merge, r=notriddle

rustdoc: Add unstable `--merge-doctests=yes/no/auto` flag

This is useful for changing the *default* for whether doctests are merged or not. Currently, that default is solely controlled by `edition = 2024`, which adds a high switching cost to get doctest merging. This flag allows opting in even on earlier editions.

Unlike the `edition = 2024` default, `--merge-doctests=yes` gives a hard error if merging fails instead of falling back to running standalone tests. The user has explicitly said they want merging, so we shouldn't silently do something else.

`--merge-doctests=auto` is equivalent to the current 2024 edition behavior, but available on earlier editions.

Helps with rust-lang#141240. `@epage` said in that issue he would like a per-doctest opt-in, and that seems useful to me in addition to this flag, but I think it's a separate use case from changing the default.

Author: JonathanBrouwer

src/librustdoc/config.rs

@@ -63,6 +63,15 @@ pub(crate) enum InputMode {
     HasFile(Input),
 }
 
+/// Whether to run multiple doctests in the same binary.
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Default)]
+pub(crate) enum MergeDoctests {
+    #[default]
+    Never,
+    Always,
+    Auto,
+}
+
 /// Configuration options for rustdoc.
 #[derive(Clone)]
 pub(crate) struct Options {
@@ -121,6 +130,8 @@ pub(crate) struct Options {
     /// Optional path to persist the doctest executables to, defaults to a
     /// temporary directory if not set.
     pub(crate) persist_doctests: Option<PathBuf>,
+    /// Whether to merge
+    pub(crate) merge_doctests: MergeDoctests,
     /// Runtool to run doctests with
     pub(crate) test_runtool: Option<String>,
     /// Arguments to pass to the runtool
@@ -801,6 +812,8 @@ impl Options {
             Ok(result) => result,
             Err(e) => dcx.fatal(format!("--merge option error: {e}")),
         };
+        let merge_doctests = parse_merge_doctests(matches, edition, dcx);
+        tracing::debug!("merge_doctests: {merge_doctests:?}");
 
         if generate_link_to_definition && (show_coverage || output_format != OutputFormat::Html) {
             dcx.struct_warn(
@@ -852,6 +865,7 @@ impl Options {
             crate_version,
             test_run_directory,
             persist_doctests,
+            merge_doctests,
             test_runtool,
             test_runtool_args,
             test_builder,
@@ -1048,3 +1062,20 @@ fn parse_merge(m: &getopts::Matches) -> Result<ShouldMerge, &'static str> {
         Some(_) => Err("argument to --merge must be `none`, `shared`, or `finalize`"),
     }
 }
+
+fn parse_merge_doctests(
+    m: &getopts::Matches,
+    edition: Edition,
+    dcx: DiagCtxtHandle<'_>,
+) -> MergeDoctests {
+    match m.opt_str("merge-doctests").as_deref() {
+        Some("y") | Some("yes") | Some("on") | Some("true") => MergeDoctests::Always,
+        Some("n") | Some("no") | Some("off") | Some("false") => MergeDoctests::Never,
+        Some("auto") => MergeDoctests::Auto,
+        None if edition < Edition::Edition2024 => MergeDoctests::Never,
+        None => MergeDoctests::Auto,
+        Some(_) => {
+            dcx.fatal("argument to --merge-doctests must be a boolean (true/false) or 'auto'")
+        }
+    }
+}

src/librustdoc/doctest.rs

@@ -33,7 +33,7 @@ use tempfile::{Builder as TempFileBuilder, TempDir};
 use tracing::debug;
 
 use self::rust::HirCollector;
-use crate::config::{Options as RustdocOptions, OutputFormat};
+use crate::config::{MergeDoctests, Options as RustdocOptions, OutputFormat};
 use crate::html::markdown::{ErrorCodes, Ignore, LangString, MdRelLine};
 use crate::lint::init_lints;
 
@@ -123,8 +123,13 @@ pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) ->
     Ok(())
 }
 
-fn get_doctest_dir() -> io::Result<TempDir> {
-    TempFileBuilder::new().prefix("rustdoctest").tempdir()
+fn get_doctest_dir(opts: &RustdocOptions) -> io::Result<TempDir> {
+    let mut builder = TempFileBuilder::new();
+    builder.prefix("rustdoctest");
+    if opts.codegen_options.save_temps {
+        builder.disable_cleanup(true);
+    }
+    builder.tempdir()
 }
 
 pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions) {
@@ -197,7 +202,7 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
     let externs = options.externs.clone();
     let json_unused_externs = options.json_unused_externs;
 
-    let temp_dir = match get_doctest_dir()
+    let temp_dir = match get_doctest_dir(&options)
         .map_err(|error| format!("failed to create temporary directory: {error:?}"))
     {
         Ok(temp_dir) => temp_dir,
@@ -207,6 +212,7 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
     crate::wrap_return(dcx, generate_args_file(&args_path, &options));
 
     let extract_doctests = options.output_format == OutputFormat::Doctest;
+    let save_temps = options.codegen_options.save_temps;
     let result = interface::run_compiler(config, |compiler| {
         let krate = rustc_interface::passes::parse(&compiler.sess);
 
@@ -259,12 +265,15 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
             eprintln!("{error}");
             // Since some files in the temporary folder are still owned and alive, we need
             // to manually remove the folder.
-            let _ = std::fs::remove_dir_all(temp_dir.path());
+            if !save_temps {
+                let _ = std::fs::remove_dir_all(temp_dir.path());
+            }
             std::process::exit(1);
         }
     };
 
     run_tests(
+        dcx,
         opts,
         &rustdoc_options,
         &unused_extern_reports,
@@ -316,6 +325,7 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
 }
 
 pub(crate) fn run_tests(
+    dcx: DiagCtxtHandle<'_>,
     opts: GlobalTestOptions,
     rustdoc_options: &Arc<RustdocOptions>,
     unused_extern_reports: &Arc<Mutex<Vec<UnusedExterns>>>,
@@ -368,6 +378,13 @@ pub(crate) fn run_tests(
             }
             continue;
         }
+
+        if rustdoc_options.merge_doctests == MergeDoctests::Always {
+            let mut diag = dcx.struct_fatal("failed to merge doctests");
+            diag.note("requested explicitly on the command line with `--merge-doctests=yes`");
+            diag.emit();
+        }
+
         // We failed to compile all compatible tests as one so we push them into the
         // `standalone_tests` doctests.
         debug!("Failed to compile compatible doctests for edition {} all at once", edition);
@@ -645,9 +662,9 @@ fn run_test(
             // tested as standalone tests.
             return (Duration::default(), Err(TestFailure::CompileError));
         }
-        if !rustdoc_options.no_capture {
-            // If `no_capture` is disabled, then we don't display rustc's output when compiling
-            // the merged doctests.
+        if !rustdoc_options.no_capture && rustdoc_options.merge_doctests == MergeDoctests::Auto {
+            // If `no_capture` is disabled, and we might fallback to standalone tests, 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
@@ -728,10 +745,12 @@ fn run_test(
             // tested as standalone tests.
             return (instant.elapsed(), Err(TestFailure::CompileError));
         }
-        if !rustdoc_options.no_capture {
-            // If `no_capture` is disabled, then we don't display rustc's output when compiling
-            // the merged doctests.
+        if !rustdoc_options.no_capture && rustdoc_options.merge_doctests == MergeDoctests::Auto {
+            // If `no_capture` is disabled and we're autodetecting whether to merge,
+            // we don't display rustc's output when compiling the merged doctests.
             runner_compiler.stderr(Stdio::null());
+        } else {
+            runner_compiler.stderr(Stdio::inherit());
         }
         runner_compiler.arg("--error-format=short");
         debug!("compiler invocation for doctest runner: {runner_compiler:?}");
@@ -888,7 +907,7 @@ impl IndividualTestOptions {
 
             DirState::Perm(path)
         } else {
-            DirState::Temp(get_doctest_dir().expect("rustdoc needs a tempdir"))
+            DirState::Temp(get_doctest_dir(options).expect("rustdoc needs a tempdir"))
         };
 
         Self { outdir, path: test_path }
@@ -977,21 +996,20 @@ struct CreateRunnableDocTests {
     visited_tests: FxHashMap<(String, usize), usize>,
     unused_extern_reports: Arc<Mutex<Vec<UnusedExterns>>>,
     compiling_test_count: AtomicUsize,
-    can_merge_doctests: bool,
+    can_merge_doctests: MergeDoctests,
 }
 
 impl CreateRunnableDocTests {
     fn new(rustdoc_options: RustdocOptions, opts: GlobalTestOptions) -> CreateRunnableDocTests {
-        let can_merge_doctests = rustdoc_options.edition >= Edition::Edition2024;
         CreateRunnableDocTests {
             standalone_tests: Vec::new(),
             mergeable_tests: FxIndexMap::default(),
-            rustdoc_options: Arc::new(rustdoc_options),
             opts,
             visited_tests: FxHashMap::default(),
             unused_extern_reports: Default::default(),
             compiling_test_count: AtomicUsize::new(0),
-            can_merge_doctests,
+            can_merge_doctests: rustdoc_options.merge_doctests,
+            rustdoc_options: Arc::new(rustdoc_options),
         }
     }
 

src/librustdoc/doctest/make.rs

@@ -20,6 +20,7 @@ use rustc_span::{DUMMY_SP, FileName, Span, kw};
 use tracing::debug;
 
 use super::GlobalTestOptions;
+use crate::config::MergeDoctests;
 use crate::display::Joined as _;
 use crate::html::markdown::LangString;
 
@@ -41,7 +42,7 @@ pub(crate) struct BuildDocTestBuilder<'a> {
     source: &'a str,
     crate_name: Option<&'a str>,
     edition: Edition,
-    can_merge_doctests: bool,
+    can_merge_doctests: MergeDoctests,
     // If `test_id` is `None`, it means we're generating code for a code example "run" link.
     test_id: Option<String>,
     lang_str: Option<&'a LangString>,
@@ -55,7 +56,7 @@ impl<'a> BuildDocTestBuilder<'a> {
             source,
             crate_name: None,
             edition: DEFAULT_EDITION,
-            can_merge_doctests: false,
+            can_merge_doctests: MergeDoctests::Never,
             test_id: None,
             lang_str: None,
             span: DUMMY_SP,
@@ -70,7 +71,7 @@ impl<'a> BuildDocTestBuilder<'a> {
     }
 
     #[inline]
-    pub(crate) fn can_merge_doctests(mut self, can_merge_doctests: bool) -> Self {
+    pub(crate) fn can_merge_doctests(mut self, can_merge_doctests: MergeDoctests) -> Self {
         self.can_merge_doctests = can_merge_doctests;
         self
     }
@@ -117,10 +118,6 @@ impl<'a> BuildDocTestBuilder<'a> {
             span,
             global_crate_attrs,
         } = self;
-        let can_merge_doctests = can_merge_doctests
-            && lang_str.is_some_and(|lang_str| {
-                !lang_str.compile_fail && !lang_str.test_harness && !lang_str.standalone_crate
-            });
 
         let result = rustc_driver::catch_fatal_errors(|| {
             rustc_span::create_session_if_not_set_then(edition, |_| {
@@ -155,14 +152,27 @@ impl<'a> BuildDocTestBuilder<'a> {
         debug!("crate_attrs:\n{crate_attrs}{maybe_crate_attrs}");
         debug!("crates:\n{crates}");
         debug!("after:\n{everything_else}");
-
-        // If it contains `#[feature]` or `#[no_std]`, we don't want it to be merged either.
-        let can_be_merged = can_merge_doctests
-            && !has_global_allocator
-            && crate_attrs.is_empty()
-            // If this is a merged doctest and a defined macro uses `$crate`, then the path will
-            // not work, so better not put it into merged doctests.
-            && !(has_macro_def && everything_else.contains("$crate"));
+        debug!("merge-doctests: {can_merge_doctests:?}");
+
+        // Up until now, we've been dealing with settings for the whole crate.
+        // Now, infer settings for this particular test.
+        //
+        // Avoid tests with incompatible attributes.
+        let opt_out = lang_str.is_some_and(|lang_str| {
+            lang_str.compile_fail || lang_str.test_harness || lang_str.standalone_crate
+        });
+        let can_be_merged = if can_merge_doctests == MergeDoctests::Auto {
+            // We try to look at the contents of the test to detect whether it should be merged.
+            // This is not a complete list of possible failures, but it catches many cases.
+            let will_probably_fail = has_global_allocator
+                || !crate_attrs.is_empty()
+                // If this is a merged doctest and a defined macro uses `$crate`, then the path will
+                // not work, so better not put it into merged doctests.
+                || (has_macro_def && everything_else.contains("$crate"));
+            !opt_out && !will_probably_fail
+        } else {
+            can_merge_doctests != MergeDoctests::Never && !opt_out
+        };
         DocTestBuilder {
             supports_color,
             has_main_fn,

src/librustdoc/doctest/markdown.rs

@@ -3,6 +3,7 @@
 use std::fs::read_to_string;
 use std::sync::{Arc, Mutex};
 
+use rustc_errors::DiagCtxtHandle;
 use rustc_session::config::Input;
 use rustc_span::{DUMMY_SP, FileName};
 use tempfile::tempdir;
@@ -78,7 +79,7 @@ impl DocTestVisitor for MdCollector {
 }
 
 /// Runs any tests/code examples in the markdown file `options.input`.
-pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
+pub(crate) fn test(input: &Input, options: Options, dcx: DiagCtxtHandle<'_>) -> Result<(), String> {
     let input_str = match input {
         Input::File(path) => {
             read_to_string(path).map_err(|err| format!("{}: {err}", path.display()))?
@@ -118,6 +119,7 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
     let CreateRunnableDocTests { opts, rustdoc_options, standalone_tests, mergeable_tests, .. } =
         collector;
     crate::doctest::run_tests(
+        dcx,
         opts,
         &rustdoc_options,
         &Arc::new(Mutex::new(Vec::new())),

src/librustdoc/lib.rs

@@ -544,6 +544,14 @@ fn opts() -> Vec<RustcOptGroup> {
             "[toolchain-shared-resources,invocation-specific,dep-info]",
         ),
         opt(Unstable, FlagMulti, "", "no-run", "Compile doctests without running them", ""),
+        opt(
+            Unstable,
+            Opt,
+            "",
+            "merge-doctests",
+            "Force all doctests to be compiled as a single binary, instead of one binary per test. If merging fails, rustdoc will emit a hard error.",
+            "yes|no|auto",
+        ),
         opt(
             Unstable,
             Multi,
@@ -822,7 +830,7 @@ fn main_args(early_dcx: &mut EarlyDiagCtxt, at_args: &[String]) {
         options.should_test || output_format == config::OutputFormat::Doctest,
         config::markdown_input(&input),
     ) {
-        (true, Some(_)) => return wrap_return(dcx, doctest::test_markdown(&input, options)),
+        (true, Some(_)) => return wrap_return(dcx, doctest::test_markdown(&input, options, dcx)),
         (true, None) => return doctest::run(dcx, input, options),
         (false, Some(md_input)) => {
             let md_input = md_input.to_owned();

tests/run-make/rustdoc-default-output/output-default.stdout

@@ -154,6 +154,10 @@ Options:
                         Comma separated list of types of output for rustdoc to
                         emit
         --no-run        Compile doctests without running them
+        --merge-doctests yes|no|auto
+                        Force all doctests to be compiled as a single binary,
+                        instead of one binary per test. If merging fails,
+                        rustdoc will emit a hard error.
         --remap-path-prefix FROM=TO
                         Remap source names in compiler messages
         --show-type-layout 

tests/rustdoc-ui/doctest/force-merge-default-not-override.rs

@@ -0,0 +1,25 @@
+//@ check-pass
+//@ edition: 2024
+//@ compile-flags: --test --test-args=--test-threads=1 --merge-doctests=yes -Z unstable-options
+//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR"
+//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME"
+//@ normalize-stdout: "ran in \d+\.\d+s" -> "ran in $$TIME"
+//@ normalize-stdout: "compilation took \d+\.\d+s" -> "compilation took $$TIME"
+//@ normalize-stdout: ".rs:\d+:\d+" -> ".rs:$$LINE:$$COL"
+
+// FIXME: compiletest doesn't support `// RAW` for doctests because the progress messages aren't
+// emitted as JSON. Instead the .stderr file tests that this doesn't contains a
+// "merged compilation took ..." message.
+
+/// ```standalone_crate
+/// let x = 12;
+/// ```
+///
+/// These two doctests should be not be merged, even though this passes `--merge-doctests=yes`.
+///
+/// ```standalone_crate
+/// fn main() {
+///     println!("owo");
+/// }
+/// ```
+pub struct Foo;

tests/rustdoc-ui/doctest/force-merge-default-not-override.stdout

@@ -0,0 +1,7 @@
+
+running 2 tests
+test $DIR/force-merge-default-not-override.rs - Foo (line 14) ... ok
+test $DIR/force-merge-default-not-override.rs - Foo (line 20) ... ok
+
+test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+