Correctly handle should_panic doctest attribute

Author: GuillaumeGomez

src/librustdoc/doctest.rs

@@ -336,6 +336,7 @@ pub(crate) fn run_tests(
                 scraped_test.langstr.test_harness,
                 &opts,
                 Some(&opts.crate_name),
+                scraped_test.langstr.should_panic,
             );
             standalone_tests.push(generate_test_desc_and_fn(
                 doctest,
@@ -773,9 +774,11 @@ fn run_test(
     match result {
         Err(e) => return Err(TestFailure::ExecutionError(e)),
         Ok(out) => {
-            if langstr.should_panic && out.status.success() {
+            // If the test panicked, then `should_panic` will return 0 as exit code as it is the
+            // the expected result.
+            if langstr.should_panic && !out.status.success() {
                 return Err(TestFailure::UnexpectedRunPass);
-            } else if !langstr.should_panic && !out.status.success() {
+            } else if !out.status.success() {
                 return Err(TestFailure::ExecutionFailure(out));
             }
         }
@@ -1059,6 +1062,7 @@ fn doctest_run_fn(
         scraped_test.langstr.test_harness,
         &global_opts,
         Some(&global_opts.crate_name),
+        scraped_test.langstr.should_panic,
     );
     let runnable_test = RunnableDocTest {
         full_test_code: wrapped.to_string(),
@@ -1083,7 +1087,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/extracted.rs

@@ -60,6 +60,7 @@ impl ExtractedDocTests {
             langstr.test_harness,
             opts,
             Some(&opts.crate_name),
+            langstr.should_panic,
         );
         self.doctests.push(ExtractedDocTest {
             file: filename.prefer_remapped_unconditionally().to_string(),

src/librustdoc/doctest/make.rs

@@ -270,6 +270,29 @@ impl std::string::ToString for DocTestWrapResult {
     }
 }
 
+/// `catch_unwind` wrapper to ensure that a `should_panic` doctest actually panicked.
+///
+/// Returns `(wrapper_pre, wrapper_post)`.
+pub(crate) fn should_panic_wrapper(
+    should_panic: bool,
+    returns_result: bool,
+) -> (&'static str, String) {
+    if !should_panic {
+        return ("", String::new());
+    }
+    (
+        "match std::panic::catch_unwind(core::panic::AssertUnwindSafe(|| {\n",
+        format!(
+            "
+}})) {{
+    Ok(_) => std::process::exit(1),
+    {}
+}}\n",
+            if returns_result { "Err(err) => err," } else { "Err(_) => {}" },
+        ),
+    )
+}
+
 impl DocTestBuilder {
     fn invalid(
         global_crate_attrs: Vec<String>,
@@ -302,6 +325,7 @@ impl DocTestBuilder {
         dont_insert_main: bool,
         opts: &GlobalTestOptions,
         crate_name: Option<&str>,
+        should_panic: bool,
     ) -> (DocTestWrapResult, usize) {
         if self.invalid_ast {
             // If the AST failed to compile, no need to go generate a complete doctest, the error
@@ -384,20 +408,24 @@ impl DocTestBuilder {
                 "_inner".into()
             };
             let inner_attr = if self.test_id.is_some() { "#[allow(non_snake_case)] " } else { "" };
+            let (should_panic_pre, should_panic_post) =
+                should_panic_wrapper(should_panic, returns_result);
             let (main_pre, main_post) = if returns_result {
                 (
                     format!(
-                        "fn main() {{ {inner_attr}fn {inner_fn_name}() -> core::result::Result<(), impl core::fmt::Debug> {{\n",
+                        "\
+fn main() {{ {inner_attr}fn {inner_fn_name}() -> core::result::Result<(), impl core::fmt::Debug> {{
+{should_panic_pre}",
                     ),
-                    format!("\n}} {inner_fn_name}().unwrap() }}"),
+                    format!("{should_panic_post}\n}} {inner_fn_name}().unwrap() }}"),
                 )
             } else if self.test_id.is_some() {
                 (
-                    format!("fn main() {{ {inner_attr}fn {inner_fn_name}() {{\n",),
-                    format!("\n}} {inner_fn_name}() }}"),
+                    format!("fn main() {{ {inner_attr}fn {inner_fn_name}() {{\n{should_panic_pre}"),
+                    format!("{should_panic_post}\n}} {inner_fn_name}() }}"),
                 )
             } else {
-                ("fn main() {\n".into(), "\n}".into())
+                (format!("fn main() {{\n{should_panic_pre}"), format!("{should_panic_post}\n}}"))
             };
             // Note on newlines: We insert a line/newline *before*, and *after*
             // the doctest and adjust the `line_offset` accordingly.

src/librustdoc/doctest/runner.rs

@@ -129,15 +129,19 @@ 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 let Some(code) = out.status.code() {{
-                eprintln!(\"Test executable failed (exit status: {{code}}).\");
+                if should_panic {{
+                    eprintln!(\"Test didn't panic, but it's marked `should_panic`.\");
+                }} else {{
+                    eprintln!(\"Test executable failed (exit status: {{code}}).\");
+                }}
             }} else {{
                 eprintln!(\"Test executable failed (terminated by signal).\");
             }}
@@ -220,34 +224,40 @@ fn generate_mergeable_doctest(
     output_merged_tests: &mut String,
 ) -> String {
     let test_id = format!("__doctest_{id}");
+    let should_panic = !scraped_test.langstr.no_run && scraped_test.langstr.should_panic;
 
     if ignore {
         // We generate nothing else.
         writeln!(output, "pub mod {test_id} {{}}\n").unwrap();
     } else {
         writeln!(output, "pub mod {test_id} {{\n{}{}", doctest.crates, doctest.maybe_crate_attrs)
             .unwrap();
+        let mut returns_result = false;
         if doctest.has_main_fn {
             output.push_str(&doctest.everything_else);
         } else {
-            let returns_result = if doctest.everything_else.trim_end().ends_with("(())") {
+            let return_value = if doctest.everything_else.trim_end().ends_with("(())") {
+                returns_result = true;
                 "-> Result<(), impl core::fmt::Debug>"
             } else {
                 ""
             };
             write!(
                 output,
                 "\
-fn main() {returns_result} {{
+fn main() {return_value} {{
 {}
 }}",
                 doctest.everything_else
             )
             .unwrap();
         }
+        let (should_panic_pre, should_panic_post) =
+            super::make::should_panic_wrapper(should_panic, returns_result);
         writeln!(
             output,
-            "\npub fn __main_fn() -> impl std::process::Termination {{ main() }} \n}}\n"
+            "\npub fn __main_fn() -> impl std::process::Termination {{{should_panic_pre}main()\
+{should_panic_post}}} \n}}\n"
         )
         .unwrap();
     }
@@ -257,7 +267,7 @@ fn main() {returns_result} {{
         "
 mod {test_id} {{
 pub const TEST: test::TestDescAndFn = test::TestDescAndFn::new_doctest(
-{test_name:?}, {ignore}, {file:?}, {line}, {no_run}, {should_panic},
+{test_name:?}, {ignore}, {file:?}, {line}, {no_run}, false,
 test::StaticTestFn(
     || {{{runner}}},
 ));
@@ -266,7 +276,6 @@ test::StaticTestFn(
         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,
         // 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 {
@@ -275,7 +284,7 @@ test::StaticTestFn(
             format!(
                 "
 if let Some(bin_path) = crate::__doctest_mod::doctest_path() {{
-    test::assert_test_result(crate::__doctest_mod::doctest_runner(bin_path, {id}))
+    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())
 }}

src/librustdoc/doctest/tests.rs

@@ -25,7 +25,7 @@ fn make_test(
     }
     let doctest = builder.build(None);
     let (wrapped, line_offset) =
-        doctest.generate_unique_doctest(test_code, dont_insert_main, opts, crate_name);
+        doctest.generate_unique_doctest(test_code, dont_insert_main, opts, crate_name, false);
     (wrapped.to_string(), line_offset)
 }
 

src/librustdoc/html/markdown.rs

@@ -308,7 +308,8 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
                 builder = builder.crate_name(krate);
             }
             let doctest = builder.build(None);
-            let (wrapped, _) = doctest.generate_unique_doctest(&test, false, &opts, krate);
+            let (wrapped, _) =
+                doctest.generate_unique_doctest(&test, false, &opts, krate, should_panic);
             let test = wrapped.to_string();
             let channel = if test.contains("#![feature(") { "&amp;version=nightly" } else { "" };