Auto merge of #143900 - GuillaumeGomez:fix-no-run, r=<try>

[rustdoc] Correctly handle `should_panic` doctest attribute and fix `--no-run` test flag on the 2024 edition

try-job: x86_64-gnu-aux
try-job: test-various

Author: rust-bors[bot]

library/std/src/error.rs

@@ -123,7 +123,7 @@ use crate::fmt::{self, Write};
 /// the `Debug` output means `Report` is an ideal starting place for formatting errors returned
 /// from `main`.
 ///
-/// ```should_panic
+/// ```
 /// #![feature(error_reporter)]
 /// use std::error::Report;
 /// # use std::error::Error;
@@ -154,10 +154,14 @@ use crate::fmt::{self, Write};
 /// #     Err(SuperError { source: SuperErrorSideKick })
 /// # }
 ///
-/// fn main() -> Result<(), Report<SuperError>> {
+/// fn run() -> Result<(), Report<SuperError>> {
 ///     get_super_error()?;
 ///     Ok(())
 /// }
+///
+/// fn main() {
+///     assert!(run().is_err());
+/// }
 /// ```
 ///
 /// This example produces the following output:
@@ -170,7 +174,7 @@ use crate::fmt::{self, Write};
 /// output format. If you want to make sure your `Report`s are pretty printed and include backtrace
 /// you will need to manually convert and enable those flags.
 ///
-/// ```should_panic
+/// ```
 /// #![feature(error_reporter)]
 /// use std::error::Report;
 /// # use std::error::Error;
@@ -201,12 +205,16 @@ use crate::fmt::{self, Write};
 /// #     Err(SuperError { source: SuperErrorSideKick })
 /// # }
 ///
-/// fn main() -> Result<(), Report<SuperError>> {
+/// fn run() -> Result<(), Report<SuperError>> {
 ///     get_super_error()
 ///         .map_err(Report::from)
 ///         .map_err(|r| r.pretty(true).show_backtrace(true))?;
 ///     Ok(())
 /// }
+///
+/// fn main() {
+///     assert!(run().is_err());
+/// }
 /// ```
 ///
 /// This example produces the following output:

src/librustdoc/doctest.rs

@@ -7,6 +7,8 @@ mod rust;
 use std::fs::File;
 use std::hash::{Hash, Hasher};
 use std::io::{self, Write};
