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

[GuillaumeGomez]

Fixes #143009.
Fixes #143858.

Since it includes fixes from #143453, it's taking it over (commits 2, 3 and 4 are from #143453).

For --no-run, we forgot to check the "global" options in the 2024 edition, fixed in the first commit.

For should_panic fix, the exit code check has been fixed.

cc @TroyKomodo (thanks so much for providing such a complete test, made my life a lot easier!)
r? @notriddle

[rustbot]

This PR modifies run-make tests.

cc @jieyouxu

[purplesyringa]

For should_panic fix, instead of checking the exit code, we directly wrap the doctest code with catch_unwind and if a panic happened, then we return success for the test, otherwise we display the appropriate error message.

I... do not see anything like that in the patch? There's two checks for exit code 101.

[GuillaumeGomez]

Woups, copied/pasted comment from original PR which was outdated after discussion with you (on the previous PR). ^^'

EDIT: Updated comment.

[purplesyringa]

if langstr.should_panic {
    if out.status.code() == Some(101) {
        return Ok(());
    } else if out.status.success() {
        return Err(TestFailure::UnexpectedRunPass);
    }
}
if !out.status.success() {
    return Err(TestFailure::ExecutionFailure(out));
}

Do you think something like this would make sense, so that it's easier for the user to differentiate between aborts and panic-less success?

[GuillaumeGomez]

TestFailure::UnexpectedRunPass displays "Test didn't panic, but it's marked `should_panic`.", which means we'd need to add a new case. Do you think it's necessary to differentiate between success and non-panic failures?

[purplesyringa]

Wouldn't changing UnexpectedRunPass to spell "Test executable succeeded, but it's marked should_panic." (i.e. rolling back the change in this PR) work? After all, if the change was to let UnexpectedRunPass represent non-panicking failures, it won't be necessary if we just use ExecutionFailure for that.

[GuillaumeGomez]

That wouldn't cover exit(1) anymore if we did so.

[purplesyringa]

I've amended my comment a moment before you replied, apologies.

[GuillaumeGomez]

Replied too fast then. 😅

The message for ExecutionFailure is too general imo ("Test executable failed"). It doesn't help to understand should_panic didn't panic. I think once this PR is merged, you can send a new one to create a new variant in case it failed but didn't panic, like that we can have a message that is covering this case. What do you think?

[purplesyringa]

The message for ExecutionFailure is too general imo ("Test executable failed"). It doesn't help to understand should_panic didn't panic.

I think we're talking past each other. I suggest that:

• If the program didn't panic and exited with code 0, we keep using UnexpectedRunPass ("test didn't panic"). This is the main use case for should_panic and has a perfectly legible description.
• If the program didn't panic and exited with a non-zero code, we use ExecutionFailure ("executable failed"). I believe that it's highly unlikely this failure mode of a should_panic test can be attributed to anything but UB or an abort, exactly like with non-should_panic tests, so I don't think it's necessary to specify that it's happened in a should_panic test specifically.

Wouldn't that work?

[GuillaumeGomez]

I still think it's too general. For end users, should_panic means the execution is supposed to fail. However it might be confusing to get "executable failed" when it's exactly what's expected (well, except it was expecting a panic and it's different). Hence why I think the current message is better and why, if we go down this road, we should add a new case to handle this case instead of relying on the too generated ExecutionFailure.

[purplesyringa]

That makes sense, yeah. Let's delay it until after this PR lands then.

[bors]

