From d5ba5c1c925f82de2efbfbbe92ffee56a6a6a334 Mon Sep 17 00:00:00 2001 From: Jules Bertholet Date: Fri, 3 Oct 2025 16:00:15 -0400 Subject: [PATCH 01/16] Mark `PatternTypo` suggestion as maybe incorrect --- compiler/rustc_passes/src/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index f0726014e0afa..e01e9e384c035 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1365,7 +1365,7 @@ pub(crate) struct UnusedVarAssignedOnly { #[multipart_suggestion( passes_unused_var_typo, style = "verbose", - applicability = "machine-applicable" + applicability = "maybe-incorrect" )] pub(crate) struct PatternTypo { #[suggestion_part(code = "{code}")] From fd96a78092e77be45d5b60cedea0806b70d751fd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 3 Oct 2025 12:14:41 -0700 Subject: [PATCH 02/16] Add documentation about unwinding to wasm targets This commit adds some documentation about the state of `-Cpanic=unwind` for the following wasm targets: * `wasm32-unknown-unknown` * `wasm32-wasip1` * `wasm32-wasip2` * `wasm32v1-none` Notably it's possible to use `-Cpanic=unwind` with `-Zbuild-std` and it's also mentioned that there are no concrete proposals at this time to adding a new set of targets which support unwinding. My hunch is that in a few years' time it would make sense to enable it by default on these targets (except for `wasm32v1-none`) but that's a problem for future folks to debate. For now this is an attempt to document the status quo. --- .../wasm32-unknown-unknown.md | 62 ++++++++++++++++++- .../src/platform-support/wasm32-wasip1.md | 6 ++ .../src/platform-support/wasm32-wasip2.md | 6 ++ .../src/platform-support/wasm32v1-none.md | 11 ++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md b/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md index 1e74c47221c78..ec20672f65403 100644 --- a/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md +++ b/src/doc/rustc/src/platform-support/wasm32-unknown-unknown.md @@ -157,7 +157,7 @@ $ cargo +nightly build -Zbuild-std=panic_abort,std --target wasm32-unknown-unkno ``` Here the `mvp` "cpu" is a placeholder in LLVM for disabling all supported -features by default. Cargo's `-Zbuild-std` feature, a Nightly Rust feature, is +features by default. Cargo's [`-Zbuild-std`] feature, a Nightly Rust feature, is then used to recompile the standard library in addition to your own code. This will produce a binary that uses only the original WebAssembly features by default and no proposals since its inception. @@ -207,3 +207,63 @@ conditionally compile code instead. This is notably different to the way native platforms such as x86\_64 work, and this is due to the fact that WebAssembly binaries must only contain code the engine understands. Native binaries work so long as the CPU doesn't execute unknown code dynamically at runtime. + +## Unwinding + +By default the `wasm32-unknown-unknown` target is compiled with `-Cpanic=abort`. +Historically this was due to the fact that there was no way to catch panics in +wasm, but since mid-2025 the WebAssembly [`exception-handling` +proposal](https://github.com/WebAssembly/exception-handling) reached +stabilization. LLVM has support for this proposal as well and when this is all +combined together it's possible to enable `-Cpanic=unwind` on wasm targets. + +Compiling wasm targets with `-Cpanic=unwind` is not as easy as just passing +`-Cpanic=unwind`, however: + +```sh +$ rustc foo.rs -Cpanic=unwind --target wasm32-unknown-unknown +error: the crate `panic_unwind` does not have the panic strategy `unwind` +``` + +Notably the precompiled standard library that is shipped through Rustup is +compiled with `-Cpanic=abort`, not `-Cpanic=unwind`. While this is the case +you're going to be required to use Cargo's [`-Zbuild-std`] feature to build with +unwinding support: + +```sh +$ RUSTFLAGS='-Cpanic=unwind' cargo +nightly build --target wasm32-unknown-unknown -Zbuild-std +``` + +Note, however, that as of 2025-10-03 LLVM is still using the "legacy exception +instructions" by default, not the officially standard version of the +exception-handling proposal: + +```sh +$ wasm-tools validate target/wasm32-unknown-unknown/debug/foo.wasm +error: /library/std/src/sys/backtrace.rs:161:5 +function `std::sys::backtrace::__rust_begin_short_backtrace` failed to validate + +Caused by: + 0: func 2 failed to validate + 1: legacy_exceptions feature required for try instruction (at offset 0x880) +``` + +Fixing this requires passing `-Cllvm-args=-wasm-use-legacy-eh=false` to the Rust +compiler as well: + +```sh +$ RUSTFLAGS='-Cpanic=unwind -Cllvm-args=-wasm-use-legacy-eh=false' cargo +nightly build --target wasm32-unknown-unknown -Zbuild-std +$ wasm-tools validate target/wasm32-unknown-unknown/debug/foo.wasm +``` + +At this time there are no concrete plans for adding new targets to the Rust +compiler which have `-Cpanic=unwind` enabled-by-default. The most likely route +to having this enabled is that in a few years when the `exception-handling` +target feature is enabled by default in LLVM (due to browsers/runtime support +propagating widely enough) the targets will switch to using `-Cpanic=unwind` by +default. This is not for certain, however, and will likely be accompanied with +either an MCP or an RFC about changing all wasm targets in the same manner. In +the meantime using `-Cpanic=unwind` will require using [`-Zbuild-std`] and +passing the appropriate flags to rustc. + +[`-Zbuild-std`]: ../../cargo/reference/unstable.html#build-std diff --git a/src/doc/rustc/src/platform-support/wasm32-wasip1.md b/src/doc/rustc/src/platform-support/wasm32-wasip1.md index a8a9e5505810b..958a34a86928c 100644 --- a/src/doc/rustc/src/platform-support/wasm32-wasip1.md +++ b/src/doc/rustc/src/platform-support/wasm32-wasip1.md @@ -133,3 +133,9 @@ to Rust 1.80 the `target_env` condition was not set. The default set of WebAssembly features enabled for compilation is currently the same as [`wasm32-unknown-unknown`](./wasm32-unknown-unknown.md). See the documentation there for more information. + +## Unwinding + +This target is compiled with `-Cpanic=abort` by default. For information on +using `-Cpanic=unwind` see the [documentation about unwinding for +`wasm32-unknown-unknown`](./wasm32-unknown-unknown.md#unwinding). diff --git a/src/doc/rustc/src/platform-support/wasm32-wasip2.md b/src/doc/rustc/src/platform-support/wasm32-wasip2.md index dea33e62f2b43..861083ad4a61a 100644 --- a/src/doc/rustc/src/platform-support/wasm32-wasip2.md +++ b/src/doc/rustc/src/platform-support/wasm32-wasip2.md @@ -67,3 +67,9 @@ It's recommended to conditionally compile code for this target with: The default set of WebAssembly features enabled for compilation is currently the same as [`wasm32-unknown-unknown`](./wasm32-unknown-unknown.md). See the documentation there for more information. + +## Unwinding + +This target is compiled with `-Cpanic=abort` by default. For information on +using `-Cpanic=unwind` see the [documentation about unwinding for +`wasm32-unknown-unknown`](./wasm32-unknown-unknown.md#unwinding). diff --git a/src/doc/rustc/src/platform-support/wasm32v1-none.md b/src/doc/rustc/src/platform-support/wasm32v1-none.md index 51b00de5e578a..d2f9b3a1f9760 100644 --- a/src/doc/rustc/src/platform-support/wasm32v1-none.md +++ b/src/doc/rustc/src/platform-support/wasm32v1-none.md @@ -107,3 +107,14 @@ $ cargo +nightly build -Zbuild-std=panic_abort,std --target wasm32-unknown-unkno Which not only rebuilds `std`, `core` and `alloc` (which is somewhat costly and annoying) but more importantly requires the use of nightly Rust toolchains (for the `-Zbuild-std` flag). This is very undesirable for the target audience, which consists of people targeting WebAssembly implementations that prioritize stability, simplicity and/or security over feature support. This `wasm32v1-none` target exists as an alternative option that works on stable Rust toolchains, without rebuilding the stdlib. + +## Unwinding + +This target is compiled with `-Cpanic=abort` by default. Using `-Cpanic=unwind` +would require using the WebAssembly exception-handling proposal stabilized +mid-2025, and if that's desired then you most likely don't want to use this +target and instead want to use `wasm32-unknown-unknown` instead. It's unlikely +that this target will ever support unwinding with the precompiled artifacts +shipped through rustup. For documentation about using `-Zbuild-std` to enable +using `-Cpanic=unwind` see the [documentation of +`wasm32-unknown-unknown`](./wasm32-unknown-unknown.md#unwinding). From e9a45e66461f3c324a249d7352eb2f4d184a6b2a Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 4 Oct 2025 14:48:55 +0800 Subject: [PATCH 03/16] Avoid to suggest pattern match on the similarly named in fn signature --- compiler/rustc_passes/src/liveness.rs | 7 +++++-- .../ui/fn/invalid-sugg-for-unused-fn-arg-147303.rs | 13 +++++++++++++ .../invalid-sugg-for-unused-fn-arg-147303.stderr | 14 ++++++++++++++ .../non-snake-case/lint-uppercase-variables.stderr | 11 +---------- 4 files changed, 33 insertions(+), 12 deletions(-) create mode 100644 tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.rs create mode 100644 tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.stderr diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 1b2ffb5b3db27..b628ff8a12dfd 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -1691,7 +1691,10 @@ impl<'tcx> Liveness<'_, 'tcx> { if ln == self.exit_ln { false } else { self.assigned_on_exit(ln, var) }; let mut typo = None; - for (hir_id, _, span) in &hir_ids_and_spans { + let filtered_hir_ids_and_spans = hir_ids_and_spans.iter().filter(|(hir_id, ..)| { + !matches!(self.ir.tcx.parent_hir_node(*hir_id), hir::Node::Param(_)) + }); + for (hir_id, _, span) in filtered_hir_ids_and_spans.clone() { let ty = self.typeck_results.node_type(*hir_id); if let ty::Adt(adt, _) = ty.peel_refs().kind() { let name = Symbol::intern(&name); @@ -1717,7 +1720,7 @@ impl<'tcx> Liveness<'_, 'tcx> { } } if typo.is_none() { - for (hir_id, _, span) in &hir_ids_and_spans { + for (hir_id, _, span) in filtered_hir_ids_and_spans { let ty = self.typeck_results.node_type(*hir_id); // Look for consts of the same type with similar names as well, not just unit // structs and variants. diff --git a/tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.rs b/tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.rs new file mode 100644 index 0000000000000..137ba8dc103e4 --- /dev/null +++ b/tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.rs @@ -0,0 +1,13 @@ +// Regression test for . + +#![deny(unused_assignments, unused_variables)] + +mod m1 { + const _MAX_FMTVER_X1X_EVENTNUM: i32 = 0; +} + +mod m2 { + fn fun(rough: i32) {} //~ERROR unused variable +} + +fn main() {} diff --git a/tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.stderr b/tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.stderr new file mode 100644 index 0000000000000..30e4a758c13ce --- /dev/null +++ b/tests/ui/fn/invalid-sugg-for-unused-fn-arg-147303.stderr @@ -0,0 +1,14 @@ +error: unused variable: `rough` + --> $DIR/invalid-sugg-for-unused-fn-arg-147303.rs:10:12 + | +LL | fn fun(rough: i32) {} + | ^^^^^ help: if this is intentional, prefix it with an underscore: `_rough` + | +note: the lint level is defined here + --> $DIR/invalid-sugg-for-unused-fn-arg-147303.rs:3:29 + | +LL | #![deny(unused_assignments, unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/lint/non-snake-case/lint-uppercase-variables.stderr b/tests/ui/lint/non-snake-case/lint-uppercase-variables.stderr index e5f2e65fd91d3..5dec20b2ac7ea 100644 --- a/tests/ui/lint/non-snake-case/lint-uppercase-variables.stderr +++ b/tests/ui/lint/non-snake-case/lint-uppercase-variables.stderr @@ -58,16 +58,7 @@ warning: unused variable: `Foo` --> $DIR/lint-uppercase-variables.rs:33:17 | LL | fn in_param(Foo: foo::Foo) {} - | ^^^ - | -help: if this is intentional, prefix it with an underscore - | -LL | fn in_param(_Foo: foo::Foo) {} - | + -help: you might have meant to pattern match on the similarly named variant `Foo` - | -LL | fn in_param(foo::Foo::Foo: foo::Foo) {} - | ++++++++++ + | ^^^ help: if this is intentional, prefix it with an underscore: `_Foo` error: structure field `X` should have a snake case name --> $DIR/lint-uppercase-variables.rs:10:5 From ba42380142e73765eab501b3ebc2b3e05ee6b0c0 Mon Sep 17 00:00:00 2001 From: EFanZh Date: Sat, 4 Oct 2025 17:16:00 +0800 Subject: [PATCH 04/16] Implement non-poisoning `Mutex::with_mut`, `RwLock::with` and `RwLock::with_mut` ACP: https://github.com/rust-lang/libs-team/issues/497. --- library/std/src/sync/nonpoison/mutex.rs | 34 +++++++++++++ library/std/src/sync/nonpoison/rwlock.rs | 62 ++++++++++++++++++++++++ library/std/tests/sync/mutex.rs | 14 ++++++ library/std/tests/sync/rwlock.rs | 22 +++++++++ 4 files changed, 132 insertions(+) diff --git a/library/std/src/sync/nonpoison/mutex.rs b/library/std/src/sync/nonpoison/mutex.rs index eeecf5d710767..ed3f8cfed821a 100644 --- a/library/std/src/sync/nonpoison/mutex.rs +++ b/library/std/src/sync/nonpoison/mutex.rs @@ -376,6 +376,40 @@ impl Mutex { pub const fn data_ptr(&self) -> *mut T { self.data.get() } + + /// Acquires the mutex and provides mutable access to the underlying data by passing + /// a mutable reference to the given closure. + /// + /// This method acquires the lock, calls the provided closure with a mutable reference + /// to the data, and returns the result of the closure. The lock is released after + /// the closure completes, even if it panics. + /// + /// # Examples + /// + /// ``` + /// #![feature(lock_value_accessors, nonpoison_mutex)] + /// + /// use std::sync::nonpoison::Mutex; + /// + /// let mutex = Mutex::new(2); + /// + /// let result = mutex.with_mut(|data| { + /// *data += 3; + /// + /// *data + 5 + /// }); + /// + /// assert_eq!(*mutex.lock(), 5); + /// assert_eq!(result, 10); + /// ``` + #[unstable(feature = "lock_value_accessors", issue = "133407")] + // #[unstable(feature = "nonpoison_mutex", issue = "134645")] + pub fn with_mut(&self, f: F) -> R + where + F: FnOnce(&mut T) -> R, + { + f(&mut self.lock()) + } } #[unstable(feature = "nonpoison_mutex", issue = "134645")] diff --git a/library/std/src/sync/nonpoison/rwlock.rs b/library/std/src/sync/nonpoison/rwlock.rs index b2f26edc083e9..568c7f3868470 100644 --- a/library/std/src/sync/nonpoison/rwlock.rs +++ b/library/std/src/sync/nonpoison/rwlock.rs @@ -498,6 +498,68 @@ impl RwLock { pub const fn data_ptr(&self) -> *mut T { self.data.get() } + + /// Locks this `RwLock` with shared read access to the underlying data by passing + /// a reference to the given closure. + /// + /// This method acquires the lock, calls the provided closure with a reference + /// to the data, and returns the result of the closure. The lock is released after + /// the closure completes, even if it panics. + /// + /// # Examples + /// + /// ``` + /// #![feature(lock_value_accessors, nonpoison_rwlock)] + /// + /// use std::sync::nonpoison::RwLock; + /// + /// let rwlock = RwLock::new(2); + /// let result = rwlock.with(|data| *data + 3); + /// + /// assert_eq!(result, 5); + /// ``` + #[unstable(feature = "lock_value_accessors", issue = "133407")] + // #[unstable(feature = "nonpoison_rwlock", issue = "134645")] + pub fn with(&self, f: F) -> R + where + F: FnOnce(&T) -> R, + { + f(&self.read()) + } + + /// Locks this `RwLock` with exclusive write access to the underlying data by passing + /// a mutable reference to the given closure. + /// + /// This method acquires the lock, calls the provided closure with a mutable reference + /// to the data, and returns the result of the closure. The lock is released after + /// the closure completes, even if it panics. + /// + /// # Examples + /// + /// ``` + /// #![feature(lock_value_accessors, nonpoison_rwlock)] + /// + /// use std::sync::nonpoison::RwLock; + /// + /// let rwlock = RwLock::new(2); + /// + /// let result = rwlock.with_mut(|data| { + /// *data += 3; + /// + /// *data + 5 + /// }); + /// + /// assert_eq!(*rwlock.read(), 5); + /// assert_eq!(result, 10); + /// ``` + #[unstable(feature = "lock_value_accessors", issue = "133407")] + // #[unstable(feature = "nonpoison_rwlock", issue = "134645")] + pub fn with_mut(&self, f: F) -> R + where + F: FnOnce(&mut T) -> R, + { + f(&mut self.write()) + } } #[unstable(feature = "nonpoison_rwlock", issue = "134645")] diff --git a/library/std/tests/sync/mutex.rs b/library/std/tests/sync/mutex.rs index 2445764001b8b..ff6aef717936f 100644 --- a/library/std/tests/sync/mutex.rs +++ b/library/std/tests/sync/mutex.rs @@ -549,3 +549,17 @@ fn panic_while_mapping_unlocked_poison() { drop(lock); } + +#[test] +fn test_mutex_with_mut() { + let mutex = std::sync::nonpoison::Mutex::new(2); + + let result = mutex.with_mut(|value| { + *value += 3; + + *value + 5 + }); + + assert_eq!(*mutex.lock(), 5); + assert_eq!(result, 10); +} diff --git a/library/std/tests/sync/rwlock.rs b/library/std/tests/sync/rwlock.rs index 65d8bac719456..392c45c8ba05d 100644 --- a/library/std/tests/sync/rwlock.rs +++ b/library/std/tests/sync/rwlock.rs @@ -861,3 +861,25 @@ fn panic_while_mapping_write_unlocked_poison() { drop(lock); } + +#[test] +fn test_rwlock_with() { + let rwlock = std::sync::nonpoison::RwLock::new(2); + let result = rwlock.with(|value| *value + 3); + + assert_eq!(result, 5); +} + +#[test] +fn test_rwlock_with_mut() { + let rwlock = std::sync::nonpoison::RwLock::new(2); + + let result = rwlock.with_mut(|value| { + *value += 3; + + *value + 5 + }); + + assert_eq!(*rwlock.read(), 5); + assert_eq!(result, 10); +} From 07d41a7cd7f3ef8ca1e9b70768975695d3536272 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 13 Jul 2025 20:38:12 +0200 Subject: [PATCH 05/16] Correctly handle `--no-run` rustdoc test option --- src/librustdoc/doctest.rs | 2 +- src/librustdoc/doctest/runner.rs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 9499258f983a1..a1e917df75ab9 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -358,7 +358,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, diff --git a/src/librustdoc/doctest/runner.rs b/src/librustdoc/doctest/runner.rs index fcfa424968e48..5493d56456872 100644 --- a/src/librustdoc/doctest/runner.rs +++ b/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; @@ -223,6 +225,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,7 +259,7 @@ fn main() {returns_result} {{ ) .unwrap(); } - let not_running = ignore || scraped_test.langstr.no_run; + let not_running = ignore || scraped_test.no_run(opts); writeln!( output_merged_tests, " @@ -270,7 +273,7 @@ test::StaticTestFn( test_name = scraped_test.name, file = scraped_test.path(), line = scraped_test.line, - no_run = scraped_test.langstr.no_run, + no_run = scraped_test.no_run(opts), 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. From 796c4efe44a57ddbaf071937e26f1279e8595302 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 7 Jul 2025 11:53:18 +0200 Subject: [PATCH 06/16] Correctly handle `should_panic` doctest attribute --- src/librustdoc/doctest.rs | 4 ++-- src/librustdoc/doctest/runner.rs | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index a1e917df75ab9..df457536b70e8 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -836,7 +836,7 @@ fn run_test( match result { Err(e) => return (duration, Err(TestFailure::ExecutionError(e))), Ok(out) => { - if langstr.should_panic && out.status.success() { + if langstr.should_panic && out.status.code() != Some(101) { return (duration, Err(TestFailure::UnexpectedRunPass)); } else if !langstr.should_panic && !out.status.success() { return (duration, Err(TestFailure::ExecutionFailure(out))); @@ -1146,7 +1146,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:?}"); diff --git a/src/librustdoc/doctest/runner.rs b/src/librustdoc/doctest/runner.rs index 5493d56456872..d02b32faa319e 100644 --- a/src/librustdoc/doctest/runner.rs +++ b/src/librustdoc/doctest/runner.rs @@ -136,13 +136,20 @@ 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::>()) .output() .expect(\"failed to run command\"); - if !out.status.success() {{ + if should_panic {{ + if out.status.code() != Some(101) {{ + eprintln!(\"Test didn't panic, but it's marked `should_panic`.\"); + ExitCode::FAILURE + }} else {{ + ExitCode::SUCCESS + }} + }} else if !out.status.success() {{ if let Some(code) = out.status.code() {{ eprintln!(\"Test executable failed (exit status: {{code}}).\"); }} else {{ @@ -265,7 +272,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}}}, )); @@ -274,7 +281,6 @@ test::StaticTestFn( file = scraped_test.path(), line = scraped_test.line, no_run = scraped_test.no_run(opts), - 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 { @@ -283,11 +289,12 @@ 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()) }} ", + should_panic = scraped_test.langstr.should_panic, ) }, ) From 11b7070577e52c1d26b35021b8a07475e794e93c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 4 Jul 2025 21:42:19 +0200 Subject: [PATCH 07/16] Add regression test for #143009 --- tests/run-make/rustdoc-should-panic/rmake.rs | 36 ++++++++++++++++++++ tests/run-make/rustdoc-should-panic/test.rs | 14 ++++++++ 2 files changed, 50 insertions(+) create mode 100644 tests/run-make/rustdoc-should-panic/rmake.rs create mode 100644 tests/run-make/rustdoc-should-panic/test.rs diff --git a/tests/run-make/rustdoc-should-panic/rmake.rs b/tests/run-make/rustdoc-should-panic/rmake.rs new file mode 100644 index 0000000000000..100a62f5db313 --- /dev/null +++ b/tests/run-make/rustdoc-should-panic/rmake.rs @@ -0,0 +1,36 @@ +// Ensure that `should_panic` doctests only succeed if the test actually panicked. +// Regression test for . + +//@ needs-target-std + +use run_make_support::rustdoc; + +fn check_output(output: String, edition: &str) { + 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 doesn't contains (edition: {edition}) {:?}\nfull output: {output}", + text + ); + } +} + +fn main() { + check_output(rustdoc().input("test.rs").arg("--test").run_fail().stdout_utf8(), "2015"); + + // Same check with the merged doctest feature (enabled with the 2024 edition). + check_output( + rustdoc().input("test.rs").arg("--test").edition("2024").run_fail().stdout_utf8(), + "2024", + ); +} diff --git a/tests/run-make/rustdoc-should-panic/test.rs b/tests/run-make/rustdoc-should-panic/test.rs new file mode 100644 index 0000000000000..1eea8e1e1958c --- /dev/null +++ b/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() {} From 21a4d9dda7b2234d4454c2413956f96d8884cd65 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 7 Jul 2025 13:35:02 +0200 Subject: [PATCH 08/16] Update std doctests --- library/std/src/error.rs | 16 ++++++++++++---- .../failed-doctest-should-panic-2021.stdout | 2 +- .../doctest/failed-doctest-should-panic.stdout | 5 +++-- tests/rustdoc-ui/doctest/wrong-ast-2024.stdout | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/library/std/src/error.rs b/library/std/src/error.rs index def5f984c88e4..09bfc83ebca6c 100644 --- a/library/std/src/error.rs +++ b/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> { +/// fn run() -> Result<(), Report> { /// 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> { +/// fn run() -> Result<(), Report> { /// 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: diff --git a/tests/rustdoc-ui/doctest/failed-doctest-should-panic-2021.stdout b/tests/rustdoc-ui/doctest/failed-doctest-should-panic-2021.stdout index 9f4d60e6f4de5..f8413756e3d6d 100644 --- a/tests/rustdoc-ui/doctest/failed-doctest-should-panic-2021.stdout +++ b/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) diff --git a/tests/rustdoc-ui/doctest/failed-doctest-should-panic.stdout b/tests/rustdoc-ui/doctest/failed-doctest-should-panic.stdout index 9047fe0dcdd93..61099e6424ae7 100644 --- a/tests/rustdoc-ui/doctest/failed-doctest-should-panic.stdout +++ b/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) diff --git a/tests/rustdoc-ui/doctest/wrong-ast-2024.stdout b/tests/rustdoc-ui/doctest/wrong-ast-2024.stdout index 13567b41e51f5..27f9a0157a6cc 100644 --- a/tests/rustdoc-ui/doctest/wrong-ast-2024.stdout +++ b/tests/rustdoc-ui/doctest/wrong-ast-2024.stdout @@ -1,6 +1,6 @@ running 1 test -test $DIR/wrong-ast-2024.rs - three (line 20) - should panic ... ok +test $DIR/wrong-ast-2024.rs - three (line 20) ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME From 69833f1d5faefc685c38266f68020f97727cdddd Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 13 Jul 2025 20:49:29 +0200 Subject: [PATCH 09/16] Add regression test for #143858 --- .../doctest/no-run.edition2021.stdout | 12 +++++ .../doctest/no-run.edition2024.stdout | 18 ++++++++ tests/rustdoc-ui/doctest/no-run.rs | 44 +++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 tests/rustdoc-ui/doctest/no-run.edition2021.stdout create mode 100644 tests/rustdoc-ui/doctest/no-run.edition2024.stdout create mode 100644 tests/rustdoc-ui/doctest/no-run.rs diff --git a/tests/rustdoc-ui/doctest/no-run.edition2021.stdout b/tests/rustdoc-ui/doctest/no-run.edition2021.stdout new file mode 100644 index 0000000000000..937cd76bfb462 --- /dev/null +++ b/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 + diff --git a/tests/rustdoc-ui/doctest/no-run.edition2024.stdout b/tests/rustdoc-ui/doctest/no-run.edition2024.stdout new file mode 100644 index 0000000000000..921e059979b1f --- /dev/null +++ b/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 diff --git a/tests/rustdoc-ui/doctest/no-run.rs b/tests/rustdoc-ui/doctest/no-run.rs new file mode 100644 index 0000000000000..78198badd43b5 --- /dev/null +++ b/tests/rustdoc-ui/doctest/no-run.rs @@ -0,0 +1,44 @@ +// This test ensures that the `--no-run` flag works the same between normal and merged doctests. +// Regression test for . + +//@ check-pass +//@ revisions: edition2021 edition2024 +//@ [edition2021]edition:2021 +//@ [edition2024]edition:2024 +//@ compile-flags:-Z unstable-options --test --no-run --test-args=--test-threads=1 +//@ 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" + +/// ``` +/// let a = true; +/// ``` +/// ```should_panic +/// panic!() +/// ``` +/// ```ignore (incomplete-code) +/// fn foo() { +/// ``` +/// ```no_run +/// loop { +/// println!("Hello, world"); +/// } +/// ``` +/// fails to compile +/// ```compile_fail +/// let x = 5; +/// x += 2; // shouldn't compile! +/// ``` +/// Ok the test does not run +/// ``` +/// panic!() +/// ``` +/// Ok the test does not run +/// ```should_panic +/// loop { +/// println!("Hello, world"); +/// panic!() +/// } +/// ``` +pub fn f() {} From 6e61a1cb81d78e821c36a5e6e5aeb4569b15b363 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 1 Aug 2025 11:28:17 +0200 Subject: [PATCH 10/16] Add FIXME comments to use `test::ERROR_EXIT_CODE` once public and fix typo --- src/librustdoc/doctest.rs | 1 + src/librustdoc/doctest/runner.rs | 1 + tests/run-make/rustdoc-should-panic/rmake.rs | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index df457536b70e8..51972f8b149c7 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -836,6 +836,7 @@ fn run_test( match result { Err(e) => return (duration, Err(TestFailure::ExecutionError(e))), Ok(out) => { + // FIXME: use test::ERROR_EXIT_CODE once public if langstr.should_panic && out.status.code() != Some(101) { return (duration, Err(TestFailure::UnexpectedRunPass)); } else if !langstr.should_panic && !out.status.success() { diff --git a/src/librustdoc/doctest/runner.rs b/src/librustdoc/doctest/runner.rs index d02b32faa319e..d241d44441d53 100644 --- a/src/librustdoc/doctest/runner.rs +++ b/src/librustdoc/doctest/runner.rs @@ -143,6 +143,7 @@ mod __doctest_mod {{ .output() .expect(\"failed to run command\"); if should_panic {{ + // FIXME: use test::ERROR_EXIT_CODE once public if out.status.code() != Some(101) {{ eprintln!(\"Test didn't panic, but it's marked `should_panic`.\"); ExitCode::FAILURE diff --git a/tests/run-make/rustdoc-should-panic/rmake.rs b/tests/run-make/rustdoc-should-panic/rmake.rs index 100a62f5db313..0c94e9b69888a 100644 --- a/tests/run-make/rustdoc-should-panic/rmake.rs +++ b/tests/run-make/rustdoc-should-panic/rmake.rs @@ -19,7 +19,7 @@ Test didn't panic, but it's marked `should_panic`.", for text in should_contain { assert!( output.contains(text), - "output doesn't contains (edition: {edition}) {:?}\nfull output: {output}", + "output (edition: {edition}) doesn't contain {:?}\nfull output: {output}", text ); } From ef56675360f5bee989f6738990b4d84193a79845 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 4 Oct 2025 13:39:31 +0200 Subject: [PATCH 11/16] Use libtest `ERROR_EXIT_CODE` constant --- src/librustdoc/doctest.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 51972f8b149c7..0eb698ca544fe 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -836,8 +836,7 @@ fn run_test( match result { Err(e) => return (duration, Err(TestFailure::ExecutionError(e))), Ok(out) => { - // FIXME: use test::ERROR_EXIT_CODE once public - if langstr.should_panic && out.status.code() != Some(101) { + if langstr.should_panic && out.status.code() != Some(test::ERROR_EXIT_CODE) { return (duration, Err(TestFailure::UnexpectedRunPass)); } else if !langstr.should_panic && !out.status.success() { return (duration, Err(TestFailure::ExecutionFailure(out))); From 2688f601ddc29b75caa2c48a4badc00d15042d7a Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 4 Oct 2025 16:07:06 +0200 Subject: [PATCH 12/16] Make `fmt::Write` a diagnostic item --- compiler/rustc_span/src/symbol.rs | 1 + library/core/src/fmt/mod.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index b34a64108e3b4..083e04730bc37 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -236,6 +236,7 @@ symbols! { File, FileType, FmtArgumentsNew, + FmtWrite, Fn, FnMut, FnOnce, diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index fcd2e52101ff0..0f255e57fe585 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -115,6 +115,7 @@ pub struct Error; /// [`std::io::Write`]: ../../std/io/trait.Write.html /// [flushable]: ../../std/io/trait.Write.html#tymethod.flush #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "FmtWrite"] pub trait Write { /// Writes a string slice into this writer, returning whether the write /// succeeded. From b5fb01d67db9726180fd85b58d00e3b954fc7e45 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 4 Oct 2025 15:53:00 -0400 Subject: [PATCH 13/16] Improve the advice given by panic_immediate_abort --- library/core/src/panicking.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 3f30038dbc03e..b5150837e6a94 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -35,7 +35,8 @@ use crate::panic::{Location, PanicInfo}; #[cfg(feature = "panic_immediate_abort")] compile_error!( "panic_immediate_abort is now a real panic strategy! \ - Enable it with the compiler flags `-Zunstable-options -Cpanic=immediate-abort`" + Enable it with `panic = \"immediate-abort\"` in Cargo.toml, \ + or with the compiler flags `-Zunstable-options -Cpanic=immediate-abort`" ); // First we define the two main entry points that all panics go through. From 757d98ce2b55162e22051be354418291e88f8d46 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 3 Oct 2025 13:20:39 +1000 Subject: [PATCH 14/16] Move `DirectiveLine` into its own submodule --- src/tools/compiletest/src/directives.rs | 68 +------------------- src/tools/compiletest/src/directives/line.rs | 67 +++++++++++++++++++ 2 files changed, 69 insertions(+), 66 deletions(-) create mode 100644 src/tools/compiletest/src/directives/line.rs diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index a79978d036cea..edb06dd7345d9 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -15,6 +15,7 @@ use crate::directives::auxiliary::{AuxProps, parse_and_update_aux}; use crate::directives::directive_names::{ KNOWN_DIRECTIVE_NAMES, KNOWN_HTMLDOCCK_DIRECTIVE_NAMES, KNOWN_JSONDOCCK_DIRECTIVE_NAMES, }; +use crate::directives::line::{DirectiveLine, line_directive}; use crate::directives::needs::CachedNeedsConditions; use crate::edition::{Edition, parse_edition}; use crate::errors::ErrorKind; @@ -25,6 +26,7 @@ use crate::{fatal, help}; pub(crate) mod auxiliary; mod cfg; mod directive_names; +mod line; mod needs; #[cfg(test)] mod tests; @@ -824,70 +826,6 @@ impl TestProps { } } -/// If the given line begins with the appropriate comment prefix for a directive, -/// returns a struct containing various parts of the directive. -fn line_directive<'line>( - line_number: usize, - original_line: &'line str, -) -> Option> { - // Ignore lines that don't start with the comment prefix. - let after_comment = - original_line.trim_start().strip_prefix(COMPILETEST_DIRECTIVE_PREFIX)?.trim_start(); - - let revision; - let raw_directive; - - if let Some(after_open_bracket) = after_comment.strip_prefix('[') { - // A comment like `//@[foo]` only applies to revision `foo`. - let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else { - panic!( - "malformed condition directive: expected `{COMPILETEST_DIRECTIVE_PREFIX}[foo]`, found `{original_line}`" - ) - }; - - revision = Some(line_revision); - raw_directive = after_close_bracket.trim_start(); - } else { - revision = None; - raw_directive = after_comment; - }; - - Some(DirectiveLine { line_number, revision, raw_directive }) -} - -/// The (partly) broken-down contents of a line containing a test directive, -/// which [`iter_directives`] passes to its callback function. -/// -/// For example: -/// -/// ```text -/// //@ compile-flags: -O -/// ^^^^^^^^^^^^^^^^^ raw_directive -/// -/// //@ [foo] compile-flags: -O -/// ^^^ revision -/// ^^^^^^^^^^^^^^^^^ raw_directive -/// ``` -struct DirectiveLine<'ln> { - line_number: usize, - /// Some test directives start with a revision name in square brackets - /// (e.g. `[foo]`), and only apply to that revision of the test. - /// If present, this field contains the revision name (e.g. `foo`). - revision: Option<&'ln str>, - /// The main part of the directive, after removing the comment prefix - /// and the optional revision specifier. - /// - /// This is "raw" because the directive's name and colon-separated value - /// (if present) have not yet been extracted or checked. - raw_directive: &'ln str, -} - -impl<'ln> DirectiveLine<'ln> { - fn applies_to_test_revision(&self, test_revision: Option<&str>) -> bool { - self.revision.is_none() || self.revision == test_revision - } -} - pub(crate) struct CheckDirectiveResult<'ln> { is_known_directive: bool, trailing_directive: Option<&'ln str>, @@ -920,8 +858,6 @@ fn check_directive<'a>( CheckDirectiveResult { is_known_directive, trailing_directive } } -const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; - fn iter_directives( mode: TestMode, poisoned: &mut bool, diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs new file mode 100644 index 0000000000000..584d5b5b35bec --- /dev/null +++ b/src/tools/compiletest/src/directives/line.rs @@ -0,0 +1,67 @@ +const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; + +/// If the given line begins with the appropriate comment prefix for a directive, +/// returns a struct containing various parts of the directive. +pub(crate) fn line_directive<'line>( + line_number: usize, + original_line: &'line str, +) -> Option> { + // Ignore lines that don't start with the comment prefix. + let after_comment = + original_line.trim_start().strip_prefix(COMPILETEST_DIRECTIVE_PREFIX)?.trim_start(); + + let revision; + let raw_directive; + + if let Some(after_open_bracket) = after_comment.strip_prefix('[') { + // A comment like `//@[foo]` only applies to revision `foo`. + let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else { + panic!( + "malformed condition directive: expected `{COMPILETEST_DIRECTIVE_PREFIX}[foo]`, found `{original_line}`" + ) + }; + + revision = Some(line_revision); + raw_directive = after_close_bracket.trim_start(); + } else { + revision = None; + raw_directive = after_comment; + }; + + Some(DirectiveLine { line_number, revision, raw_directive }) +} + +/// The (partly) broken-down contents of a line containing a test directive, +/// which `iter_directives` passes to its callback function. +/// +/// For example: +/// +/// ```text +/// //@ compile-flags: -O +/// ^^^^^^^^^^^^^^^^^ raw_directive +/// +/// //@ [foo] compile-flags: -O +/// ^^^ revision +/// ^^^^^^^^^^^^^^^^^ raw_directive +/// ``` +pub(crate) struct DirectiveLine<'ln> { + pub(crate) line_number: usize, + + /// Some test directives start with a revision name in square brackets + /// (e.g. `[foo]`), and only apply to that revision of the test. + /// If present, this field contains the revision name (e.g. `foo`). + pub(crate) revision: Option<&'ln str>, + + /// The main part of the directive, after removing the comment prefix + /// and the optional revision specifier. + /// + /// This is "raw" because the directive's name and colon-separated value + /// (if present) have not yet been extracted or checked. + pub(crate) raw_directive: &'ln str, +} + +impl<'ln> DirectiveLine<'ln> { + pub(crate) fn applies_to_test_revision(&self, test_revision: Option<&str>) -> bool { + self.revision.is_none() || self.revision == test_revision + } +} From 6783e9465b62e77c7e3cb76a747d92b1de4339f7 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 2 Oct 2025 20:08:40 +1000 Subject: [PATCH 15/16] Allow easy extraction of name/value from a `DirectiveLine` --- src/tools/compiletest/src/directives.rs | 10 +++-- src/tools/compiletest/src/directives/line.rs | 45 +++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index edb06dd7345d9..bbbeecc1b71b0 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -875,15 +875,17 @@ fn iter_directives( // FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later. if mode == TestMode::CoverageRun { let extra_directives: &[&str] = &[ - "needs-profiler-runtime", + "//@ needs-profiler-runtime", // FIXME(pietroalbini): this test currently does not work on cross-compiled targets // because remote-test is not capable of sending back the *.profraw files generated by // the LLVM instrumentation. - "ignore-cross-compile", + "//@ ignore-cross-compile", ]; // Process the extra implied directives, with a dummy line number of 0. - for raw_directive in extra_directives { - it(DirectiveLine { line_number: 0, revision: None, raw_directive }); + for directive_str in extra_directives { + let directive_line = line_directive(0, directive_str) + .unwrap_or_else(|| panic!("bad extra-directive line: {directive_str:?}")); + it(directive_line); } } diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 584d5b5b35bec..5c1b592d045d8 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -1,3 +1,7 @@ +#![expect(dead_code)] // (removed later in this PR) + +use std::fmt; + const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; /// If the given line begins with the appropriate comment prefix for a directive, @@ -28,7 +32,10 @@ pub(crate) fn line_directive<'line>( raw_directive = after_comment; }; - Some(DirectiveLine { line_number, revision, raw_directive }) + // The directive name ends at the first occurrence of colon, space, or end-of-string. + let name = raw_directive.split([':', ' ']).next().expect("split is never empty"); + + Some(DirectiveLine { line_number, revision, raw_directive, name }) } /// The (partly) broken-down contents of a line containing a test directive, @@ -39,10 +46,12 @@ pub(crate) fn line_directive<'line>( /// ```text /// //@ compile-flags: -O /// ^^^^^^^^^^^^^^^^^ raw_directive +/// ^^^^^^^^^^^^^ name /// /// //@ [foo] compile-flags: -O /// ^^^ revision /// ^^^^^^^^^^^^^^^^^ raw_directive +/// ^^^^^^^^^^^^^ name /// ``` pub(crate) struct DirectiveLine<'ln> { pub(crate) line_number: usize, @@ -58,10 +67,44 @@ pub(crate) struct DirectiveLine<'ln> { /// This is "raw" because the directive's name and colon-separated value /// (if present) have not yet been extracted or checked. pub(crate) raw_directive: &'ln str, + + /// Name of the directive. + /// + /// Invariant: `self.raw_directive.starts_with(self.name)` + pub(crate) name: &'ln str, } impl<'ln> DirectiveLine<'ln> { pub(crate) fn applies_to_test_revision(&self, test_revision: Option<&str>) -> bool { self.revision.is_none() || self.revision == test_revision } + + /// Helper method used by `value_after_colon` and `remark_after_space`. + /// Don't call this directly. + fn rest_after_separator(&self, separator: u8) -> Option<&'ln str> { + let n = self.name.len(); + if self.raw_directive.as_bytes().get(n) != Some(&separator) { + return None; + } + + Some(&self.raw_directive[n + 1..]) + } + + /// If this directive uses `name: value` syntax, returns the part after + /// the colon character. + pub(crate) fn value_after_colon(&self) -> Option<&'ln str> { + self.rest_after_separator(b':') + } + + /// If this directive uses `name remark` syntax, returns the part after + /// the separating space. + pub(crate) fn remark_after_space(&self) -> Option<&'ln str> { + self.rest_after_separator(b' ') + } + + /// Allows callers to print `raw_directive` if necessary, + /// without accessing the field directly. + pub(crate) fn display(&self) -> impl fmt::Display { + self.raw_directive + } } From 33c99a0468302f1bb9bd7dd0d63d460b1a88ed52 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 2 Oct 2025 20:16:59 +1000 Subject: [PATCH 16/16] Use new `DirectiveLine` features in directive parsing --- src/tools/compiletest/src/directives.rs | 102 +++++++++--------- .../compiletest/src/directives/auxiliary.rs | 4 +- src/tools/compiletest/src/directives/cfg.rs | 26 ++--- src/tools/compiletest/src/directives/line.rs | 4 +- src/tools/compiletest/src/directives/needs.rs | 15 +-- src/tools/compiletest/src/directives/tests.rs | 11 +- 6 files changed, 73 insertions(+), 89 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index bbbeecc1b71b0..dab6850e0d62a 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -835,9 +835,7 @@ fn check_directive<'a>( directive_ln: &DirectiveLine<'a>, mode: TestMode, ) -> CheckDirectiveResult<'a> { - let &DirectiveLine { raw_directive: directive_ln, .. } = directive_ln; - - let (directive_name, post) = directive_ln.split_once([':', ' ']).unwrap_or((directive_ln, "")); + let &DirectiveLine { name: directive_name, .. } = directive_ln; let is_known_directive = KNOWN_DIRECTIVE_NAMES.contains(&directive_name) || match mode { @@ -846,14 +844,13 @@ fn check_directive<'a>( _ => false, }; - let trailing = post.trim().split_once(' ').map(|(pre, _)| pre).unwrap_or(post); - let trailing_directive = { - // 1. is the directive name followed by a space? (to exclude `:`) - directive_ln.get(directive_name.len()..).is_some_and(|s| s.starts_with(' ')) - // 2. is what is after that directive also a directive (ex: "only-x86 only-arm") - && KNOWN_DIRECTIVE_NAMES.contains(&trailing) - } - .then_some(trailing); + // If it looks like the user tried to put two directives on the same line + // (e.g. `//@ only-linux only-x86_64`), signal an error, because the + // second "directive" would actually be ignored with no effect. + let trailing_directive = directive_ln + .remark_after_space() + .map(|remark| remark.trim_start().split(' ').next().unwrap()) + .filter(|token| KNOWN_DIRECTIVE_NAMES.contains(token)); CheckDirectiveResult { is_known_directive, trailing_directive } } @@ -915,7 +912,7 @@ fn iter_directives( error!( "{testfile}:{line_number}: detected unknown compiletest test directive `{}`", - directive_line.raw_directive, + directive_line.display(), ); return; @@ -1011,13 +1008,9 @@ impl Config { } fn parse_custom_normalization(&self, line: &DirectiveLine<'_>) -> Option { - let &DirectiveLine { raw_directive, .. } = line; - - // FIXME(Zalathar): Integrate name/value splitting into `DirectiveLine` - // instead of doing it here. - let (directive_name, raw_value) = raw_directive.split_once(':')?; + let &DirectiveLine { name, .. } = line; - let kind = match directive_name { + let kind = match name { "normalize-stdout" => NormalizeKind::Stdout, "normalize-stderr" => NormalizeKind::Stderr, "normalize-stderr-32bit" => NormalizeKind::Stderr32bit, @@ -1025,21 +1018,20 @@ impl Config { _ => return None, }; - let Some((regex, replacement)) = parse_normalize_rule(raw_value) else { - error!("couldn't parse custom normalization rule: `{raw_directive}`"); - help!("expected syntax is: `{directive_name}: \"REGEX\" -> \"REPLACEMENT\"`"); + let Some((regex, replacement)) = line.value_after_colon().and_then(parse_normalize_rule) + else { + error!("couldn't parse custom normalization rule: `{}`", line.display()); + help!("expected syntax is: `{name}: \"REGEX\" -> \"REPLACEMENT\"`"); panic!("invalid normalization rule detected"); }; Some(NormalizeRule { kind, regex, replacement }) } fn parse_name_directive(&self, line: &DirectiveLine<'_>, directive: &str) -> bool { - let &DirectiveLine { raw_directive: line, .. } = line; - - // Ensure the directive is a whole word. Do not match "ignore-x86" when - // the line says "ignore-x86_64". - line.starts_with(directive) - && matches!(line.as_bytes().get(directive.len()), None | Some(&b' ') | Some(&b':')) + // FIXME(Zalathar): Ideally, this should raise an error if a name-only + // directive is followed by a colon, since that's the wrong syntax. + // But we would need to fix tests that rely on the current behaviour. + line.name == directive } fn parse_name_value_directive( @@ -1048,22 +1040,26 @@ impl Config { directive: &str, testfile: &Utf8Path, ) -> Option { - let &DirectiveLine { line_number, raw_directive: line, .. } = line; - - let colon = directive.len(); - if line.starts_with(directive) && line.as_bytes().get(colon) == Some(&b':') { - let value = line[(colon + 1)..].to_owned(); - debug!("{}: {}", directive, value); - let value = expand_variables(value, self); - if value.is_empty() { - error!("{testfile}:{line_number}: empty value for directive `{directive}`"); - help!("expected syntax is: `{directive}: value`"); - panic!("empty directive value detected"); - } - Some(value) - } else { - None + let &DirectiveLine { line_number, .. } = line; + + if line.name != directive { + return None; + }; + + // FIXME(Zalathar): This silently discards directives with a matching + // name but no colon. Unfortunately, some directives (e.g. "pp-exact") + // currently rely on _not_ panicking here. + let value = line.value_after_colon()?; + debug!("{}: {}", directive, value); + let value = expand_variables(value.to_owned(), self); + + if value.is_empty() { + error!("{testfile}:{line_number}: empty value for directive `{directive}`"); + help!("expected syntax is: `{directive}: value`"); + panic!("empty directive value detected"); } + + Some(value) } fn set_name_directive(&self, line: &DirectiveLine<'_>, directive: &str, value: &mut bool) { @@ -1453,14 +1449,14 @@ pub(crate) fn make_test_description( } fn ignore_cdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - let &DirectiveLine { raw_directive: line, .. } = line; - if config.debugger != Some(Debugger::Cdb) { return IgnoreDecision::Continue; } if let Some(actual_version) = config.cdb_version { - if let Some(rest) = line.strip_prefix("min-cdb-version:").map(str::trim) { + if line.name == "min-cdb-version" + && let Some(rest) = line.value_after_colon().map(str::trim) + { let min_version = extract_cdb_version(rest).unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); }); @@ -1478,14 +1474,14 @@ fn ignore_cdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { } fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - let &DirectiveLine { raw_directive: line, .. } = line; - if config.debugger != Some(Debugger::Gdb) { return IgnoreDecision::Continue; } if let Some(actual_version) = config.gdb_version { - if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { + if line.name == "min-gdb-version" + && let Some(rest) = line.value_after_colon().map(str::trim) + { let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version) .unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); @@ -1501,7 +1497,9 @@ fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { reason: format!("ignored when the GDB version is lower than {rest}"), }; } - } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { + } else if line.name == "ignore-gdb-version" + && let Some(rest) = line.value_after_colon().map(str::trim) + { let (min_version, max_version) = extract_version_range(rest, extract_gdb_version) .unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); @@ -1528,14 +1526,14 @@ fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { } fn ignore_lldb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - let &DirectiveLine { raw_directive: line, .. } = line; - if config.debugger != Some(Debugger::Lldb) { return IgnoreDecision::Continue; } if let Some(actual_version) = config.lldb_version { - if let Some(rest) = line.strip_prefix("min-lldb-version:").map(str::trim) { + if line.name == "min-lldb-version" + && let Some(rest) = line.value_after_colon().map(str::trim) + { let min_version = rest.parse().unwrap_or_else(|e| { panic!("Unexpected format of LLDB version string: {}\n{:?}", rest, e); }); diff --git a/src/tools/compiletest/src/directives/auxiliary.rs b/src/tools/compiletest/src/directives/auxiliary.rs index 0675a6feac3f0..7cf98178e733f 100644 --- a/src/tools/compiletest/src/directives/auxiliary.rs +++ b/src/tools/compiletest/src/directives/auxiliary.rs @@ -50,9 +50,7 @@ pub(super) fn parse_and_update_aux( testfile: &Utf8Path, aux: &mut AuxProps, ) { - let &DirectiveLine { raw_directive: ln, .. } = directive_line; - - if !(ln.starts_with("aux-") || ln.starts_with("proc-macro")) { + if !(directive_line.name.starts_with("aux-") || directive_line.name == "proc-macro") { return; } diff --git a/src/tools/compiletest/src/directives/cfg.rs b/src/tools/compiletest/src/directives/cfg.rs index 62a4b88a33a6e..3855ba7ac5f42 100644 --- a/src/tools/compiletest/src/directives/cfg.rs +++ b/src/tools/compiletest/src/directives/cfg.rs @@ -6,8 +6,8 @@ use crate::directives::{DirectiveLine, IgnoreDecision}; const EXTRA_ARCHS: &[&str] = &["spirv"]; pub(super) fn handle_ignore(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - let parsed = parse_cfg_name_directive(config, line, "ignore"); - let &DirectiveLine { raw_directive: line, .. } = line; + let parsed = parse_cfg_name_directive(config, line, "ignore-"); + let line = line.display(); match parsed.outcome { MatchOutcome::NoMatch => IgnoreDecision::Continue, @@ -24,8 +24,8 @@ pub(super) fn handle_ignore(config: &Config, line: &DirectiveLine<'_>) -> Ignore } pub(super) fn handle_only(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - let parsed = parse_cfg_name_directive(config, line, "only"); - let &DirectiveLine { raw_directive: line, .. } = line; + let parsed = parse_cfg_name_directive(config, line, "only-"); + let line = line.display(); match parsed.outcome { MatchOutcome::Match => IgnoreDecision::Continue, @@ -50,18 +50,14 @@ fn parse_cfg_name_directive<'a>( line: &'a DirectiveLine<'a>, prefix: &str, ) -> ParsedNameDirective<'a> { - let &DirectiveLine { raw_directive: line, .. } = line; - - if !line.as_bytes().starts_with(prefix.as_bytes()) { - return ParsedNameDirective::not_a_directive(); - } - if line.as_bytes().get(prefix.len()) != Some(&b'-') { + let Some(name) = line.name.strip_prefix(prefix) else { return ParsedNameDirective::not_a_directive(); - } - let line = &line[prefix.len() + 1..]; + }; - let (name, comment) = - line.split_once(&[':', ' ']).map(|(l, c)| (l, Some(c))).unwrap_or((line, None)); + // FIXME(Zalathar): This currently allows either a space or a colon, and + // treats any "value" after a colon as though it were a remark. + // We should instead forbid the colon syntax for these directives. + let comment = line.remark_after_space().or_else(|| line.value_after_colon()); // Some of the matchers might be "" depending on what the target information is. To avoid // problems we outright reject empty directives. @@ -269,7 +265,7 @@ fn parse_cfg_name_directive<'a>( message: "when performing tests on dist toolchain" } - if prefix == "ignore" && outcome == MatchOutcome::Invalid { + if prefix == "ignore-" && outcome == MatchOutcome::Invalid { // Don't error out for ignore-tidy-* diretives, as those are not handled by compiletest. if name.starts_with("tidy-") { outcome = MatchOutcome::External; diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 5c1b592d045d8..49907207d2e61 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -1,5 +1,3 @@ -#![expect(dead_code)] // (removed later in this PR) - use std::fmt; const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; @@ -66,7 +64,7 @@ pub(crate) struct DirectiveLine<'ln> { /// /// This is "raw" because the directive's name and colon-separated value /// (if present) have not yet been extracted or checked. - pub(crate) raw_directive: &'ln str, + raw_directive: &'ln str, /// Name of the directive. /// diff --git a/src/tools/compiletest/src/directives/needs.rs b/src/tools/compiletest/src/directives/needs.rs index 9d72492e5b078..5e9fe59d8d1c3 100644 --- a/src/tools/compiletest/src/directives/needs.rs +++ b/src/tools/compiletest/src/directives/needs.rs @@ -181,17 +181,10 @@ pub(super) fn handle_needs( }, ]; - let &DirectiveLine { raw_directive: ln, .. } = ln; + let &DirectiveLine { name, .. } = ln; - let (name, rest) = match ln.split_once([':', ' ']) { - Some((name, rest)) => (name, Some(rest)), - None => (ln, None), - }; - - // FIXME(jieyouxu): tighten up this parsing to reject using both `:` and ` ` as means to - // delineate value. if name == "needs-target-has-atomic" { - let Some(rest) = rest else { + let Some(rest) = ln.value_after_colon() else { return IgnoreDecision::Error { message: "expected `needs-target-has-atomic` to have a comma-separated list of atomic widths".to_string(), }; @@ -233,7 +226,7 @@ pub(super) fn handle_needs( // FIXME(jieyouxu): share multi-value directive logic with `needs-target-has-atomic` above. if name == "needs-crate-type" { - let Some(rest) = rest else { + let Some(rest) = ln.value_after_colon() else { return IgnoreDecision::Error { message: "expected `needs-crate-type` to have a comma-separated list of crate types" @@ -292,7 +285,7 @@ pub(super) fn handle_needs( break; } else { return IgnoreDecision::Ignore { - reason: if let Some(comment) = rest { + reason: if let Some(comment) = ln.remark_after_space() { format!("{} ({})", need.ignore_reason, comment.trim()) } else { need.ignore_reason.into() diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 93621192d4bd6..95dd46532ba88 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -3,12 +3,11 @@ use std::io::Read; use camino::Utf8Path; use semver::Version; -use super::{ +use crate::common::{Config, Debugger, TestMode}; +use crate::directives::{ DirectivesCache, EarlyProps, Edition, EditionRange, extract_llvm_version, - extract_version_range, iter_directives, parse_normalize_rule, + extract_version_range, iter_directives, line_directive, parse_edition, parse_normalize_rule, }; -use crate::common::{Config, Debugger, TestMode}; -use crate::directives::parse_edition; use crate::executor::{CollectedTestDesc, ShouldPanic}; fn make_test_description( @@ -955,7 +954,9 @@ fn test_needs_target_std() { fn parse_edition_range(line: &str) -> Option { let config = cfg().build(); - let line = super::DirectiveLine { line_number: 0, revision: None, raw_directive: line }; + + let line_with_comment = format!("//@ {line}"); + let line = line_directive(0, &line_with_comment).unwrap(); super::parse_edition_range(&config, &line, "tmp.rs".into()) }