From c42b1773feb5808ae870f8b578a4a8601c7bd322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nurzhan=20Sak=C3=A9n?= Date: Tue, 1 Jul 2025 17:02:32 +0400 Subject: [PATCH 01/16] Add `uX::strict_sub_signed` --- library/core/src/num/uint_macros.rs | 45 ++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/library/core/src/num/uint_macros.rs b/library/core/src/num/uint_macros.rs index 079032f2f96d4..cf34ab8a3cdb4 100644 --- a/library/core/src/num/uint_macros.rs +++ b/library/core/src/num/uint_macros.rs @@ -556,7 +556,7 @@ macro_rules! uint_impl { pub const fn strict_add(self, rhs: Self) -> Self { let (a, b) = self.overflowing_add(rhs); if b { overflow_panic::add() } else { a } - } + } /// Unchecked integer addition. Computes `self + rhs`, assuming overflow /// cannot occur. @@ -653,7 +653,7 @@ macro_rules! uint_impl { pub const fn strict_add_signed(self, rhs: $SignedT) -> Self { let (a, b) = self.overflowing_add_signed(rhs); if b { overflow_panic::add() } else { a } - } + } /// Checked integer subtraction. Computes `self - rhs`, returning /// `None` if overflow occurred. @@ -713,7 +713,7 @@ macro_rules! uint_impl { pub const fn strict_sub(self, rhs: Self) -> Self { let (a, b) = self.overflowing_sub(rhs); if b { overflow_panic::sub() } else { a } - } + } /// Unchecked integer subtraction. Computes `self - rhs`, assuming overflow /// cannot occur. @@ -805,6 +805,43 @@ macro_rules! uint_impl { } } + /// Strict subtraction with a signed integer. Computes `self - rhs`, + /// panicking if overflow occurred. + /// + /// # Panics + /// + /// ## Overflow behavior + /// + /// This function will always panic on overflow, regardless of whether overflow checks are enabled. + /// + /// # Examples + /// + /// ``` + /// #![feature(strict_overflow_ops)] + #[doc = concat!("assert_eq!(3", stringify!($SelfT), ".strict_sub_signed(2), 1);")] + /// ``` + /// + /// The following panic because of overflow: + /// + /// ```should_panic + /// #![feature(strict_overflow_ops)] + #[doc = concat!("let _ = 1", stringify!($SelfT), ".strict_sub_signed(2);")] + /// ``` + /// + /// ```should_panic + /// #![feature(strict_overflow_ops)] + #[doc = concat!("let _ = (", stringify!($SelfT), "::MIN + 2).strict_sub_signed(3);")] + /// ``` + #[unstable(feature = "strict_overflow_ops", issue = "118260")] + #[must_use = "this returns the result of the operation, \ + without modifying the original"] + #[inline] + #[track_caller] + pub const fn strict_sub_signed(self, rhs: $SignedT) -> Self { + let (a, b) = self.overflowing_sub_signed(rhs); + if b { overflow_panic::sub() } else { a } + } + #[doc = concat!( "Checked integer subtraction. Computes `self - rhs` and checks if the result fits into an [`", stringify!($SignedT), "`], returning `None` if overflow occurred." @@ -913,7 +950,7 @@ macro_rules! uint_impl { pub const fn strict_mul(self, rhs: Self) -> Self { let (a, b) = self.overflowing_mul(rhs); if b { overflow_panic::mul() } else { a } - } + } /// Unchecked integer multiplication. Computes `self * rhs`, assuming overflow /// cannot occur. From 79ed7c1f415f40b3167d946c4a292b84b92eede5 Mon Sep 17 00:00:00 2001 From: Nurzhan Saken Date: Tue, 1 Jul 2025 22:32:19 +0400 Subject: [PATCH 02/16] Test upper overflow in `strict_sub_signed` Co-authored-by: zachs18 <8355914+zachs18@users.noreply.github.com> --- library/core/src/num/uint_macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/num/uint_macros.rs b/library/core/src/num/uint_macros.rs index cf34ab8a3cdb4..78002593b63e4 100644 --- a/library/core/src/num/uint_macros.rs +++ b/library/core/src/num/uint_macros.rs @@ -830,7 +830,7 @@ macro_rules! uint_impl { /// /// ```should_panic /// #![feature(strict_overflow_ops)] - #[doc = concat!("let _ = (", stringify!($SelfT), "::MIN + 2).strict_sub_signed(3);")] + #[doc = concat!("let _ = (", stringify!($SelfT), "::MAX).strict_sub_signed(-1);")] /// ``` #[unstable(feature = "strict_overflow_ops", issue = "118260")] #[must_use = "this returns the result of the operation, \ From 004478a829c7b4b7aa608f0f2d4d58a97c7836af Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Wed, 2 Jul 2025 13:19:41 +0000 Subject: [PATCH 03/16] int_log10.rs: change top level doc comments to outer --- library/core/src/num/int_log10.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/num/int_log10.rs b/library/core/src/num/int_log10.rs index 28a3f5d880ad7..649a736b6e7b5 100644 --- a/library/core/src/num/int_log10.rs +++ b/library/core/src/num/int_log10.rs @@ -1,5 +1,5 @@ -/// These functions compute the integer logarithm of their type, assuming -/// that someone has already checked that the value is strictly positive. +//! These functions compute the integer logarithm of their type, assuming +//! that someone has already checked that the value is strictly positive. // 0 < val <= u8::MAX #[inline] From f96d5d960256557a0c2434a308fd32c98e94aa7f Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Wed, 2 Jul 2025 13:29:16 +0000 Subject: [PATCH 04/16] collect.rs: remove empty line after doc comment --- library/core/src/fmt/mod.rs | 1 - library/core/src/iter/traits/collect.rs | 1 - library/core/src/lib.rs | 6 +++--- library/core/src/net/ip_addr.rs | 1 - library/core/src/primitive_docs.rs | 1 - library/core/src/str/mod.rs | 1 - 6 files changed, 3 insertions(+), 8 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 5e50eacec6eef..77ab1c4201cd0 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -854,7 +854,6 @@ impl Display for Arguments<'_> { /// }"; /// assert_eq!(format!("The origin is: {origin:#?}"), expected); /// ``` - #[stable(feature = "rust1", since = "1.0.0")] #[rustc_on_unimplemented( on( diff --git a/library/core/src/iter/traits/collect.rs b/library/core/src/iter/traits/collect.rs index 3bc9cff8072bf..ab27650067980 100644 --- a/library/core/src/iter/traits/collect.rs +++ b/library/core/src/iter/traits/collect.rs @@ -436,7 +436,6 @@ pub trait Extend { /// **For implementors:** For a collection to unsafely rely on this method's safety precondition (that is, /// invoke UB if they are violated), it must implement `extend_reserve` correctly. In other words, /// callers may assume that if they `extend_reserve`ed enough space they can call this method. - // This method is for internal usage only. It is only on the trait because of specialization's limitations. #[unstable(feature = "extend_one_unchecked", issue = "none")] #[doc(hidden)] diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 2f701171505c7..de5174bcbf6f0 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -39,9 +39,9 @@ //! return. You should mark your implementation using `#[panic_handler]`. //! //! * `rust_eh_personality` - is used by the failure mechanisms of the -//! compiler. This is often mapped to GCC's personality function, but crates -//! which do not trigger a panic can be assured that this function is never -//! called. The `lang` attribute is called `eh_personality`. +//! compiler. This is often mapped to GCC's personality function, but crates +//! which do not trigger a panic can be assured that this function is never +//! called. The `lang` attribute is called `eh_personality`. #![stable(feature = "core", since = "1.6.0")] #![doc( diff --git a/library/core/src/net/ip_addr.rs b/library/core/src/net/ip_addr.rs index aaa68e8d7d1ad..49a7ae5de5c8f 100644 --- a/library/core/src/net/ip_addr.rs +++ b/library/core/src/net/ip_addr.rs @@ -787,7 +787,6 @@ impl Ipv4Addr { /// [IANA IPv4 Special-Purpose Address Registry]: https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml /// [unspecified address]: Ipv4Addr::UNSPECIFIED /// [broadcast address]: Ipv4Addr::BROADCAST - /// /// # Examples /// diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index 2c77c55745b46..28124ac5aa8b2 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -1365,7 +1365,6 @@ mod prim_f16 {} /// x = a + b + c + d; // As written /// x = (a + c) + (b + d); // Reordered to shorten critical path and enable vectorization /// ``` - #[stable(feature = "rust1", since = "1.0.0")] mod prim_f32 {} diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index fe64132ff227d..98b5e29009c4e 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -2648,7 +2648,6 @@ impl str { /// you're trying to parse into. /// /// `parse` can parse into any type that implements the [`FromStr`] trait. - /// /// # Errors /// From ca4a712b59b9ed216e8bb6e24140b08d7020c68c Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Thu, 3 Jul 2025 12:41:05 +0000 Subject: [PATCH 05/16] clippy fix: markdown indentation for indented items after line break --- library/core/src/async_iter/async_iter.rs | 10 +++++----- library/core/src/mem/mod.rs | 2 +- library/core/src/ptr/docs/as_uninit_slice.md | 20 ++++++++++---------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/library/core/src/async_iter/async_iter.rs b/library/core/src/async_iter/async_iter.rs index 069c50c2531b0..c21c08320bef6 100644 --- a/library/core/src/async_iter/async_iter.rs +++ b/library/core/src/async_iter/async_iter.rs @@ -28,15 +28,15 @@ pub trait AsyncIterator { /// async iterator state: /// /// - `Poll::Pending` means that this async iterator's next value is not ready - /// yet. Implementations will ensure that the current task will be notified - /// when the next value may be ready. + /// yet. Implementations will ensure that the current task will be notified + /// when the next value may be ready. /// /// - `Poll::Ready(Some(val))` means that the async iterator has successfully - /// produced a value, `val`, and may produce further values on subsequent - /// `poll_next` calls. + /// produced a value, `val`, and may produce further values on subsequent + /// `poll_next` calls. /// /// - `Poll::Ready(None)` means that the async iterator has terminated, and - /// `poll_next` should not be invoked again. + /// `poll_next` should not be invoked again. /// /// # Panics /// diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index c00585de06496..8db52a38014e3 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -36,7 +36,7 @@ pub use crate::intrinsics::transmute; /// * If you want to leak memory, see [`Box::leak`]. /// * If you want to obtain a raw pointer to the memory, see [`Box::into_raw`]. /// * If you want to dispose of a value properly, running its destructor, see -/// [`mem::drop`]. +/// [`mem::drop`]. /// /// # Safety /// diff --git a/library/core/src/ptr/docs/as_uninit_slice.md b/library/core/src/ptr/docs/as_uninit_slice.md index c80c04058838f..1113f4748c2df 100644 --- a/library/core/src/ptr/docs/as_uninit_slice.md +++ b/library/core/src/ptr/docs/as_uninit_slice.md @@ -10,24 +10,24 @@ When calling this method, you have to ensure that *either* the pointer is null * all of the following is true: * The pointer must be [valid] for reads for `ptr.len() * size_of::()` many bytes, -and it must be properly aligned. This means in particular: + and it must be properly aligned. This means in particular: * The entire memory range of this slice must be contained within a single [allocation]! -Slices can never span across multiple allocations. + Slices can never span across multiple allocations. * The pointer must be aligned even for zero-length slices. One -reason for this is that enum layout optimizations may rely on references -(including slices of any length) being aligned and non-null to distinguish -them from other data. You can obtain a pointer that is usable as `data` -for zero-length slices using [`NonNull::dangling()`]. + reason for this is that enum layout optimizations may rely on references + (including slices of any length) being aligned and non-null to distinguish + them from other data. You can obtain a pointer that is usable as `data` + for zero-length slices using [`NonNull::dangling()`]. * The total size `ptr.len() * size_of::()` of the slice must be no larger than `isize::MAX`. -See the safety documentation of [`pointer::offset`]. + See the safety documentation of [`pointer::offset`]. * You must enforce Rust's aliasing rules, since the returned lifetime `'a` is -arbitrarily chosen and does not necessarily reflect the actual lifetime of the data. -In particular, while this reference exists, the memory the pointer points to must -not get mutated (except inside `UnsafeCell`). + arbitrarily chosen and does not necessarily reflect the actual lifetime of the data. + In particular, while this reference exists, the memory the pointer points to must + not get mutated (except inside `UnsafeCell`). This applies even if the result of this method is unused! From 8388b31c5bb1478defee2963c6caae69348a7e1a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 18 Jul 2025 14:02:00 +1000 Subject: [PATCH 06/16] Split some multi-snapshot tests to make blessing easier When a snapshot test fails, it only emits a `.pending-snap` file for the first snapshot assertion that actually failed, because subsequent assertions aren't executed. That makes it cumbersome to re-bless tests that contain multiple snapshot assertions. --- src/bootstrap/src/core/builder/tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 51a906496923b..e71edbb8bc5fb 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -712,7 +712,11 @@ mod snapshot { [build] llvm [build] rustc 0 -> rustc 1 "); + } + #[test] + fn build_rustc_no_explicit_stage() { + let ctx = TestCtx::new(); insta::assert_snapshot!( ctx.config("build") .path("rustc") @@ -1299,7 +1303,11 @@ mod snapshot { [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 "); + } + #[test] + fn check_rustc_no_explicit_stage() { + let ctx = TestCtx::new(); insta::assert_snapshot!( ctx.config("check") .path("rustc") From 7a217e1ba51b486cd7d56eb684ae44dc3a44e1b3 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 16 Jul 2025 17:16:15 +1000 Subject: [PATCH 07/16] Don't trigger an LLVM build from check builds using the stage 0 compiler --- src/bootstrap/src/core/build_steps/compile.rs | 29 ++++++++++++------- src/bootstrap/src/core/builder/tests.rs | 8 ----- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 09bb2e35bdaa2..9d29b1320b115 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1323,7 +1323,7 @@ pub fn rustc_cargo_env( builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection, - build_stage: u32, + _build_stage: u32, ) { // Set some configuration variables picked up by build scripts and // the compiler alike @@ -1379,18 +1379,24 @@ pub fn rustc_cargo_env( cargo.rustflag("--cfg=llvm_enzyme"); } - // Note that this is disabled if LLVM itself is disabled or we're in a check - // build. If we are in a check build we still go ahead here presuming we've - // detected that LLVM is already built and good to go which helps prevent - // busting caches (e.g. like #71152). + // These conditionals represent a tension between three forces: + // - For non-check builds, we need to define some LLVM-related environment + // variables, requiring LLVM to have been built. + // - For check builds, we want to avoid building LLVM if possible. + // - Check builds and non-check builds should have the same environment if + // possible, to avoid unnecessary rebuilds due to cache-busting. + // + // Therefore we try to avoid building LLVM for check builds, but only if + // building LLVM would be expensive. If "building" LLVM is cheap + // (i.e. it's already built or is downloadable), we prefer to maintain a + // consistent environment between check and non-check builds. if builder.config.llvm_enabled(target) { - let building_is_expensive = + let building_llvm_is_expensive = crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target, false) .should_build(); - // `top_stage == stage` might be false for `check --stage 1`, if we are building the stage 1 compiler - let can_skip_build = builder.kind == Kind::Check && builder.top_stage == build_stage; - let should_skip_build = building_is_expensive && can_skip_build; - if !should_skip_build { + + let skip_llvm = (builder.kind == Kind::Check) && building_llvm_is_expensive; + if !skip_llvm { rustc_llvm_env(builder, cargo, target) } } @@ -1407,6 +1413,9 @@ pub fn rustc_cargo_env( /// Pass down configuration from the LLVM build into the build of /// rustc_llvm and rustc_codegen_llvm. +/// +/// Note that this has the side-effect of _building LLVM_, which is sometimes +/// unwanted (e.g. for check builds). fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) { if builder.config.is_rust_llvm(target) { cargo.env("LLVM_RUSTLLVM", "1"); diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index e71edbb8bc5fb..97e7e512ce1a9 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1298,7 +1298,6 @@ mod snapshot { ctx.config("check") .path("compiler") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1312,7 +1311,6 @@ mod snapshot { ctx.config("check") .path("rustc") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 "); } @@ -1332,7 +1330,6 @@ mod snapshot { .path("compiler") .stage(1) .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1464,7 +1461,6 @@ mod snapshot { .paths(&["library", "compiler"]) .args(&args) .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1478,7 +1474,6 @@ mod snapshot { ctx.config("check") .path("miri") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> Miri 1 "); @@ -1499,7 +1494,6 @@ mod snapshot { .path("miri") .stage(1) .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> Miri 1 "); @@ -1552,7 +1546,6 @@ mod snapshot { ctx.config("check") .path("rustc_codegen_cranelift") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1566,7 +1559,6 @@ mod snapshot { ctx.config("check") .path("rust-analyzer") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> rust-analyzer 1 "); From a028ca5f9bd771e825e5c8c3c55f938aea89985e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 16 Jul 2025 20:53:29 +1000 Subject: [PATCH 08/16] Clean up an unused compiler-stage parameter --- src/bootstrap/src/core/build_steps/check.rs | 2 +- src/bootstrap/src/core/build_steps/compile.rs | 11 +++-------- src/bootstrap/src/core/build_steps/test.rs | 4 ++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/check.rs b/src/bootstrap/src/core/build_steps/check.rs index 3278b55305c8b..0aec3cf7ca4b3 100644 --- a/src/bootstrap/src/core/build_steps/check.rs +++ b/src/bootstrap/src/core/build_steps/check.rs @@ -350,7 +350,7 @@ impl Step for CodegenBackend { cargo .arg("--manifest-path") .arg(builder.src.join(format!("compiler/rustc_codegen_{backend}/Cargo.toml"))); - rustc_cargo_env(builder, &mut cargo, target, build_compiler.stage); + rustc_cargo_env(builder, &mut cargo, target); let _guard = builder.msg_check(format!("rustc_codegen_{backend}"), target, None); diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 9d29b1320b115..fc5bf6dc354ea 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1316,15 +1316,10 @@ pub fn rustc_cargo( cargo.env("RUSTC_WRAPPER", ccache); } - rustc_cargo_env(builder, cargo, target, build_compiler.stage); + rustc_cargo_env(builder, cargo, target); } -pub fn rustc_cargo_env( - builder: &Builder<'_>, - cargo: &mut Cargo, - target: TargetSelection, - _build_stage: u32, -) { +pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) { // Set some configuration variables picked up by build scripts and // the compiler alike cargo @@ -1674,7 +1669,7 @@ impl Step for CodegenBackend { cargo .arg("--manifest-path") .arg(builder.src.join(format!("compiler/rustc_codegen_{backend}/Cargo.toml"))); - rustc_cargo_env(builder, &mut cargo, target, compiler.stage); + rustc_cargo_env(builder, &mut cargo, target); // Ideally, we'd have a separate step for the individual codegen backends, // like we have in tests (test::CodegenGCC) but that would require a lot of restructuring. diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9e7ea5c115f19..b7dd2b778bb66 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -3386,7 +3386,7 @@ impl Step for CodegenCranelift { cargo .arg("--manifest-path") .arg(builder.src.join("compiler/rustc_codegen_cranelift/build_system/Cargo.toml")); - compile::rustc_cargo_env(builder, &mut cargo, target, compiler.stage); + compile::rustc_cargo_env(builder, &mut cargo, target); // Avoid incremental cache issues when changing rustc cargo.env("CARGO_BUILD_INCREMENTAL", "false"); @@ -3518,7 +3518,7 @@ impl Step for CodegenGCC { cargo .arg("--manifest-path") .arg(builder.src.join("compiler/rustc_codegen_gcc/build_system/Cargo.toml")); - compile::rustc_cargo_env(builder, &mut cargo, target, compiler.stage); + compile::rustc_cargo_env(builder, &mut cargo, target); add_cg_gcc_cargo_flags(&mut cargo, &gcc); // Avoid incremental cache issues when changing rustc From cadcc1ce7137e63c70740e6cc3897557094382af Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Thu, 17 Jul 2025 20:14:16 +0200 Subject: [PATCH 09/16] bootstrap: Ignore `rust.debuginfo-level-tests` for codegen tests codegen tests typically depend on the raw LLVM IR output and are sensitive to debuginfo level. So do not apply `rust.debuginfo-level-tests` for codegen tests. Before this commit: $ ./x test --set rust.debuginfo-level-tests=2 tests/codegen --force-rerun test result: FAILED. 654 passed; 136 failed; 75 ignored; 0 measured; 0 filtered out; finished in 3.22s After this commit: $ ./x test --set rust.debuginfo-level-tests=2 tests/codegen --force-rerun NOTE: ignoring `rust.debuginfo-level-tests=2` for codegen tests test result: ok. 790 passed; 0 failed; 75 ignored; 0 measured; 0 filtered out; finished in 3.21s --- src/bootstrap/src/core/build_steps/test.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9e7ea5c115f19..38f36de8f05c0 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1810,7 +1810,24 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the } let mut flags = if is_rustdoc { Vec::new() } else { vec!["-Crpath".to_string()] }; - flags.push(format!("-Cdebuginfo={}", builder.config.rust_debuginfo_level_tests)); + flags.push(format!( + "-Cdebuginfo={}", + if suite == "codegen" { + // codegen tests typically check LLVM IR and are sensitive to additional debuginfo. + // So do not apply `rust.debuginfo-level-tests` for codegen tests. + if builder.config.rust_debuginfo_level_tests + != crate::core::config::DebuginfoLevel::None + { + println!( + "NOTE: ignoring `rust.debuginfo-level-tests={}` for codegen tests", + builder.config.rust_debuginfo_level_tests + ); + } + crate::core::config::DebuginfoLevel::None + } else { + builder.config.rust_debuginfo_level_tests + } + )); flags.extend(builder.config.cmd.compiletest_rustc_args().iter().map(|s| s.to_string())); if suite != "mir-opt" { From 664d742933e020f70032e0fd8cd9c8869848fd4f Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Fri, 18 Jul 2025 18:29:29 +0200 Subject: [PATCH 10/16] rustc_codegen_ssa: Don't skip target-features after crt-static The current behaviour introduced by commit a50a3b8e318594c41783294e440d864763e412ef would discard any target features specified after crt-static (the only member of RUSTC_SPECIFIC_FEATURES). This is because it returned instead of continuing processing the next flag. Signed-off-by: Jens Reidel --- compiler/rustc_codegen_ssa/src/target_features.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index 53df99993f06b..def4ec13e87b5 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -149,14 +149,14 @@ fn parse_rust_feature_flag<'a>( if let Some(base_feature) = feature.strip_prefix('+') { // Skip features that are not target features, but rustc features. if RUSTC_SPECIFIC_FEATURES.contains(&base_feature) { - return; + continue; } callback(base_feature, sess.target.implied_target_features(base_feature), true) } else if let Some(base_feature) = feature.strip_prefix('-') { // Skip features that are not target features, but rustc features. if RUSTC_SPECIFIC_FEATURES.contains(&base_feature) { - return; + continue; } // If `f1` implies `f2`, then `!f2` implies `!f1` -- this is standard logical From 1b35d5f89c4e3c605cd455a28f00aad24de0a662 Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Fri, 18 Jul 2025 18:55:33 +0200 Subject: [PATCH 11/16] tests: Add a regression test for crt-static with target features Signed-off-by: Jens Reidel --- .../crt-static-with-target-features-works.rs | 24 +++++++++++++++++++ ...t-static-with-target-features-works.stderr | 8 +++++++ 2 files changed, 32 insertions(+) create mode 100644 tests/ui/cfg/crt-static-with-target-features-works.rs create mode 100644 tests/ui/cfg/crt-static-with-target-features-works.stderr diff --git a/tests/ui/cfg/crt-static-with-target-features-works.rs b/tests/ui/cfg/crt-static-with-target-features-works.rs new file mode 100644 index 0000000000000..bce022296245f --- /dev/null +++ b/tests/ui/cfg/crt-static-with-target-features-works.rs @@ -0,0 +1,24 @@ +// Test to ensure that specifying a value for crt-static in target features +// does not result in skipping the features following it. +// This is a regression test for #144143 + +//@ add-core-stubs +//@ needs-llvm-components: x86 +//@ compile-flags: --target=x86_64-unknown-linux-gnu +//@ compile-flags: -Ctarget-feature=+crt-static,+avx2 + +#![crate_type = "rlib"] +#![feature(no_core, rustc_attrs, lang_items)] +#![no_core] + +extern crate minicore; +use minicore::*; + +#[rustc_builtin_macro] +macro_rules! compile_error { + () => {}; +} + +#[cfg(target_feature = "avx2")] +compile_error!("+avx2"); +//~^ ERROR: +avx2 diff --git a/tests/ui/cfg/crt-static-with-target-features-works.stderr b/tests/ui/cfg/crt-static-with-target-features-works.stderr new file mode 100644 index 0000000000000..6f265c685bb9f --- /dev/null +++ b/tests/ui/cfg/crt-static-with-target-features-works.stderr @@ -0,0 +1,8 @@ +error: +avx2 + --> $DIR/crt-static-with-target-features-works.rs:23:1 + | +LL | compile_error!("+avx2"); + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From 2d51acd2fbcbadb6f30709c5dd305494d413d388 Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Fri, 18 Jul 2025 19:44:20 +0200 Subject: [PATCH 12/16] tests: assembly: cstring-merging: Disable GlobalMerge pass The test relies on LLVM not merging all the globals into one and would currently otherwise fail on powerpc64le. Signed-off-by: Jens Reidel --- tests/assembly/cstring-merging.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/assembly/cstring-merging.rs b/tests/assembly/cstring-merging.rs index f7d0775f7affd..03688e0068b79 100644 --- a/tests/assembly/cstring-merging.rs +++ b/tests/assembly/cstring-merging.rs @@ -2,7 +2,7 @@ // other architectures (including ARM and x86-64) use the prefix `.Lanon.` //@ only-linux //@ assembly-output: emit-asm -//@ compile-flags: --crate-type=lib -Copt-level=3 +//@ compile-flags: --crate-type=lib -Copt-level=3 -Cllvm-args=-enable-global-merge=0 //@ edition: 2024 use std::ffi::CStr; From c79f62d1ea5d210de9bc98ddc7e0588533a2b311 Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Wed, 2 Jul 2025 13:27:29 +0000 Subject: [PATCH 13/16] clippy fix: bound in one place --- library/core/src/cmp.rs | 5 +++-- library/core/src/iter/traits/iterator.rs | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index b1ca3701fa5ae..7abadc9124ba3 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -1481,13 +1481,14 @@ pub trait PartialOrd: PartialEq + PointeeSized { } } -fn default_chaining_impl( +fn default_chaining_impl( lhs: &T, rhs: &U, p: impl FnOnce(Ordering) -> bool, ) -> ControlFlow where - T: PartialOrd, + T: PartialOrd + PointeeSized, + U: PointeeSized, { // It's important that this only call `partial_cmp` once, not call `eq` then // one of the relational operators. We don't want to `bcmp`-then-`memcp` a diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index f296792b1dcb2..10f9d464f7d7f 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -3414,10 +3414,10 @@ pub trait Iterator { /// ``` #[stable(feature = "iter_copied", since = "1.36.0")] #[rustc_diagnostic_item = "iter_copied"] - fn copied<'a, T: 'a>(self) -> Copied + fn copied<'a, T>(self) -> Copied where + T: Copy + 'a, Self: Sized + Iterator, - T: Copy, { Copied::new(self) } @@ -3462,10 +3462,10 @@ pub trait Iterator { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[rustc_diagnostic_item = "iter_cloned"] - fn cloned<'a, T: 'a>(self) -> Cloned + fn cloned<'a, T>(self) -> Cloned where + T: Clone + 'a, Self: Sized + Iterator, - T: Clone, { Cloned::new(self) } From 14d097f6418eacf58e300abfb1531111192e9767 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Fri, 18 Jul 2025 17:19:33 -0700 Subject: [PATCH 14/16] Give a message with a span on validation error --- compiler/rustc_mir_transform/src/validate.rs | 18 ++++++++++-------- .../miri/tests/panic/mir-validation.stderr | 14 ++++++++++---- tests/ui/mir/validate/critical-edge.rs | 4 ++-- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs index cbb9bbfd12f9f..659ca4df159d0 100644 --- a/compiler/rustc_mir_transform/src/validate.rs +++ b/compiler/rustc_mir_transform/src/validate.rs @@ -119,14 +119,16 @@ impl<'a, 'tcx> CfgChecker<'a, 'tcx> { #[track_caller] fn fail(&self, location: Location, msg: impl AsRef) { // We might see broken MIR when other errors have already occurred. - assert!( - self.tcx.dcx().has_errors().is_some(), - "broken MIR in {:?} ({}) at {:?}:\n{}", - self.body.source.instance, - self.when, - location, - msg.as_ref(), - ); + if self.tcx.dcx().has_errors().is_none() { + span_bug!( + self.body.source_info(location).span, + "broken MIR in {:?} ({}) at {:?}:\n{}", + self.body.source.instance, + self.when, + location, + msg.as_ref(), + ); + } } fn check_edge(&mut self, location: Location, bb: BasicBlock, edge_kind: EdgeKind) { diff --git a/src/tools/miri/tests/panic/mir-validation.stderr b/src/tools/miri/tests/panic/mir-validation.stderr index dc70d129da3c3..f801ac907e6b8 100644 --- a/src/tools/miri/tests/panic/mir-validation.stderr +++ b/src/tools/miri/tests/panic/mir-validation.stderr @@ -1,11 +1,15 @@ +error: internal compiler error: compiler/rustc_mir_transform/src/validate.rs:LL:CC: broken MIR in Item(DefId) (after phase change to runtime-optimized) at bb0[1]: + place (*(_2.0: *mut i32)) has deref as a later projection (it is only permitted as the first projection) + --> tests/panic/mir-validation.rs:LL:CC + | +LL | *(tuple.0) = 1; + | ^^^^^^^^^^^^^^ + thread 'rustc' panicked at compiler/rustc_mir_transform/src/validate.rs:LL:CC: -broken MIR in Item(DefId) (after phase change to runtime-optimized) at bb0[1]: -place (*(_2.0: *mut i32)) has deref as a later projection (it is only permitted as the first projection) +Box stack backtrace: -error: the compiler unexpectedly panicked. this is a bug. - @@ -20,3 +24,5 @@ LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | +error: aborting due to 1 previous error + diff --git a/tests/ui/mir/validate/critical-edge.rs b/tests/ui/mir/validate/critical-edge.rs index 2a3bf6a6181b4..7fe3891d6424e 100644 --- a/tests/ui/mir/validate/critical-edge.rs +++ b/tests/ui/mir/validate/critical-edge.rs @@ -21,6 +21,8 @@ pub fn f(a: u32) -> u32 { } bb1 = { Call(RET = f(1), ReturnTo(bb2), UnwindTerminate(ReasonAbi)) +//~^ ERROR broken MIR in Item +//~| ERROR encountered critical edge in `Call` terminator } bb2 = { @@ -29,5 +31,3 @@ pub fn f(a: u32) -> u32 { } } } - -//~? RAW encountered critical edge in `Call` terminator From 0586c63e070981af7df53e2f778d3e50293d8103 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 9 Jul 2025 23:59:00 -0700 Subject: [PATCH 15/16] Allow `Rvalue::Repeat` to return true in `rvalue_creates_operand` too The conversation in 143502 made be realize how easy this is to handle, since the only possibilty is ZSTs -- everything else ends up with the destination being `LocalKind::Memory` and thus doesn't call `codegen_rvalue_operand` at all. This gets us perilously close to a world where `rvalue_creates_operand` only ever returns true. I'll try out such a world next :) --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 16 ++++++-- tests/codegen/repeat-operand.rs | 39 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 tests/codegen/repeat-operand.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 2587e89417a6a..e872f8434e56e 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -630,7 +630,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { OperandRef { val: OperandValue::Immediate(static_), layout } } mir::Rvalue::Use(ref operand) => self.codegen_operand(bx, operand), - mir::Rvalue::Repeat(..) => bug!("{rvalue:?} in codegen_rvalue_operand"), + mir::Rvalue::Repeat(ref elem, len_const) => { + // All arrays have `BackendRepr::Memory`, so only the ZST cases + // end up here. Anything else forces the destination local to be + // `Memory`, and thus ends up handled in `codegen_rvalue` instead. + let operand = self.codegen_operand(bx, elem); + let array_ty = Ty::new_array_with_const_len(bx.tcx(), operand.layout.ty, len_const); + let array_ty = self.monomorphize(array_ty); + let array_layout = bx.layout_of(array_ty); + assert!(array_layout.is_zst()); + OperandRef { val: OperandValue::ZeroSized, layout: array_layout } + } mir::Rvalue::Aggregate(ref kind, ref fields) => { let (variant_index, active_field_index) = match **kind { mir::AggregateKind::Adt(_, variant_index, _, _, active_field_index) => { @@ -1000,12 +1010,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { mir::Rvalue::NullaryOp(..) | mir::Rvalue::ThreadLocalRef(_) | mir::Rvalue::Use(..) | + mir::Rvalue::Repeat(..) | // (*) mir::Rvalue::Aggregate(..) | // (*) mir::Rvalue::WrapUnsafeBinder(..) => // (*) true, - // Arrays are always aggregates, so it's not worth checking anything here. - // (If it's really `[(); N]` or `[T; 0]` and we use the place path, fine.) - mir::Rvalue::Repeat(..) => false, } // (*) this is only true if the type is suitable diff --git a/tests/codegen/repeat-operand.rs b/tests/codegen/repeat-operand.rs new file mode 100644 index 0000000000000..6464b35ddcad3 --- /dev/null +++ b/tests/codegen/repeat-operand.rs @@ -0,0 +1,39 @@ +//@ compile-flags: -Copt-level=1 -Cno-prepopulate-passes + +// This test is here to hit the `Rvalue::Repeat` case in `codegen_rvalue_operand`. +// It only applies when the resulting array is a ZST, so the test is written in +// such a way as to keep MIR optimizations from seeing that fact and removing +// the local and statement altogether. (At the time of writing, no other codegen +// test hit that code path, nor did a stage 2 build of the compiler.) + +#![crate_type = "lib"] + +#[repr(transparent)] +pub struct Wrapper([T; N]); + +// CHECK-LABEL: define {{.+}}do_repeat{{.+}}() +// CHECK-NEXT: start: +// CHECK-NOT: alloca +// CHECK-NEXT: ret void +// CHECK-LABEL: define {{.+}}do_repeat{{.+}}(i32 noundef %x) +// CHECK-NEXT: start: +// CHECK-NOT: alloca +// CHECK-NEXT: ret void +#[inline(never)] +pub fn do_repeat(x: T) -> Wrapper { + Wrapper([x; N]) +} + +// CHECK-LABEL: @trigger_repeat_zero_len +#[no_mangle] +pub fn trigger_repeat_zero_len() -> Wrapper { + // CHECK: call void {{.+}}do_repeat{{.+}}(i32 noundef 4) + do_repeat(4) +} + +// CHECK-LABEL: @trigger_repeat_zst +#[no_mangle] +pub fn trigger_repeat_zst() -> Wrapper<(), 8> { + // CHECK: call void {{.+}}do_repeat{{.+}}() + do_repeat(()) +} From 4b8f869c638e6d5090420ff46bd14e7b7d909690 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 19 Jul 2025 20:49:52 -0700 Subject: [PATCH 16/16] Split repeat-operand codegen test Looks like the output it's looking for can be in different orders, so separate tests will fix that. --- ...-operand.rs => repeat-operand-zero-len.rs} | 11 -------- tests/codegen/repeat-operand-zst-elem.rs | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 11 deletions(-) rename tests/codegen/{repeat-operand.rs => repeat-operand-zero-len.rs} (77%) create mode 100644 tests/codegen/repeat-operand-zst-elem.rs diff --git a/tests/codegen/repeat-operand.rs b/tests/codegen/repeat-operand-zero-len.rs similarity index 77% rename from tests/codegen/repeat-operand.rs rename to tests/codegen/repeat-operand-zero-len.rs index 6464b35ddcad3..b4cec42a07c5c 100644 --- a/tests/codegen/repeat-operand.rs +++ b/tests/codegen/repeat-operand-zero-len.rs @@ -11,10 +11,6 @@ #[repr(transparent)] pub struct Wrapper([T; N]); -// CHECK-LABEL: define {{.+}}do_repeat{{.+}}() -// CHECK-NEXT: start: -// CHECK-NOT: alloca -// CHECK-NEXT: ret void // CHECK-LABEL: define {{.+}}do_repeat{{.+}}(i32 noundef %x) // CHECK-NEXT: start: // CHECK-NOT: alloca @@ -30,10 +26,3 @@ pub fn trigger_repeat_zero_len() -> Wrapper { // CHECK: call void {{.+}}do_repeat{{.+}}(i32 noundef 4) do_repeat(4) } - -// CHECK-LABEL: @trigger_repeat_zst -#[no_mangle] -pub fn trigger_repeat_zst() -> Wrapper<(), 8> { - // CHECK: call void {{.+}}do_repeat{{.+}}() - do_repeat(()) -} diff --git a/tests/codegen/repeat-operand-zst-elem.rs b/tests/codegen/repeat-operand-zst-elem.rs new file mode 100644 index 0000000000000..c3637759afa34 --- /dev/null +++ b/tests/codegen/repeat-operand-zst-elem.rs @@ -0,0 +1,28 @@ +//@ compile-flags: -Copt-level=1 -Cno-prepopulate-passes + +// This test is here to hit the `Rvalue::Repeat` case in `codegen_rvalue_operand`. +// It only applies when the resulting array is a ZST, so the test is written in +// such a way as to keep MIR optimizations from seeing that fact and removing +// the local and statement altogether. (At the time of writing, no other codegen +// test hit that code path, nor did a stage 2 build of the compiler.) + +#![crate_type = "lib"] + +#[repr(transparent)] +pub struct Wrapper([T; N]); + +// CHECK-LABEL: define {{.+}}do_repeat{{.+}}() +// CHECK-NEXT: start: +// CHECK-NOT: alloca +// CHECK-NEXT: ret void +#[inline(never)] +pub fn do_repeat(x: T) -> Wrapper { + Wrapper([x; N]) +} + +// CHECK-LABEL: @trigger_repeat_zst_elem +#[no_mangle] +pub fn trigger_repeat_zst_elem() -> Wrapper<(), 8> { + // CHECK: call void {{.+}}do_repeat{{.+}}() + do_repeat(()) +}