☔ The latest upstream changes (presumably #144692) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

Why can't I reproduce this bug locally? O.o

[samueltardieu]

Maybe attempt a bors try (I haven't the required privileges to do it, you'll have to request it from someone else).

[GuillaumeGomez]

I have the needed rights, just surprised that it keeps failing in CI and not locally.

@bors try

[rust-bors]

⌛ Trying commit 7b8015c with merge 266f8fc…

To cancel the try build, run the command @bors try cancel.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 266f8fc (266f8fcd236482895d8d611731e49335a8b66f0d, parent: 889701db1ff614160314734fe4138c2f820a95bb)

[GuillaumeGomez]

So seems like it's another PR that creates an issue with this common PR. Let's wait a bit before putting it back in I guess...

[samueltardieu]

Since I've r-ed -:, let's r+ back but iffy:

@bors r=lolbinarycat,fmease rollup=iffy

[bors]

📌 Commit 7b8015c has been approved by lolbinarycat,fmease

It is now in the queue for this repository.

[bors]

⌛ Testing commit 7b8015c with merge 058edea...

[Zalathar]

Yielding to enclosing rollup (#144844):

@bors retry

[bors]

⌛ Testing commit 7b8015c with merge b6a6d52...

[Zalathar]

Yielding to enclosing rollup (#144845) again:

@bors retry

[bors]

⌛ Testing commit 7b8015c with merge 0721cd7...

[rust-log-analyzer]

The job test-various failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

---- [run-make] tests/run-make/rustdoc-should-panic stdout ----

error: rmake recipe failed to complete
status: exit status: 101
command: cd "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/rustdoc-should-panic/rmake_out" && env -u RUSTFLAGS -u __RUSTC_DEBUG_ASSERTIONS_ENABLED -u __STD_DEBUG_ASSERTIONS_ENABLED AR="/wasi-sdk-27.0-x86_64-linux/bin/llvm-ar" BUILD_ROOT="/checkout/obj/build/x86_64-unknown-linux-gnu" CARGO="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools-bin/cargo" CC="/wasi-sdk-27.0-x86_64-linux/bin/wasm32-wasip1-clang" CC_DEFAULT_FLAGS="-ffunction-sections -fdata-sections -fno-exceptions --target=wasm32-wasip1" CXX="/wasi-sdk-27.0-x86_64-linux/bin/wasm32-wasip1-clang++" CXX_DEFAULT_FLAGS="-ffunction-sections -fdata-sections -fno-exceptions --target=wasm32-wasip1" HOST_RUSTC_DYLIB_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/lib" LD_LIB_PATH_ENVVAR="LD_LIBRARY_PATH" LLVM_BIN_DIR="/checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm/bin" LLVM_COMPONENTS="aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgputargetmca amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard cgdata codegen codegentypes core coroutines coverage csky cskyasmparser cskycodegen cskydesc cskydisassembler cskyinfo debuginfobtf debuginfocodeview debuginfodwarf debuginfogsym debuginfologicalview debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker dwarflinkerclassic dwarflinkerparallel dwp engine executionengine extensions filecheck frontendatomic frontenddriver frontendhlsl frontendoffloading frontendopenacc frontendopenmp fuzzercli fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo hipstdpar instcombine instrumentation interfacestub interpreter ipo irprinter irreader jitlink libdriver lineeditor linker loongarch loongarchasmparser loongarchcodegen loongarchdesc loongarchdisassembler loongarchinfo lto m68k m68kasmparser m68kcodegen m68kdesc m68kdisassembler m68kinfo mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts objcopy object objectyaml option orcdebugging orcjit orcshared orctargetprocess passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvtargetmca runtimedyld sandboxir scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target targetparser telemetry textapi textapibinaryreader transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo webassemblyutils windowsdriver windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86targetmca xray xtensa xtensaasmparser xtensacodegen xtensadesc xtensadisassembler xtensainfo" LLVM_FILECHECK="/checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm/bin/FileCheck" NODE="/node/bin/node" PYTHON="/usr/bin/python3" RUNNER="/wasmtime-v19.0.0-x86_64-linux/wasmtime run -C cache=n --dir . --env RUSTC_BOOTSTRAP" RUSTC="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" RUSTDOC="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" SOURCE_ROOT="/checkout" TARGET="wasm32-wasip1" TARGET_EXE_DYLIB_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/wasm32-wasip1/lib" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/rustdoc-should-panic/rmake"
stdout: none
--- stderr -------------------------------

thread 'main' panicked at /checkout/tests/run-make/rustdoc-should-panic/rmake.rs:20:9:
output (edition: 2015) doesn't contain "test test.rs - did_panic (line 11) ... ok"
full output: 
running 3 tests
test test.rs - did_panic (line 11) ... FAILED
test test.rs - bad_exit_code (line 1) ... FAILED
test test.rs - did_not_panic (line 6) ... FAILED

[bors]

💔 Test failed - checks-actions