+#[cfg(unix)]
+use std::os::unix::process::ExitStatusExt;
 use std::path::{Path, PathBuf};
 use std::process::{self, Command, Stdio};
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -351,7 +353,7 @@ pub(crate) fn run_tests(
         );
 
         for (doctest, scraped_test) in &doctests {
-            tests_runner.add_test(doctest, scraped_test, &target_str);
+            tests_runner.add_test(doctest, scraped_test, &target_str, rustdoc_options);
         }
         let (duration, ret) = tests_runner.run_merged_tests(
             rustdoc_test_options,
@@ -801,6 +803,22 @@ fn run_test(
     let duration = instant.elapsed();
     if doctest.no_run {
         return (duration, Ok(()));
+    } else if doctest.langstr.should_panic
+        // Equivalent of:
+        //
+        // ```
+        // (cfg!(target_family = "wasm") || cfg!(target_os = "zkvm"))
+        //     && !cfg!(target_os = "emscripten")
+        // ```
+        && let TargetTuple::TargetTuple(ref s) = rustdoc_options.target
+        && let mut iter = s.split('-')
+        && let Some(arch) = iter.next()
+        && iter.next().is_some()
+        && let os = iter.next()
+        && (arch.starts_with("wasm") || os == Some("zkvm")) && os != Some("emscripten")
+    {
+        // We cannot correctly handle `should_panic` in some wasm targets so we exit early.
+        return (duration, Ok(()));
     }
 
     // Run the code!
@@ -831,13 +849,40 @@ fn run_test(
     } else {
         cmd.output()
     };
+
+    // FIXME: Make `test::get_result_from_exit_code` public and use this code instead of this.
+    //
+    // On Zircon (the Fuchsia kernel), an abort from userspace calls the
+    // LLVM implementation of __builtin_trap(), e.g., ud2 on x86, which
+    // raises a kernel exception. If a userspace process does not
+    // otherwise arrange exception handling, the kernel kills the process
+    // with this return code.
+    #[cfg(target_os = "fuchsia")]
+    const ZX_TASK_RETCODE_EXCEPTION_KILL: i32 = -1028;
+    // On Windows we use __fastfail to abort, which is documented to use this
+    // exception code.
+    #[cfg(windows)]
+    const STATUS_FAIL_FAST_EXCEPTION: i32 = 0xC0000409u32 as i32;
+    #[cfg(unix)]
+    const SIGABRT: std::ffi::c_int = 6;
     match result {
         Err(e) => return (duration, Err(TestFailure::ExecutionError(e))),
         Ok(out) => {
-            if langstr.should_panic && out.status.success() {
-                return (duration, Err(TestFailure::UnexpectedRunPass));
-            } else if !langstr.should_panic && !out.status.success() {
+            if !langstr.should_panic && !out.status.success() {
                 return (duration, Err(TestFailure::ExecutionFailure(out)));
+            } else if langstr.should_panic {
+                match out.status.code() {
+                    Some(test::ERROR_EXIT_CODE) => {}
+                    #[cfg(windows)]
+                    Some(STATUS_FAIL_FAST_EXCEPTION) => {}
+                    #[cfg(unix)]
+                    None if out.status.signal() == Some(SIGABRT) => {}
+                    // Upon an abort, Fuchsia returns the status code
+                    // `ZX_TASK_RETCODE_EXCEPTION_KILL`.
+                    #[cfg(target_os = "fuchsia")]
+                    Some(ZX_TASK_RETCODE_EXCEPTION_KILL) => {}
+                    _ => return (duration, Err(TestFailure::UnexpectedRunPass)),
+                }
             }
         }
     }
@@ -1144,7 +1189,7 @@ fn doctest_run_fn(
                 eprint!("Test compiled successfully, but it's marked `compile_fail`.");
             }
             TestFailure::UnexpectedRunPass => {
-                eprint!("Test executable succeeded, but it's marked `should_panic`.");
+                eprint!("Test didn't panic, but it's marked `should_panic`.");
             }
             TestFailure::MissingErrorCodes(codes) => {
                 eprint!("Some expected error codes were not found: {codes:?}");

src/librustdoc/doctest/runner.rs

@@ -39,6 +39,7 @@ impl DocTestRunner {
         doctest: &DocTestBuilder,
         scraped_test: &ScrapedDocTest,
         target_str: &str,
+        opts: &RustdocOptions,
     ) {
         let ignore = match scraped_test.langstr.ignore {
             Ignore::All => true,
@@ -62,6 +63,7 @@ impl DocTestRunner {
                 self.nb_tests,
                 &mut self.output,
                 &mut self.output_merged_tests,
+                opts,
             ),
         ));
         self.supports_color &= doctest.supports_color;
@@ -134,13 +136,41 @@ mod __doctest_mod {{
     }}
 
     #[allow(unused)]
-    pub fn doctest_runner(bin: &std::path::Path, test_nb: usize) -> ExitCode {{
+    pub fn doctest_runner(bin: &std::path::Path, test_nb: usize, should_panic: bool) -> ExitCode {{
         let out = std::process::Command::new(bin)
             .env(self::RUN_OPTION, test_nb.to_string())
             .args(std::env::args().skip(1).collect::<Vec<_>>())
             .output()
             .expect(\"failed to run command\");
-        if !out.status.success() {{
+        if should_panic {{
+            // FIXME: Make `test::get_result_from_exit_code` public and use this code instead of this.
+            //
+            // On Zircon (the Fuchsia kernel), an abort from userspace calls the
+            // LLVM implementation of __builtin_trap(), e.g., ud2 on x86, which
+            // raises a kernel exception. If a userspace process does not
+            // otherwise arrange exception handling, the kernel kills the process
+            // with this return code.
+            #[cfg(target_os = \"fuchsia\")]
+            const ZX_TASK_RETCODE_EXCEPTION_KILL: i32 = -1028;
+            // On Windows we use __fastfail to abort, which is documented to use this
+            // exception code.
+            #[cfg(windows)]
+            const STATUS_FAIL_FAST_EXCEPTION: i32 = 0xC0000409u32 as i32;
+            #[cfg(unix)]
+            const SIGABRT: std::ffi::c_int = 6;
+
+            match out.status.code() {{
+                Some(test::ERROR_EXIT_CODE) => ExitCode::SUCCESS,
+                #[cfg(windows)]
+                Some(STATUS_FAIL_FAST_EXCEPTION) => ExitCode::SUCCESS,
+                #[cfg(unix)]
+                None if out.status.signal() == Some(libc::SIGABRT) => ExitCode::SUCCESS,
+                // Upon an abort, Fuchsia returns the status code ZX_TASK_RETCODE_EXCEPTION_KILL.
+                #[cfg(target_os = \"fuchsia\")]
+                Some(ZX_TASK_RETCODE_EXCEPTION_KILL) => ExitCode::SUCCESS,
+                _ => ExitCode::FAILURE,
+            }}
+        }} else if !out.status.success() {{
             if let Some(code) = out.status.code() {{
                 eprintln!(\"Test executable failed (exit status: {{code}}).\");
             }} else {{
@@ -223,6 +253,7 @@ fn generate_mergeable_doctest(
     id: usize,
     output: &mut String,
     output_merged_tests: &mut String,
+    opts: &RustdocOptions,
 ) -> String {
     let test_id = format!("__doctest_{id}");
 
@@ -256,31 +287,33 @@ fn main() {returns_result} {{
         )
         .unwrap();
     }
-    let not_running = ignore || scraped_test.langstr.no_run;
+    let should_panic = scraped_test.langstr.should_panic;
+    let not_running = ignore || scraped_test.no_run(opts);
     writeln!(
         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_name:?}, {ignore} || ({should_panic} && crate::__doctest_mod::SHOULD_PANIC_DISABLED), {file:?}, {line}, {no_run}, false,
 test::StaticTestFn(
     || {{{runner}}},
 ));
 }}",
         test_name = scraped_test.name,
         file = scraped_test.path(),
         line = scraped_test.line,
-        no_run = scraped_test.langstr.no_run,
-        should_panic = !scraped_test.langstr.no_run && scraped_test.langstr.should_panic,
+        no_run = scraped_test.no_run(opts),
         // Setting `no_run` to `true` in `TestDesc` still makes the test run, so we simply
         // don't give it the function to run.
         runner = if not_running {
             "test::assert_test_result(Ok::<(), String>(()))".to_string()
         } else {
             format!(
                 "
-if let Some(bin_path) = crate::__doctest_mod::doctest_path() {{
-    test::assert_test_result(crate::__doctest_mod::doctest_runner(bin_path, {id}))
+if {should_panic} && crate::__doctest_mod::SHOULD_PANIC_DISABLED {{
+    test::assert_test_result(Ok::<(), String>(()))
+}} else if let Some(bin_path) = crate::__doctest_mod::doctest_path() {{
+    test::assert_test_result(crate::__doctest_mod::doctest_runner(bin_path, {id}, {should_panic}))
 }} else {{
     test::assert_test_result(doctest_bundle::{test_id}::__main_fn())
 }}

tests/run-make/rustdoc-should-panic/rmake.rs

@@ -0,0 +1,43 @@
+// Ensure that `should_panic` doctests only succeed if the test actually panicked.
+// Regression test for <https://github.com/rust-lang/rust/issues/143009>.
+
+//@ ignore-cross-compile
+
+use run_make_support::rustdoc;
+
+fn check_output(edition: &str, panic_abort: bool) {
+    let mut rustdoc_cmd = rustdoc();
+    rustdoc_cmd.input("test.rs").arg("--test").edition(edition);
+    if panic_abort {
+        rustdoc_cmd.args(["-C", "panic=abort"]);
+    }
+    let output = rustdoc_cmd.run_fail().stdout_utf8();
+    let should_contain = &[
+        "test test.rs - bad_exit_code (line 1) ... FAILED",
+        "test test.rs - did_not_panic (line 6) ... FAILED",
+        "test test.rs - did_panic (line 11) ... ok",
+        "---- test.rs - bad_exit_code (line 1) stdout ----
+Test executable failed (exit status: 1).",
+        "---- test.rs - did_not_panic (line 6) stdout ----
+Test didn't panic, but it's marked `should_panic`.",
+        "test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out;",
+    ];
+    for text in should_contain {
+        assert!(
+            output.contains(text),
+            "output (edition: {edition}) doesn't contain {:?}\nfull output: {output}",
+            text
+        );
+    }
+}
+
+fn main() {
+    check_output("2015", false);
+
+    // Same check with the merged doctest feature (enabled with the 2024 edition).
+    check_output("2024", false);
+
+    // Checking that `-C panic=abort` is working too.
+    check_output("2015", true);
+    check_output("2024", true);
+}

tests/run-make/rustdoc-should-panic/test.rs

@@ -0,0 +1,14 @@
+/// ```
+/// std::process::exit(1);
+/// ```
+fn bad_exit_code() {}
+
+/// ```should_panic
+/// std::process::exit(1);
+/// ```
+fn did_not_panic() {}
+
+/// ```should_panic
+/// panic!("yeay");
+/// ```
+fn did_panic() {}

tests/rustdoc-ui/doctest/failed-doctest-should-panic-2021.stdout

@@ -5,7 +5,7 @@ test $DIR/failed-doctest-should-panic-2021.rs - Foo (line 10) ... FAILED
 failures:
 
 ---- $DIR/failed-doctest-should-panic-2021.rs - Foo (line 10) stdout ----
-Test executable succeeded, but it's marked `should_panic`.
+Test didn't panic, but it's marked `should_panic`.
 
 failures:
     $DIR/failed-doctest-should-panic-2021.rs - Foo (line 10)

tests/rustdoc-ui/doctest/failed-doctest-should-panic.stdout

@@ -1,11 +1,12 @@
 
 running 1 test
-test $DIR/failed-doctest-should-panic.rs - Foo (line 12) - should panic ... FAILED
+test $DIR/failed-doctest-should-panic.rs - Foo (line 12) ... FAILED
 
 failures:
 
 ---- $DIR/failed-doctest-should-panic.rs - Foo (line 12) stdout ----
-note: test did not panic as expected at $DIR/failed-doctest-should-panic.rs:12:0
+Test didn't panic, but it's marked `should_panic`.
+
 
 failures:
     $DIR/failed-doctest-should-panic.rs - Foo (line 12)

tests/rustdoc-ui/doctest/no-run.edition2021.stdout

@@ -0,0 +1,12 @@
+
+running 7 tests
+test $DIR/no-run.rs - f (line 14) - compile ... ok
+test $DIR/no-run.rs - f (line 17) - compile ... ok
+test $DIR/no-run.rs - f (line 20) ... ignored
+test $DIR/no-run.rs - f (line 23) - compile ... ok
+test $DIR/no-run.rs - f (line 29) - compile fail ... ok
+test $DIR/no-run.rs - f (line 34) - compile ... ok
+test $DIR/no-run.rs - f (line 38) - compile ... ok
+
+test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in $TIME
+

tests/rustdoc-ui/doctest/no-run.edition2024.stdout

@@ -0,0 +1,18 @@
+
+running 5 tests
+test $DIR/no-run.rs - f (line 14) - compile ... ok
+test $DIR/no-run.rs - f (line 17) - compile ... ok
+test $DIR/no-run.rs - f (line 23) - compile ... ok
+test $DIR/no-run.rs - f (line 34) - compile ... ok
+test $DIR/no-run.rs - f (line 38) - compile ... ok
+
+test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+
+
+running 2 tests
+test $DIR/no-run.rs - f (line 20) ... ignored
+test $DIR/no-run.rs - f (line 29) - compile fail ... ok
+
+test result: ok. 1 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in $TIME
+
+all doctests ran in $TIME; merged doctests compilation took $TIME