From 08ee5f5e63afe3e6a3d852932484c506d0eaee87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 18 Jan 2025 23:40:27 +0000 Subject: [PATCH 01/28] Do not ICE on default_field_value const with lifetimes Fix #135649. --- .../rustc_borrowck/src/universal_regions.rs | 6 ++++- .../do-not-ice-on-invalid-lifetime.rs | 6 +++++ .../do-not-ice-on-invalid-lifetime.stderr | 23 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.rs create mode 100644 tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.stderr diff --git a/compiler/rustc_borrowck/src/universal_regions.rs b/compiler/rustc_borrowck/src/universal_regions.rs index 26af86c0cdd49..b25f449079a76 100644 --- a/compiler/rustc_borrowck/src/universal_regions.rs +++ b/compiler/rustc_borrowck/src/universal_regions.rs @@ -21,6 +21,7 @@ use std::iter; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Diag; use rustc_hir::BodyOwnerKind; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::lang_items::LangItem; use rustc_index::IndexVec; @@ -603,7 +604,10 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { BodyOwnerKind::Const { .. } | BodyOwnerKind::Static(..) => { let identity_args = GenericArgs::identity_for_item(tcx, typeck_root_def_id); - if self.mir_def.to_def_id() == typeck_root_def_id { + if self.mir_def.to_def_id() == typeck_root_def_id + // Do not ICE when checking default_field_values consts with lifetimes (#135649) + && DefKind::Field != tcx.def_kind(tcx.parent(typeck_root_def_id)) + { let args = self.infcx.replace_free_regions_with_nll_infer_vars(FR, identity_args); DefiningTy::Const(self.mir_def.to_def_id(), args) diff --git a/tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.rs b/tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.rs new file mode 100644 index 0000000000000..71d90ddd935ff --- /dev/null +++ b/tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.rs @@ -0,0 +1,6 @@ +#![feature(default_field_values)] +struct A<'a> { //~ ERROR lifetime parameter `'a` is never used + x: Vec = Vec::new(), //~ ERROR missing lifetime specifier +} + +fn main() {} diff --git a/tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.stderr b/tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.stderr new file mode 100644 index 0000000000000..20b9afe80cdc8 --- /dev/null +++ b/tests/ui/structs/default-field-values/do-not-ice-on-invalid-lifetime.stderr @@ -0,0 +1,23 @@ +error[E0106]: missing lifetime specifier + --> $DIR/do-not-ice-on-invalid-lifetime.rs:3:12 + | +LL | x: Vec = Vec::new(), + | ^ expected named lifetime parameter + | +help: consider using the `'a` lifetime + | +LL | x: Vec> = Vec::new(), + | ++++ + +error[E0392]: lifetime parameter `'a` is never used + --> $DIR/do-not-ice-on-invalid-lifetime.rs:2:10 + | +LL | struct A<'a> { + | ^^ unused lifetime parameter + | + = help: consider removing `'a`, referring to it in a field, or using a marker such as `PhantomData` + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0106, E0392. +For more information about an error, try `rustc --explain E0106`. From 16abb39c9dd7e346ffecd5bf03c315bf374ece5f Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Tue, 11 Feb 2025 17:48:36 +0100 Subject: [PATCH 02/28] Reword file lock documentation to clarify advisory vs mandatory Remove the word "advisory", and make it more explicit that the lock may be advisory or mandatory depending on platform. --- library/std/src/fs.rs | 75 +++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 83b009c86dc08..7c8279f2555bd 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -624,20 +624,20 @@ impl File { self.inner.datasync() } - /// Acquire an exclusive advisory lock on the file. Blocks until the lock can be acquired. + /// Acquire an exclusive lock on the file. Blocks until the lock can be acquired. /// - /// This acquires an exclusive advisory lock; no other file handle to this file may acquire - /// another lock. + /// This acquires an exclusive lock; no other file handle to this file may acquire another lock. /// - /// If this file handle/descriptor, or a clone of it, already holds an advisory lock the exact - /// behavior is unspecified and platform dependent, including the possibility that it will - /// deadlock. However, if this method returns, then an exclusive lock is held. + /// If this file handle/descriptor, or a clone of it, already holds an lock the exact behavior + /// is unspecified and platform dependent, including the possibility that it will deadlock. + /// However, if this method returns, then an exclusive lock is held. /// /// If the file not open for writing, it is unspecified whether this function returns an error. /// - /// Note, this is an advisory lock meant to interact with [`lock_shared`], [`try_lock`], - /// [`try_lock_shared`], and [`unlock`]. Its interactions with other methods, such as [`read`] - /// and [`write`] are platform specific, and it may or may not cause non-lockholders to block. + /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], + /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with + /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not + /// cause non-lockholders to block. /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. @@ -650,6 +650,7 @@ impl File { /// /// [changes]: io#platform-specific-behavior /// + /// [`lock`]: File::lock /// [`lock_shared`]: File::lock_shared /// [`try_lock`]: File::try_lock /// [`try_lock_shared`]: File::try_lock_shared @@ -674,18 +675,19 @@ impl File { self.inner.lock() } - /// Acquire a shared (non-exclusive) advisory lock on the file. Blocks until the lock can be acquired. + /// Acquire a shared (non-exclusive) lock on the file. Blocks until the lock can be acquired. /// - /// This acquires a shared advisory lock; more than one file handle may hold a shared lock, but - /// none may hold an exclusive lock at the same time. + /// This acquires a shared lock; more than one file handle may hold a shared lock, but none may + /// hold an exclusive lock at the same time. /// - /// If this file handle/descriptor, or a clone of it, already holds an advisory lock, the exact - /// behavior is unspecified and platform dependent, including the possibility that it will - /// deadlock. However, if this method returns, then a shared lock is held. + /// If this file handle/descriptor, or a clone of it, already holds an lock, the exact behavior + /// is unspecified and platform dependent, including the possibility that it will deadlock. + /// However, if this method returns, then a shared lock is held. /// - /// Note, this is an advisory lock meant to interact with [`lock`], [`try_lock`], - /// [`try_lock_shared`], and [`unlock`]. Its interactions with other methods, such as [`read`] - /// and [`write`] are platform specific, and it may or may not cause non-lockholders to block. + /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], + /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with + /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not + /// cause non-lockholders to block. /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. @@ -699,6 +701,7 @@ impl File { /// [changes]: io#platform-specific-behavior /// /// [`lock`]: File::lock + /// [`lock_shared`]: File::lock_shared /// [`try_lock`]: File::try_lock /// [`try_lock_shared`]: File::try_lock_shared /// [`unlock`]: File::unlock @@ -722,24 +725,23 @@ impl File { self.inner.lock_shared() } - /// Try to acquire an exclusive advisory lock on the file. + /// Try to acquire an exclusive lock on the file. /// /// Returns `Ok(false)` if a different lock is already held on this file (via another /// handle/descriptor). /// - /// This acquires an exclusive advisory lock; no other file handle to this file may acquire - /// another lock. + /// This acquires an exclusive lock; no other file handle to this file may acquire another lock. /// - /// If this file handle/descriptor, or a clone of it, already holds an advisory lock, the exact - /// behavior is unspecified and platform dependent, including the possibility that it will - /// deadlock. However, if this method returns `Ok(true)`, then it has acquired an exclusive - /// lock. + /// If this file handle/descriptor, or a clone of it, already holds an lock, the exact behavior + /// is unspecified and platform dependent, including the possibility that it will deadlock. + /// However, if this method returns `Ok(true)`, then it has acquired an exclusive lock. /// /// If the file not open for writing, it is unspecified whether this function returns an error. /// - /// Note, this is an advisory lock meant to interact with [`lock`], [`lock_shared`], - /// [`try_lock_shared`], and [`unlock`]. Its interactions with other methods, such as [`read`] - /// and [`write`] are platform specific, and it may or may not cause non-lockholders to block. + /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], + /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with + /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not + /// cause non-lockholders to block. /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. @@ -755,6 +757,7 @@ impl File { /// /// [`lock`]: File::lock /// [`lock_shared`]: File::lock_shared + /// [`try_lock`]: File::try_lock /// [`try_lock_shared`]: File::try_lock_shared /// [`unlock`]: File::unlock /// [`read`]: Read::read @@ -777,21 +780,22 @@ impl File { self.inner.try_lock() } - /// Try to acquire a shared (non-exclusive) advisory lock on the file. + /// Try to acquire a shared (non-exclusive) lock on the file. /// /// Returns `Ok(false)` if an exclusive lock is already held on this file (via another /// handle/descriptor). /// - /// This acquires a shared advisory lock; more than one file handle may hold a shared lock, but - /// none may hold an exclusive lock at the same time. + /// This acquires a shared lock; more than one file handle may hold a shared lock, but none may + /// hold an exclusive lock at the same time. /// - /// If this file handle, or a clone of it, already holds an advisory lock, the exact behavior is + /// If this file handle, or a clone of it, already holds an lock, the exact behavior is /// unspecified and platform dependent, including the possibility that it will deadlock. /// However, if this method returns `Ok(true)`, then it has acquired a shared lock. /// - /// Note, this is an advisory lock meant to interact with [`lock`], [`try_lock`], - /// [`try_lock`], and [`unlock`]. Its interactions with other methods, such as [`read`] - /// and [`write`] are platform specific, and it may or may not cause non-lockholders to block. + /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], + /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with + /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not + /// cause non-lockholders to block. /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. @@ -808,6 +812,7 @@ impl File { /// [`lock`]: File::lock /// [`lock_shared`]: File::lock_shared /// [`try_lock`]: File::try_lock + /// [`try_lock_shared`]: File::try_lock_shared /// [`unlock`]: File::unlock /// [`read`]: Read::read /// [`write`]: Write::write From bc59397f8f320eefc5c2c60d1ad9fa816bbee2e4 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Tue, 11 Feb 2025 17:51:30 +0100 Subject: [PATCH 03/28] Document that locking a file fails on Windows if the file is opened only for append --- library/std/src/fs.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 7c8279f2555bd..0707115cbdd93 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -648,6 +648,9 @@ impl File { /// and the `LockFileEx` function on Windows with the `LOCKFILE_EXCLUSIVE_LOCK` flag. Note that, /// this [may change in the future][changes]. /// + /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, + /// open it with either `.read(true).append(true)` or `.write(true)`. + /// /// [changes]: io#platform-specific-behavior /// /// [`lock`]: File::lock @@ -698,6 +701,9 @@ impl File { /// and the `LockFileEx` function on Windows. Note that, this /// [may change in the future][changes]. /// + /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, + /// open it with either `.read(true).append(true)` or `.write(true)`. + /// /// [changes]: io#platform-specific-behavior /// /// [`lock`]: File::lock @@ -753,6 +759,9 @@ impl File { /// and `LOCKFILE_FAIL_IMMEDIATELY` flags. Note that, this /// [may change in the future][changes]. /// + /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, + /// open it with either `.read(true).append(true)` or `.write(true)`. + /// /// [changes]: io#platform-specific-behavior /// /// [`lock`]: File::lock @@ -807,6 +816,9 @@ impl File { /// `LOCKFILE_FAIL_IMMEDIATELY` flag. Note that, this /// [may change in the future][changes]. /// + /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, + /// open it with either `.read(true).append(true)` or `.write(true)`. + /// /// [changes]: io#platform-specific-behavior /// /// [`lock`]: File::lock @@ -849,6 +861,9 @@ impl File { /// and the `UnlockFile` function on Windows. Note that, this /// [may change in the future][changes]. /// + /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, + /// open it with either `.read(true).append(true)` or `.write(true)`. + /// /// [changes]: io#platform-specific-behavior /// /// # Examples From 345c313def0a559e7a29bc059c2e242ed398a760 Mon Sep 17 00:00:00 2001 From: may Date: Sun, 16 Feb 2025 12:02:06 +0100 Subject: [PATCH 04/28] fix docs for inherent str constructors --- library/core/src/str/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index 05c16791ce7ed..062f7dcc94029 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -198,7 +198,7 @@ impl str { /// Basic usage: /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// // some bytes, in a vector /// let sparkle_heart = vec![240, 159, 146, 150]; @@ -207,13 +207,13 @@ impl str { /// let sparkle_heart = str::from_utf8(&sparkle_heart)?; /// /// assert_eq!("💖", sparkle_heart); - /// # Ok::<_, str::Utf8Error>(()) + /// # Ok::<_, std::str::Utf8Error>(()) /// ``` /// /// Incorrect bytes: /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// // some invalid bytes, in a vector /// let sparkle_heart = vec![0, 159, 146, 150]; @@ -227,7 +227,7 @@ impl str { /// A "stack allocated string": /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// // some bytes, in a stack-allocated array /// let sparkle_heart = [240, 159, 146, 150]; @@ -249,7 +249,7 @@ impl str { /// Basic usage: /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// // "Hello, Rust!" as a mutable vector /// let mut hellorust = vec![72, 101, 108, 108, 111, 44, 32, 82, 117, 115, 116, 33]; @@ -263,7 +263,7 @@ impl str { /// Incorrect bytes: /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// // Some invalid bytes in a mutable vector /// let mut invalid = vec![128, 223]; @@ -292,7 +292,7 @@ impl str { /// Basic usage: /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// // some bytes, in a vector /// let sparkle_heart = vec![240, 159, 146, 150]; @@ -321,7 +321,7 @@ impl str { /// Basic usage: /// /// ``` - /// use std::str; + /// #![feature(inherent_str_constructors)] /// /// let mut heart = vec![240, 159, 146, 150]; /// let heart = unsafe { str::from_utf8_unchecked_mut(&mut heart) }; From 7af46307705f1ab76d3ad50a25cd8137766ea6b3 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 16 Feb 2025 19:17:52 -0800 Subject: [PATCH 05/28] add a failing test --- .../auxiliary/migration_lint_macros.rs | 7 +++++++ .../migration_lint.fixed | 5 +++++ .../migration_lint.rs | 5 +++++ .../migration_lint.stderr | 16 +++++++++++++++- 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/auxiliary/migration_lint_macros.rs b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/auxiliary/migration_lint_macros.rs index daa9b7368fd0e..b18f87fd56995 100644 --- a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/auxiliary/migration_lint_macros.rs +++ b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/auxiliary/migration_lint_macros.rs @@ -9,3 +9,10 @@ macro_rules! mixed_edition_pat { Some(mut $foo) }; } + +#[macro_export] +macro_rules! bind_ref { + ($foo:ident) => { + ref $foo + }; +} diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed index 0a22e939496e5..c81c7d2a87b87 100644 --- a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed +++ b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed @@ -239,4 +239,9 @@ fn main() { assert_type_eq(b, &0u32); assert_type_eq(c, &[0u32]); assert_type_eq(d, 0u32); + + // Test that we use the correct message and suggestion style when pointing inside expansions. + let [migration_lint_macros::bind_ref!(a)] = &[0]; + //~^ ERROR: binding modifiers may only be written when the default binding mode is `move` + assert_type_eq(a, &0u32); } diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.rs b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.rs index 7a6f2269d44a2..10a23e6f2fa18 100644 --- a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.rs +++ b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.rs @@ -239,4 +239,9 @@ fn main() { assert_type_eq(b, &0u32); assert_type_eq(c, &[0u32]); assert_type_eq(d, 0u32); + + // Test that we use the correct message and suggestion style when pointing inside expansions. + let [migration_lint_macros::bind_ref!(a)] = &[0]; + //~^ ERROR: binding modifiers may only be written when the default binding mode is `move` + assert_type_eq(a, &0u32); } diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr index 191800df07a2e..33256e8cbb9b0 100644 --- a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr +++ b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr @@ -562,5 +562,19 @@ help: make the implied reference patterns explicit LL | let [&Foo(&ref a @ [ref b]), &Foo(&ref c @ [d])] = [&Foo(&[0]); 2]; | + + -error: aborting due to 29 previous errors +error: reference patterns may only be written when the default binding mode is `move` + --> $DIR/migration_lint.rs:244:10 + | +LL | let [migration_lint_macros::bind_ref!(a)] = &[0]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ occurs within macro expansion + | + = note: for more information, see +note: matching on a reference type with a non-reference pattern changes the default binding mode + --> $DIR/migration_lint.rs:244:9 + | +LL | let [migration_lint_macros::bind_ref!(a)] = &[0]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_` + = note: this error originates in the macro `migration_lint_macros::bind_ref` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 30 previous errors From 82678df0de4369ca79164df93a56733a7538eac1 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 16 Feb 2025 19:27:48 -0800 Subject: [PATCH 06/28] bookkeep properly when pointing into macro expansions --- compiler/rustc_hir_typeck/src/pat.rs | 34 ++++++++++--------- .../migration_lint.fixed | 2 +- .../migration_lint.stderr | 6 +++- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 7c2a2b3fdf7da..c5c11c2bd2535 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -2806,31 +2806,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { && !self.tcx.features().ref_pat_eat_one_layer_2024_structural(), }); + let pat_kind = if let PatKind::Binding(user_bind_annot, _, _, _) = subpat.kind { + info.bad_modifiers = true; + // If the user-provided binding modifier doesn't match the default binding mode, we'll + // need to suggest reference patterns, which can affect other bindings. + // For simplicity, we opt to suggest making the pattern fully explicit. + info.suggest_eliding_modes &= + user_bind_annot == BindingMode(ByRef::Yes(def_br_mutbl), Mutability::Not); + "binding modifier" + } else { + info.bad_ref_pats = true; + // For simplicity, we don't try to suggest eliding reference patterns. Thus, we'll + // suggest adding them instead, which can affect the types assigned to bindings. + // As such, we opt to suggest making the pattern fully explicit. + info.suggest_eliding_modes = false; + "reference pattern" + }; // Only provide a detailed label if the problematic subpattern isn't from an expansion. // In the case that it's from a macro, we'll add a more detailed note in the emitter. let from_expansion = subpat.span.from_expansion(); let primary_label = if from_expansion { + // We can't suggest eliding modifiers within expansions. + info.suggest_eliding_modes = false; // NB: This wording assumes the only expansions that can produce problematic reference // patterns and bindings are macros. If a desugaring or AST pass is added that can do // so, we may want to inspect the span's source callee or macro backtrace. "occurs within macro expansion".to_owned() } else { - let pat_kind = if let PatKind::Binding(user_bind_annot, _, _, _) = subpat.kind { - info.bad_modifiers |= true; - // If the user-provided binding modifier doesn't match the default binding mode, we'll - // need to suggest reference patterns, which can affect other bindings. - // For simplicity, we opt to suggest making the pattern fully explicit. - info.suggest_eliding_modes &= - user_bind_annot == BindingMode(ByRef::Yes(def_br_mutbl), Mutability::Not); - "binding modifier" - } else { - info.bad_ref_pats |= true; - // For simplicity, we don't try to suggest eliding reference patterns. Thus, we'll - // suggest adding them instead, which can affect the types assigned to bindings. - // As such, we opt to suggest making the pattern fully explicit. - info.suggest_eliding_modes = false; - "reference pattern" - }; let dbm_str = match def_br_mutbl { Mutability::Not => "ref", Mutability::Mut => "ref mut", diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed index c81c7d2a87b87..e35896f32ad7d 100644 --- a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed +++ b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.fixed @@ -241,7 +241,7 @@ fn main() { assert_type_eq(d, 0u32); // Test that we use the correct message and suggestion style when pointing inside expansions. - let [migration_lint_macros::bind_ref!(a)] = &[0]; + let &[migration_lint_macros::bind_ref!(a)] = &[0]; //~^ ERROR: binding modifiers may only be written when the default binding mode is `move` assert_type_eq(a, &0u32); } diff --git a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr index 33256e8cbb9b0..3dd91c86a3b8f 100644 --- a/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr +++ b/tests/ui/pattern/rfc-3627-match-ergonomics-2024/migration_lint.stderr @@ -562,7 +562,7 @@ help: make the implied reference patterns explicit LL | let [&Foo(&ref a @ [ref b]), &Foo(&ref c @ [d])] = [&Foo(&[0]); 2]; | + + -error: reference patterns may only be written when the default binding mode is `move` +error: binding modifiers may only be written when the default binding mode is `move` --> $DIR/migration_lint.rs:244:10 | LL | let [migration_lint_macros::bind_ref!(a)] = &[0]; @@ -575,6 +575,10 @@ note: matching on a reference type with a non-reference pattern changes the defa LL | let [migration_lint_macros::bind_ref!(a)] = &[0]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_` = note: this error originates in the macro `migration_lint_macros::bind_ref` (in Nightly builds, run with -Z macro-backtrace for more info) +help: make the implied reference pattern explicit + | +LL | let &[migration_lint_macros::bind_ref!(a)] = &[0]; + | + error: aborting due to 30 previous errors From 2cdb7fac959381a44a409ae39d8d5cb4af988bb9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 14 Feb 2025 01:11:21 +0000 Subject: [PATCH 07/28] Prefer param-env candidates even when alias's trait bound isn't proven via param-env --- .../src/solve/assembly/mod.rs | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs b/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs index b0f59ed1474c2..bfb590e87679d 100644 --- a/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs @@ -791,7 +791,7 @@ where return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Ambiguity)); }; - let responses: Vec<_> = match proven_via { + match proven_via { // Even when a trait bound has been proven using a where-bound, we // still need to consider alias-bounds for normalization, see // tests/ui/next-solver/alias-bound-shadowed-by-env.rs. @@ -800,7 +800,7 @@ where // constness checking. Doing so is *at least theoretically* breaking, // see github.com/rust-lang/rust/issues/133044#issuecomment-2500709754 TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => { - let mut candidates_from_env: Vec<_> = candidates + let mut candidates_from_env_and_bounds: Vec<_> = candidates .iter() .filter(|c| { matches!( @@ -813,16 +813,37 @@ where // If the trait goal has been proven by using the environment, we want to treat // aliases as rigid if there are no applicable projection bounds in the environment. - if candidates_from_env.is_empty() { + if candidates_from_env_and_bounds.is_empty() { if let Ok(response) = inject_normalize_to_rigid_candidate(self) { - candidates_from_env.push(response); + candidates_from_env_and_bounds.push(response); } } - candidates_from_env + + if let Some(response) = self.try_merge_responses(&candidates_from_env_and_bounds) { + Ok(response) + } else { + self.flounder(&candidates_from_env_and_bounds) + } } - TraitGoalProvenVia::Misc => candidates.iter().map(|c| c.result).collect(), - }; + TraitGoalProvenVia::Misc => { + // Prefer "orphaned" param-env normalization predicates, which are used + // (for example, and ideally only) when proving item bounds for an impl. + let candidates_from_env: Vec<_> = candidates + .iter() + .filter(|c| matches!(c.source, CandidateSource::ParamEnv(_))) + .map(|c| c.result) + .collect(); + if let Some(response) = self.try_merge_responses(&candidates_from_env) { + return Ok(response); + } - self.try_merge_responses(&responses).map_or_else(|| self.flounder(&responses), Ok) + let responses: Vec<_> = candidates.iter().map(|c| c.result).collect(); + if let Some(response) = self.try_merge_responses(&responses) { + Ok(response) + } else { + self.flounder(&responses) + } + } + } } } From b002b5cc82b8138308c1aad791ae1f80ca6f5c44 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 14 Feb 2025 01:28:15 +0000 Subject: [PATCH 08/28] Deeply normalize associated type bounds before proving them --- .../src/check/compare_impl_item.rs | 64 +++++++------------ ...1883.stderr => issue-91883.current.stderr} | 6 +- .../issue-91883.next.stderr | 20 ++++++ .../generic-associated-types/issue-91883.rs | 4 ++ .../in-trait/default-body-with-rpit.rs | 3 + .../in-trait/nested-rpitit-bounds.rs | 3 + .../predicate-entailment-passes.rs | 11 ---- ...trait_ref_is_knowable-norm-overflow.stderr | 12 +--- tests/ui/traits/next-solver/gat-wf.rs | 16 +++++ tests/ui/traits/next-solver/gat-wf.stderr | 15 +++++ 10 files changed, 87 insertions(+), 67 deletions(-) rename tests/ui/generic-associated-types/{issue-91883.stderr => issue-91883.current.stderr} (90%) create mode 100644 tests/ui/generic-associated-types/issue-91883.next.stderr create mode 100644 tests/ui/traits/next-solver/gat-wf.rs create mode 100644 tests/ui/traits/next-solver/gat-wf.stderr diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 84d5ec4a1e5b0..79cf86191e0d7 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -2105,18 +2105,21 @@ pub(super) fn check_type_bounds<'tcx>( ObligationCause::new(impl_ty_span, impl_ty_def_id, code) }; - let mut obligations: Vec<_> = tcx - .explicit_item_bounds(trait_ty.def_id) - .iter_instantiated_copied(tcx, rebased_args) - .map(|(concrete_ty_bound, span)| { - debug!(?concrete_ty_bound); - traits::Obligation::new(tcx, mk_cause(span), param_env, concrete_ty_bound) - }) - .collect(); + let mut obligations: Vec<_> = util::elaborate( + tcx, + tcx.explicit_item_bounds(trait_ty.def_id).iter_instantiated_copied(tcx, rebased_args).map( + |(concrete_ty_bound, span)| { + debug!(?concrete_ty_bound); + traits::Obligation::new(tcx, mk_cause(span), param_env, concrete_ty_bound) + }, + ), + ) + .collect(); // Only in a const implementation do we need to check that the `~const` item bounds hold. if tcx.is_conditionally_const(impl_ty_def_id) { - obligations.extend( + obligations.extend(util::elaborate( + tcx, tcx.explicit_implied_const_bounds(trait_ty.def_id) .iter_instantiated_copied(tcx, rebased_args) .map(|(c, span)| { @@ -2127,7 +2130,7 @@ pub(super) fn check_type_bounds<'tcx>( c.to_host_effect_clause(tcx, ty::BoundConstness::Maybe), ) }), - ); + )); } debug!(item_bounds=?obligations); @@ -2135,26 +2138,19 @@ pub(super) fn check_type_bounds<'tcx>( // to its definition type. This should be the param-env we use to *prove* the // predicate too, but we don't do that because of performance issues. // See . - let trait_projection_ty = Ty::new_projection_from_args(tcx, trait_ty.def_id, rebased_args); - let impl_identity_ty = tcx.type_of(impl_ty.def_id).instantiate_identity(); let normalize_param_env = param_env_with_gat_bounds(tcx, impl_ty, impl_trait_ref); - for mut obligation in util::elaborate(tcx, obligations) { - let normalized_predicate = if infcx.next_trait_solver() { - obligation.predicate.fold_with(&mut ReplaceTy { - tcx, - from: trait_projection_ty, - to: impl_identity_ty, - }) - } else { - ocx.normalize(&normalize_cause, normalize_param_env, obligation.predicate) - }; - debug!(?normalized_predicate); - obligation.predicate = normalized_predicate; - - ocx.register_obligation(obligation); + for obligation in &mut obligations { + match ocx.deeply_normalize(&normalize_cause, normalize_param_env, obligation.predicate) { + Ok(pred) => obligation.predicate = pred, + Err(e) => { + return Err(infcx.err_ctxt().report_fulfillment_errors(e)); + } + } } + // Check that all obligations are satisfied by the implementation's // version. + ocx.register_obligations(obligations); let errors = ocx.select_all_or_error(); if !errors.is_empty() { let reported = infcx.err_ctxt().report_fulfillment_errors(errors); @@ -2166,22 +2162,6 @@ pub(super) fn check_type_bounds<'tcx>( ocx.resolve_regions_and_report_errors(impl_ty_def_id, param_env, assumed_wf_types) } -struct ReplaceTy<'tcx> { - tcx: TyCtxt<'tcx>, - from: Ty<'tcx>, - to: Ty<'tcx>, -} - -impl<'tcx> TypeFolder> for ReplaceTy<'tcx> { - fn cx(&self) -> TyCtxt<'tcx> { - self.tcx - } - - fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { - if self.from == ty { self.to } else { ty.super_fold_with(self) } - } -} - /// Install projection predicates that allow GATs to project to their own /// definition types. This is not allowed in general in cases of default /// associated types in trait definitions, or when specialization is involved, diff --git a/tests/ui/generic-associated-types/issue-91883.stderr b/tests/ui/generic-associated-types/issue-91883.current.stderr similarity index 90% rename from tests/ui/generic-associated-types/issue-91883.stderr rename to tests/ui/generic-associated-types/issue-91883.current.stderr index ac636ebb648cb..0741cf9581d58 100644 --- a/tests/ui/generic-associated-types/issue-91883.stderr +++ b/tests/ui/generic-associated-types/issue-91883.current.stderr @@ -1,5 +1,5 @@ error[E0478]: lifetime bound not satisfied - --> $DIR/issue-91883.rs:30:24 + --> $DIR/issue-91883.rs:34:24 | LL | type Cursor<'tx>: Cursor<'tx> | ----------------------------- definition of `Cursor` from trait @@ -8,12 +8,12 @@ LL | type Cursor<'tx> = CursorImpl<'tx>; | ^^^^^^^^^^^^^^^ | note: lifetime parameter instantiated with the lifetime `'db` as defined here - --> $DIR/issue-91883.rs:29:6 + --> $DIR/issue-91883.rs:33:6 | LL | impl<'db> Transaction<'db> for TransactionImpl<'db> { | ^^^ note: but lifetime parameter must outlive the lifetime `'tx` as defined here - --> $DIR/issue-91883.rs:30:17 + --> $DIR/issue-91883.rs:34:17 | LL | type Cursor<'tx> = CursorImpl<'tx>; | ^^^ diff --git a/tests/ui/generic-associated-types/issue-91883.next.stderr b/tests/ui/generic-associated-types/issue-91883.next.stderr new file mode 100644 index 0000000000000..b3ed2d81b63e8 --- /dev/null +++ b/tests/ui/generic-associated-types/issue-91883.next.stderr @@ -0,0 +1,20 @@ +error[E0478]: lifetime bound not satisfied + --> $DIR/issue-91883.rs:34:24 + | +LL | type Cursor<'tx> = CursorImpl<'tx>; + | ^^^^^^^^^^^^^^^ + | +note: lifetime parameter instantiated with the lifetime `'db` as defined here + --> $DIR/issue-91883.rs:33:6 + | +LL | impl<'db> Transaction<'db> for TransactionImpl<'db> { + | ^^^ +note: but lifetime parameter must outlive the lifetime `'tx` as defined here + --> $DIR/issue-91883.rs:34:17 + | +LL | type Cursor<'tx> = CursorImpl<'tx>; + | ^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0478`. diff --git a/tests/ui/generic-associated-types/issue-91883.rs b/tests/ui/generic-associated-types/issue-91883.rs index e870e08a3a2b0..d31e1736cf28f 100644 --- a/tests/ui/generic-associated-types/issue-91883.rs +++ b/tests/ui/generic-associated-types/issue-91883.rs @@ -1,3 +1,7 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver + use std::fmt::Debug; use std::marker::PhantomData; diff --git a/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs b/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs index c1a78bc23885c..1bbff839ffa56 100644 --- a/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs +++ b/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs @@ -1,5 +1,8 @@ //@ edition:2021 //@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver use std::fmt::Debug; diff --git a/tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs b/tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs index 10c2a81124346..6954a7d806726 100644 --- a/tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs +++ b/tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs @@ -1,4 +1,7 @@ //@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver use std::ops::Deref; diff --git a/tests/ui/traits/const-traits/predicate-entailment-passes.rs b/tests/ui/traits/const-traits/predicate-entailment-passes.rs index 9c8d5a5e3f6ad..28ae21891f386 100644 --- a/tests/ui/traits/const-traits/predicate-entailment-passes.rs +++ b/tests/ui/traits/const-traits/predicate-entailment-passes.rs @@ -6,32 +6,21 @@ #[const_trait] trait Bar {} impl const Bar for () {} - #[const_trait] trait TildeConst { - type Bar where T: ~const Bar; - fn foo() where T: ~const Bar; } impl TildeConst for () { - type Bar = () where T: Bar; - fn foo() where T: Bar {} } #[const_trait] trait AlwaysConst { - type Bar where T: const Bar; - fn foo() where T: const Bar; } impl AlwaysConst for i32 { - type Bar = () where T: Bar; - fn foo() where T: Bar {} } impl const AlwaysConst for u32 { - type Bar = () where T: ~const Bar; - fn foo() where T: ~const Bar {} } diff --git a/tests/ui/traits/next-solver/coherence/trait_ref_is_knowable-norm-overflow.stderr b/tests/ui/traits/next-solver/coherence/trait_ref_is_knowable-norm-overflow.stderr index 1d42dbdfe00e7..294fa0d7613c5 100644 --- a/tests/ui/traits/next-solver/coherence/trait_ref_is_knowable-norm-overflow.stderr +++ b/tests/ui/traits/next-solver/coherence/trait_ref_is_knowable-norm-overflow.stderr @@ -1,18 +1,8 @@ -error[E0275]: overflow evaluating the requirement `::Assoc: Sized` +error[E0275]: overflow evaluating the requirement `::Assoc == _` --> $DIR/trait_ref_is_knowable-norm-overflow.rs:10:18 | LL | type Assoc = ::Assoc; | ^^^^^^^^^^^^^^^^^^^^^^ - | -note: required by a bound in `Overflow::Assoc` - --> $DIR/trait_ref_is_knowable-norm-overflow.rs:7:5 - | -LL | type Assoc; - | ^^^^^^^^^^^ required by this bound in `Overflow::Assoc` -help: consider relaxing the implicit `Sized` restriction - | -LL | type Assoc: ?Sized; - | ++++++++ error[E0119]: conflicting implementations of trait `Trait` --> $DIR/trait_ref_is_knowable-norm-overflow.rs:18:1 diff --git a/tests/ui/traits/next-solver/gat-wf.rs b/tests/ui/traits/next-solver/gat-wf.rs new file mode 100644 index 0000000000000..ff6e2665ef3ee --- /dev/null +++ b/tests/ui/traits/next-solver/gat-wf.rs @@ -0,0 +1,16 @@ +//@ compile-flags: -Znext-solver + +// Make sure that, like the old trait solver, we end up requiring that the WC of +// impl GAT matches that of the trait. This is not a restriction that we *need*, +// but is a side-effect of registering the where clauses when normalizing the GAT +// when proving it satisfies its item bounds. + +trait Foo { + type T<'a>: Sized where Self: 'a; +} + +impl Foo for &() { + type T<'a> = (); //~ the type `&()` does not fulfill the required lifetime +} + +fn main() {} diff --git a/tests/ui/traits/next-solver/gat-wf.stderr b/tests/ui/traits/next-solver/gat-wf.stderr new file mode 100644 index 0000000000000..620bca77e4b97 --- /dev/null +++ b/tests/ui/traits/next-solver/gat-wf.stderr @@ -0,0 +1,15 @@ +error[E0477]: the type `&()` does not fulfill the required lifetime + --> $DIR/gat-wf.rs:13:18 + | +LL | type T<'a> = (); + | ^^ + | +note: type must outlive the lifetime `'a` as defined here + --> $DIR/gat-wf.rs:13:12 + | +LL | type T<'a> = (); + | ^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0477`. From a7a43cd37308b774e4d562f1dbaf2793461ae09e Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Wed, 5 Feb 2025 12:54:31 +0000 Subject: [PATCH 09/28] use `Iterator::zip` instead of enumerating+indexing --- src/librustdoc/clean/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 92ef3ab7a1d47..dcc5fd12d81bf 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1150,9 +1150,9 @@ fn clean_args_from_types_and_body_id<'tcx>( Arguments { values: types .iter() - .enumerate() - .map(|(i, ty)| Argument { - name: name_from_pat(body.params[i].pat), + .zip(body.params) + .map(|(ty, param)| Argument { + name: name_from_pat(param.pat), type_: clean_ty(ty, cx), is_const: false, }) From 4a76615054efaea8a7c0a77ec4b77277da7c64be Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Wed, 5 Feb 2025 13:03:12 +0000 Subject: [PATCH 10/28] coalesce match patterns with identical bodies --- src/librustdoc/clean/utils.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 8a7d140bb1a69..418e60fd97254 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -299,10 +299,15 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol { Symbol::intern(&match p.kind { // FIXME(never_patterns): does this make sense? - PatKind::Wild | PatKind::Err(_) | PatKind::Never | PatKind::Struct(..) => { + PatKind::Wild + | PatKind::Err(_) + | PatKind::Never + | PatKind::Struct(..) + | PatKind::Range(..) => { return kw::Underscore; } PatKind::Binding(_, _, ident, _) => return ident.name, + PatKind::Box(p) | PatKind::Ref(p, _) | PatKind::Guard(p, _) => return name_from_pat(p), PatKind::TupleStruct(ref p, ..) | PatKind::Expr(PatExpr { kind: PatExprKind::Path(ref p), .. }) => qpath_to_string(p), PatKind::Or(pats) => { @@ -312,17 +317,13 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol { "({})", elts.iter().map(|p| name_from_pat(p).to_string()).collect::>().join(", ") ), - PatKind::Box(p) => return name_from_pat(p), PatKind::Deref(p) => format!("deref!({})", name_from_pat(p)), - PatKind::Ref(p, _) => return name_from_pat(p), PatKind::Expr(..) => { warn!( "tried to get argument name from PatKind::Expr, which is silly in function arguments" ); return Symbol::intern("()"); } - PatKind::Guard(p, _) => return name_from_pat(p), - PatKind::Range(..) => return kw::Underscore, PatKind::Slice(begin, ref mid, end) => { let begin = begin.iter().map(|p| name_from_pat(p).to_string()); let mid = mid.as_ref().map(|p| format!("..{}", name_from_pat(p))).into_iter(); From 32b1bffa11640f3ad88c4761e0ec9e5546940159 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 17 Feb 2025 10:47:48 -0800 Subject: [PATCH 11/28] Update mdbook to 0.4.45 Changelog: https://github.com/rust-lang/mdBook/blob/master/CHANGELOG.md#mdbook-0445 --- src/tools/rustbook/Cargo.lock | 4 ++-- src/tools/rustbook/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/rustbook/Cargo.lock b/src/tools/rustbook/Cargo.lock index b31bf61a6fbad..91f72e0d007fc 100644 --- a/src/tools/rustbook/Cargo.lock +++ b/src/tools/rustbook/Cargo.lock @@ -867,9 +867,9 @@ dependencies = [ [[package]] name = "mdbook" -version = "0.4.44" +version = "0.4.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9da1e54401fe5d45a664c57e112e70f18e8c5a73e268c179305b932ee864574" +checksum = "b07d36d96ffe1b5b16ddf2bc80b3b26bb7a498b2a6591061250bf0af8e8095ad" dependencies = [ "ammonia", "anyhow", diff --git a/src/tools/rustbook/Cargo.toml b/src/tools/rustbook/Cargo.toml index 9f9846cdee07d..c9d4d41d1155e 100644 --- a/src/tools/rustbook/Cargo.toml +++ b/src/tools/rustbook/Cargo.toml @@ -14,6 +14,6 @@ mdbook-i18n-helpers = "0.3.3" mdbook-spec = { path = "../../doc/reference/mdbook-spec" } [dependencies.mdbook] -version = "0.4.44" +version = "0.4.45" default-features = false features = ["search"] From 2773456f5f3560bb0590d32452ca58f21b0181d0 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 17 Feb 2025 10:57:51 -0800 Subject: [PATCH 12/28] Move error_index_generator to the rustbook workspace I had forgotten that error_index_generator is using mdbook. This moves it to be part of the rustbook workspace so that it can share the dependency with rustbook. --- Cargo.lock | 194 +-------------------- Cargo.toml | 1 - src/bootstrap/src/utils/proc_macro_deps.rs | 15 -- src/tools/error_index_generator/Cargo.toml | 1 + src/tools/rustbook/Cargo.lock | 7 + src/tools/rustbook/Cargo.toml | 1 + src/tools/tidy/src/deps.rs | 1 - 7 files changed, 10 insertions(+), 210 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e9ee3c6b36ec1..bb17717aceb42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -61,19 +61,6 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" -[[package]] -name = "ammonia" -version = "4.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ab99eae5ee58501ab236beb6f20f6ca39be615267b014899c89b2f0bc18a459" -dependencies = [ - "html5ever", - "maplit", - "once_cell", - "tendril", - "url", -] - [[package]] name = "android-tzdata" version = "0.1.1" @@ -513,16 +500,6 @@ dependencies = [ "anstyle", "clap_lex", "strsim", - "terminal_size", -] - -[[package]] -name = "clap_complete" -version = "4.5.42" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33a7e468e750fa4b6be660e8b5651ad47372e8fb114030b594c2d75d48c5ffd0" -dependencies = [ - "clap", ] [[package]] @@ -1084,18 +1061,6 @@ version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" -[[package]] -name = "elasticlunr-rs" -version = "3.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41e83863a500656dfa214fee6682de9c5b9f03de6860fec531235ed2ae9f6571" -dependencies = [ - "regex", - "serde", - "serde_derive", - "serde_json", -] - [[package]] name = "elsa" version = "1.11.0" @@ -1159,13 +1124,6 @@ dependencies = [ "windows-sys 0.59.0", ] -[[package]] -name = "error_index_generator" -version = "0.0.0" -dependencies = [ - "mdbook", -] - [[package]] name = "expect-test" version = "1.5.1" @@ -1517,22 +1475,6 @@ dependencies = [ "serde", ] -[[package]] -name = "handlebars" -version = "6.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d6b224b95c1e668ac0270325ad563b2eef1469fbbb8959bc7c692c844b813d9" -dependencies = [ - "derive_builder", - "log", - "num-order", - "pest", - "pest_derive", - "serde", - "serde_json", - "thiserror 2.0.11", -] - [[package]] name = "hashbrown" version = "0.14.5" @@ -2189,12 +2131,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" -[[package]] -name = "maplit" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d" - [[package]] name = "markup5ever" version = "0.12.1" @@ -2228,34 +2164,6 @@ dependencies = [ "digest", ] -[[package]] -name = "mdbook" -version = "0.4.43" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe1f98b8d66e537d2f0ba06e7dec4f44001deec539a2d18bfc102d6a86189148" -dependencies = [ - "ammonia", - "anyhow", - "chrono", - "clap", - "clap_complete", - "elasticlunr-rs", - "env_logger", - "handlebars", - "log", - "memchr", - "once_cell", - "opener", - "pulldown-cmark 0.10.3", - "regex", - "serde", - "serde_json", - "shlex", - "tempfile", - "toml 0.5.11", - "topological-sort", -] - [[package]] name = "measureme" version = "11.0.1" @@ -2483,21 +2391,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "num-modular" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17bb261bf36fa7d83f4c294f834e91256769097b3cb505d44831e0a179ac647f" - -[[package]] -name = "num-order" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "537b596b97c40fcf8056d153049eb22f481c17ebce72a513ec9286e4986d1bb6" -dependencies = [ - "num-modular", -] - [[package]] name = "num-rational" version = "0.4.2" @@ -2718,51 +2611,6 @@ dependencies = [ "libc", ] -[[package]] -name = "pest" -version = "2.7.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b7cafe60d6cf8e62e1b9b2ea516a089c008945bb5a275416789e7db0bc199dc" -dependencies = [ - "memchr", - "thiserror 2.0.11", - "ucd-trie", -] - -[[package]] -name = "pest_derive" -version = "2.7.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "816518421cfc6887a0d62bf441b6ffb4536fcc926395a69e1a85852d4363f57e" -dependencies = [ - "pest", - "pest_generator", -] - -[[package]] -name = "pest_generator" -version = "2.7.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d1396fd3a870fc7838768d171b4616d5c91f6cc25e377b673d714567d99377b" -dependencies = [ - "pest", - "pest_meta", - "proc-macro2", - "quote", - "syn 2.0.96", -] - -[[package]] -name = "pest_meta" -version = "2.7.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1e58089ea25d717bfd31fb534e4f3afcc2cc569c70de3e239778991ea3b7dea" -dependencies = [ - "once_cell", - "pest", - "sha2", -] - [[package]] name = "phf" version = "0.11.3" @@ -2921,18 +2769,6 @@ dependencies = [ "unicase", ] -[[package]] -name = "pulldown-cmark" -version = "0.10.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76979bea66e7875e7509c4ec5300112b316af87fa7a252ca91c448b32dfe3993" -dependencies = [ - "bitflags", - "memchr", - "pulldown-cmark-escape 0.10.1", - "unicase", -] - [[package]] name = "pulldown-cmark" version = "0.11.3" @@ -2941,16 +2777,10 @@ checksum = "679341d22c78c6c649893cbd6c3278dcbe9fc4faa62fea3a9296ae2b50c14625" dependencies = [ "bitflags", "memchr", - "pulldown-cmark-escape 0.11.0", + "pulldown-cmark-escape", "unicase", ] -[[package]] -name = "pulldown-cmark-escape" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd348ff538bc9caeda7ee8cad2d1d48236a1f443c1fa3913c6a02fe0043b1dd3" - [[package]] name = "pulldown-cmark-escape" version = "0.11.0" @@ -5310,16 +5140,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "terminal_size" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5352447f921fda68cf61b4101566c0bdb5104eff6804d0678e5227580ab6a4e9" -dependencies = [ - "rustix", - "windows-sys 0.59.0", -] - [[package]] name = "termize" version = "0.1.1" @@ -5560,12 +5380,6 @@ dependencies = [ "winnow", ] -[[package]] -name = "topological-sort" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea68304e134ecd095ac6c3574494fc62b909f416c4fca77e440530221e549d3d" - [[package]] name = "tracing" version = "0.1.37" @@ -5686,12 +5500,6 @@ dependencies = [ "regex-lite", ] -[[package]] -name = "ucd-trie" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2896d95c02a80c6d6a5d6e953d479f5ddf2dfdb6a244441010e373ac0fb88971" - [[package]] name = "ui_test" version = "0.26.5" diff --git a/Cargo.toml b/Cargo.toml index 97e782d0df020..20a43aaaeeb37 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ members = [ "src/tools/clippy/clippy_dev", "src/tools/compiletest", "src/tools/run-make-support", - "src/tools/error_index_generator", "src/tools/linkchecker", "src/tools/lint-docs", "src/tools/miropt-test-tools", diff --git a/src/bootstrap/src/utils/proc_macro_deps.rs b/src/bootstrap/src/utils/proc_macro_deps.rs index dbfd6f47dc67f..34bf6bb7013ff 100644 --- a/src/bootstrap/src/utils/proc_macro_deps.rs +++ b/src/bootstrap/src/utils/proc_macro_deps.rs @@ -6,25 +6,18 @@ pub static CRATES: &[&str] = &[ "annotate-snippets", "anstyle", "basic-toml", - "block-buffer", "bumpalo", - "cfg-if", - "cpufeatures", - "crypto-common", "darling", "darling_core", "derive_builder_core", - "digest", "fluent-bundle", "fluent-langneg", "fluent-syntax", "fnv", - "generic-array", "heck", "ident_case", "intl-memoizer", "intl_pluralrules", - "libc", "log", "memchr", "mime", @@ -32,17 +25,12 @@ pub static CRATES: &[&str] = &[ "minimal-lexical", "nom", "num-conv", - "once_cell", - "pest", - "pest_generator", - "pest_meta", "proc-macro2", "quote", "rinja_parser", "rustc-hash", "self_cell", "serde", - "sha2", "smallvec", "stable_deref_trait", "strsim", @@ -52,15 +40,12 @@ pub static CRATES: &[&str] = &[ "time-core", "tinystr", "type-map", - "typenum", - "ucd-trie", "unic-langid", "unic-langid-impl", "unic-langid-macros", "unicase", "unicode-ident", "unicode-width", - "version_check", "wasm-bindgen-backend", "wasm-bindgen-macro-support", "wasm-bindgen-shared", diff --git a/src/tools/error_index_generator/Cargo.toml b/src/tools/error_index_generator/Cargo.toml index f4dac6e947e32..54fe7f6eb5a9b 100644 --- a/src/tools/error_index_generator/Cargo.toml +++ b/src/tools/error_index_generator/Cargo.toml @@ -2,6 +2,7 @@ name = "error_index_generator" version = "0.0.0" edition = "2021" +workspace = "../rustbook" [dependencies] mdbook = { version = "0.4", default-features = false, features = ["search"] } diff --git a/src/tools/rustbook/Cargo.lock b/src/tools/rustbook/Cargo.lock index 91f72e0d007fc..ddcf315a2672d 100644 --- a/src/tools/rustbook/Cargo.lock +++ b/src/tools/rustbook/Cargo.lock @@ -446,6 +446,13 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "error_index_generator" +version = "0.0.0" +dependencies = [ + "mdbook", +] + [[package]] name = "fastrand" version = "2.3.0" diff --git a/src/tools/rustbook/Cargo.toml b/src/tools/rustbook/Cargo.toml index c9d4d41d1155e..6aec0bec0fa09 100644 --- a/src/tools/rustbook/Cargo.toml +++ b/src/tools/rustbook/Cargo.toml @@ -1,4 +1,5 @@ [workspace] +members = ["../error_index_generator"] [package] name = "rustbook" diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index e8e7dfe0d84c1..bfff5a8e17e13 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -99,7 +99,6 @@ const EXCEPTIONS: ExceptionList = &[ ("dissimilar", "Apache-2.0"), // rustdoc, rustc_lexer (few tests) via expect-test, (dev deps) ("fluent-langneg", "Apache-2.0"), // rustc (fluent translations) ("foldhash", "Zlib"), // rustc - ("mdbook", "MPL-2.0"), // mdbook ("option-ext", "MPL-2.0"), // cargo-miri (via `directories`) ("rustc_apfloat", "Apache-2.0 WITH LLVM-exception"), // rustc (license is the same as LLVM uses) ("ryu", "Apache-2.0 OR BSL-1.0"), // BSL is not acceptble, but we use it under Apache-2.0 // cargo/... (because of serde) From 123217b3b8233ba0cc852f81fe6ca55c47bed8cc Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 17 Feb 2025 10:58:57 -0800 Subject: [PATCH 13/28] Fix what looks like an inverted message I believe this is trying to say there is something that is in the file, but shouldn't be. --- src/tools/tidy/src/deps.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index bfff5a8e17e13..51e58b4e4fcfd 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -666,7 +666,7 @@ pub static CRATES: &[&str] = &[ for extra in expected.difference(&proc_macro_deps) { tidy_error!( bad, - "`{extra}` is not registered in `src/bootstrap/src/utils/proc_macro_deps.rs`, but is not a proc-macro crate dependency", + "`{extra}` is registered in `src/bootstrap/src/utils/proc_macro_deps.rs`, but is not a proc-macro crate dependency", ); } if *bad != old_bad { From c0b1c6eed516ca39a88dcd7d6a11051fe645fd4a Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Wed, 5 Feb 2025 17:48:26 +0000 Subject: [PATCH 14/28] librustdoc: more usages of `Joined::joined` --- src/librustdoc/clean/utils.rs | 60 +++++++++++++++--------- src/librustdoc/doctest/make.rs | 16 ++++--- src/librustdoc/html/render/print_item.rs | 28 +++++------ 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 418e60fd97254..c69d22001d9b4 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -1,5 +1,5 @@ use std::assert_matches::debug_assert_matches; -use std::fmt::Write as _; +use std::fmt::{self, Display, Write as _}; use std::mem; use std::sync::LazyLock as Lazy; @@ -24,6 +24,7 @@ use crate::clean::{ clean_middle_ty, inline, }; use crate::core::DocContext; +use crate::display::Joined as _; #[cfg(test)] mod tests; @@ -250,16 +251,20 @@ pub(crate) fn qpath_to_string(p: &hir::QPath<'_>) -> String { hir::QPath::LangItem(lang_item, ..) => return lang_item.name().to_string(), }; - let mut s = String::new(); - for (i, seg) in segments.iter().enumerate() { - if i > 0 { - s.push_str("::"); - } - if seg.ident.name != kw::PathRoot { - s.push_str(seg.ident.as_str()); - } - } - s + fmt::from_fn(|f| { + segments + .iter() + .map(|seg| { + fmt::from_fn(|f| { + if seg.ident.name != kw::PathRoot { + write!(f, "{}", seg.ident)?; + } + Ok(()) + }) + }) + .joined("::", f) + }) + .to_string() } pub(crate) fn build_deref_target_impls( @@ -311,12 +316,11 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol { PatKind::TupleStruct(ref p, ..) | PatKind::Expr(PatExpr { kind: PatExprKind::Path(ref p), .. }) => qpath_to_string(p), PatKind::Or(pats) => { - pats.iter().map(|p| name_from_pat(p).to_string()).collect::>().join(" | ") + fmt::from_fn(|f| pats.iter().map(|p| name_from_pat(p)).joined(" | ", f)).to_string() + } + PatKind::Tuple(elts, _) => { + format!("({})", fmt::from_fn(|f| elts.iter().map(|p| name_from_pat(p)).joined(", ", f))) } - PatKind::Tuple(elts, _) => format!( - "({})", - elts.iter().map(|p| name_from_pat(p).to_string()).collect::>().join(", ") - ), PatKind::Deref(p) => format!("deref!({})", name_from_pat(p)), PatKind::Expr(..) => { warn!( @@ -324,11 +328,25 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol { ); return Symbol::intern("()"); } - PatKind::Slice(begin, ref mid, end) => { - let begin = begin.iter().map(|p| name_from_pat(p).to_string()); - let mid = mid.as_ref().map(|p| format!("..{}", name_from_pat(p))).into_iter(); - let end = end.iter().map(|p| name_from_pat(p).to_string()); - format!("[{}]", begin.chain(mid).chain(end).collect::>().join(", ")) + PatKind::Slice(begin, mid, end) => { + fn print_pat<'a>(pat: &'a Pat<'a>, wild: bool) -> impl Display + 'a { + fmt::from_fn(move |f| { + if wild { + f.write_str("..")?; + } + name_from_pat(pat).fmt(f) + }) + } + + format!( + "[{}]", + fmt::from_fn(|f| { + let begin = begin.iter().map(|p| print_pat(p, false)); + let mid = mid.map(|p| print_pat(p, true)); + let end = end.iter().map(|p| print_pat(p, false)); + begin.chain(mid).chain(end).joined(", ", f) + }) + ) } }) } diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index d89caabefe3e0..4792bc525a592 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -1,6 +1,7 @@ //! Logic for transforming the raw code given by the user into something actually //! runnable, e.g. by adding a `main` function if it doesn't already exist. +use std::fmt::{self, Write as _}; use std::io; use std::sync::Arc; @@ -17,6 +18,7 @@ use rustc_span::symbol::sym; use tracing::debug; use super::GlobalTestOptions; +use crate::display::Joined as _; use crate::html::markdown::LangString; /// This struct contains information about the doctest itself which is then used to generate @@ -232,13 +234,15 @@ impl DocTestBuilder { // add extra 4 spaces for each line to offset the code block if opts.insert_indent_space { - prog.push_str( - &everything_else + write!( + prog, + "{}", + fmt::from_fn(|f| everything_else .lines() - .map(|line| format!(" {}", line)) - .collect::>() - .join("\n"), - ); + .map(|line| fmt::from_fn(move |f| write!(f, " {line}"))) + .joined("\n", f)) + ) + .unwrap(); } else { prog.push_str(everything_else); }; diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index d2d7415261b9f..f320114703996 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -2,7 +2,6 @@ use std::cmp::Ordering; use std::fmt; use std::fmt::Display; -use itertools::Itertools; use rinja::Template; use rustc_abi::VariantIdx; use rustc_data_structures::captures::Captures; @@ -514,11 +513,7 @@ fn item_module(w: &mut String, cx: &Context<'_>, item: &clean::Item, items: &[cl class = myitem.type_(), unsafety_flag = unsafety_flag, href = item_path(myitem.type_(), myitem.name.unwrap().as_str()), - title = [myitem.type_().to_string(), full_path(cx, myitem)] - .iter() - .filter_map(|s| if !s.is_empty() { Some(s.as_str()) } else { None }) - .collect::>() - .join(" "), + title = format_args!("{} {}", myitem.type_(), full_path(cx, myitem)), ), ); } @@ -915,7 +910,7 @@ fn item_trait(w: &mut String, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra w, format_args!( "
At least one of the `{}` methods is required.
", - list.iter().join("`, `") + fmt::from_fn(|f| list.iter().joined("`, `", f)) ), ); } @@ -1168,17 +1163,18 @@ fn item_trait(w: &mut String, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra js_src_path.extend(cx.current.iter().copied()); js_src_path.push_fmt(format_args!("{}.{}.js", it.type_(), it.name.unwrap())); } - let extern_crates = extern_crates - .into_iter() - .map(|cnum| tcx.crate_name(cnum).to_string()) - .collect::>() - .join(","); - let (extern_before, extern_after) = - if extern_crates.is_empty() { ("", "") } else { (" data-ignore-extern-crates=\"", "\"") }; + let extern_crates = fmt::from_fn(|f| { + if !extern_crates.is_empty() { + f.write_str(" data-ignore-extern-crates=\"")?; + extern_crates.iter().map(|&cnum| tcx.crate_name(cnum)).joined(",", f)?; + f.write_str("\"")?; + } + Ok(()) + }); write_str( w, format_args!( - "", + "", src = js_src_path.finish() ), ); @@ -1400,7 +1396,7 @@ fn item_type_alias(w: &mut String, cx: &Context<'_>, it: &clean::Item, t: &clean .collect(); js_src_path.extend(target_fqp[..target_fqp.len() - 1].iter().copied()); js_src_path.push_fmt(format_args!("{target_type}.{}.js", target_fqp.last().unwrap())); - let self_path = self_fqp.iter().map(Symbol::as_str).collect::>().join("::"); + let self_path = fmt::from_fn(|f| self_fqp.iter().joined("::", f)); write_str( w, format_args!( From a1daa34ad005d0b34d30c878cb4e2e995346d300 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 11:24:57 +1100 Subject: [PATCH 15/28] Use `MirPatch` in `EnumSizeOpt`. Instead of `expand_statements`. This makes the code shorter and consistent with other MIR transform passes. The tests require updating because there is a slight change in MIR output: - the old code replaced the original statement with twelve new statements. - the new code inserts converts the original statement to a `nop` and then insert twelve new statements in front of it. I.e. we now end up with an extra `nop`, which doesn't matter at all. --- .../rustc_mir_transform/src/large_enums.rs | 186 +++++++----------- .../enum_opt.cand.EnumSizeOpt.32bit.diff | 2 + .../enum_opt.cand.EnumSizeOpt.64bit.diff | 2 + .../enum_opt.unin.EnumSizeOpt.32bit.diff | 2 + .../enum_opt.unin.EnumSizeOpt.64bit.diff | 2 + 5 files changed, 82 insertions(+), 112 deletions(-) diff --git a/compiler/rustc_mir_transform/src/large_enums.rs b/compiler/rustc_mir_transform/src/large_enums.rs index 1e546bfbeb303..47cb478fe33ee 100644 --- a/compiler/rustc_mir_transform/src/large_enums.rs +++ b/compiler/rustc_mir_transform/src/large_enums.rs @@ -6,6 +6,8 @@ use rustc_middle::ty::util::IntTypeExt; use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt}; use rustc_session::Session; +use crate::patch::MirPatch; + /// A pass that seeks to optimize unnecessary moves of large enum types, if there is a large /// enough discrepancy between them. /// @@ -41,31 +43,34 @@ impl<'tcx> crate::MirPass<'tcx> for EnumSizeOpt { let mut alloc_cache = FxHashMap::default(); let typing_env = body.typing_env(tcx); - let blocks = body.basic_blocks.as_mut(); - let local_decls = &mut body.local_decls; + let mut patch = MirPatch::new(body); - for bb in blocks { - bb.expand_statements(|st| { + for (block, data) in body.basic_blocks.as_mut().iter_enumerated_mut() { + for (statement_index, st) in data.statements.iter_mut().enumerate() { let StatementKind::Assign(box ( lhs, Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)), )) = &st.kind else { - return None; + continue; }; - let ty = lhs.ty(local_decls, tcx).ty; + let location = Location { block, statement_index }; - let (adt_def, num_variants, alloc_id) = - self.candidate(tcx, typing_env, ty, &mut alloc_cache)?; + let ty = lhs.ty(&body.local_decls, tcx).ty; - let source_info = st.source_info; - let span = source_info.span; + let Some((adt_def, num_variants, alloc_id)) = + self.candidate(tcx, typing_env, ty, &mut alloc_cache) + else { + continue; + }; + + let span = st.source_info.span; let tmp_ty = Ty::new_array(tcx, tcx.types.usize, num_variants as u64); - let size_array_local = local_decls.push(LocalDecl::new(tmp_ty, span)); - let store_live = - Statement { source_info, kind: StatementKind::StorageLive(size_array_local) }; + let size_array_local = patch.new_temp(tmp_ty, span); + + let store_live = StatementKind::StorageLive(size_array_local); let place = Place::from(size_array_local); let constant_vals = ConstOperand { @@ -77,108 +82,63 @@ impl<'tcx> crate::MirPass<'tcx> for EnumSizeOpt { ), }; let rval = Rvalue::Use(Operand::Constant(Box::new(constant_vals))); - let const_assign = - Statement { source_info, kind: StatementKind::Assign(Box::new((place, rval))) }; - - let discr_place = Place::from( - local_decls.push(LocalDecl::new(adt_def.repr().discr_type().to_ty(tcx), span)), - ); - let store_discr = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - discr_place, - Rvalue::Discriminant(*rhs), - ))), - }; - - let discr_cast_place = - Place::from(local_decls.push(LocalDecl::new(tcx.types.usize, span))); - let cast_discr = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - discr_cast_place, - Rvalue::Cast( - CastKind::IntToInt, - Operand::Copy(discr_place), - tcx.types.usize, - ), - ))), - }; - - let size_place = - Place::from(local_decls.push(LocalDecl::new(tcx.types.usize, span))); - let store_size = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - size_place, - Rvalue::Use(Operand::Copy(Place { - local: size_array_local, - projection: tcx - .mk_place_elems(&[PlaceElem::Index(discr_cast_place.local)]), - })), - ))), - }; - - let dst = - Place::from(local_decls.push(LocalDecl::new(Ty::new_mut_ptr(tcx, ty), span))); - let dst_ptr = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - dst, - Rvalue::RawPtr(RawPtrKind::Mut, *lhs), - ))), - }; + let const_assign = StatementKind::Assign(Box::new((place, rval))); + + let discr_place = + Place::from(patch.new_temp(adt_def.repr().discr_type().to_ty(tcx), span)); + let store_discr = + StatementKind::Assign(Box::new((discr_place, Rvalue::Discriminant(*rhs)))); + + let discr_cast_place = Place::from(patch.new_temp(tcx.types.usize, span)); + let cast_discr = StatementKind::Assign(Box::new(( + discr_cast_place, + Rvalue::Cast(CastKind::IntToInt, Operand::Copy(discr_place), tcx.types.usize), + ))); + + let size_place = Place::from(patch.new_temp(tcx.types.usize, span)); + let store_size = StatementKind::Assign(Box::new(( + size_place, + Rvalue::Use(Operand::Copy(Place { + local: size_array_local, + projection: tcx.mk_place_elems(&[PlaceElem::Index(discr_cast_place.local)]), + })), + ))); + + let dst = Place::from(patch.new_temp(Ty::new_mut_ptr(tcx, ty), span)); + let dst_ptr = + StatementKind::Assign(Box::new((dst, Rvalue::RawPtr(RawPtrKind::Mut, *lhs)))); let dst_cast_ty = Ty::new_mut_ptr(tcx, tcx.types.u8); - let dst_cast_place = - Place::from(local_decls.push(LocalDecl::new(dst_cast_ty, span))); - let dst_cast = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - dst_cast_place, - Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(dst), dst_cast_ty), - ))), - }; + let dst_cast_place = Place::from(patch.new_temp(dst_cast_ty, span)); + let dst_cast = StatementKind::Assign(Box::new(( + dst_cast_place, + Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(dst), dst_cast_ty), + ))); - let src = - Place::from(local_decls.push(LocalDecl::new(Ty::new_imm_ptr(tcx, ty), span))); - let src_ptr = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - src, - Rvalue::RawPtr(RawPtrKind::Const, *rhs), - ))), - }; + let src = Place::from(patch.new_temp(Ty::new_imm_ptr(tcx, ty), span)); + let src_ptr = + StatementKind::Assign(Box::new((src, Rvalue::RawPtr(RawPtrKind::Const, *rhs)))); let src_cast_ty = Ty::new_imm_ptr(tcx, tcx.types.u8); - let src_cast_place = - Place::from(local_decls.push(LocalDecl::new(src_cast_ty, span))); - let src_cast = Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - src_cast_place, - Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(src), src_cast_ty), - ))), - }; + let src_cast_place = Place::from(patch.new_temp(src_cast_ty, span)); + let src_cast = StatementKind::Assign(Box::new(( + src_cast_place, + Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(src), src_cast_ty), + ))); - let deinit_old = - Statement { source_info, kind: StatementKind::Deinit(Box::new(dst)) }; - - let copy_bytes = Statement { - source_info, - kind: StatementKind::Intrinsic(Box::new( - NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping { - src: Operand::Copy(src_cast_place), - dst: Operand::Copy(dst_cast_place), - count: Operand::Copy(size_place), - }), - )), - }; + let deinit_old = StatementKind::Deinit(Box::new(dst)); + + let copy_bytes = StatementKind::Intrinsic(Box::new( + NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping { + src: Operand::Copy(src_cast_place), + dst: Operand::Copy(dst_cast_place), + count: Operand::Copy(size_place), + }), + )); - let store_dead = - Statement { source_info, kind: StatementKind::StorageDead(size_array_local) }; + let store_dead = StatementKind::StorageDead(size_array_local); - let iter = [ + let stmts = [ store_live, const_assign, store_discr, @@ -191,14 +151,16 @@ impl<'tcx> crate::MirPass<'tcx> for EnumSizeOpt { deinit_old, copy_bytes, store_dead, - ] - .into_iter(); + ]; + for stmt in stmts { + patch.add_statement(location, stmt); + } st.make_nop(); - - Some(iter) - }); + } } + + patch.apply(body); } fn is_required(&self) -> bool { diff --git a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff index 727efe4b0d95d..267a4c1cf6beb 100644 --- a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff +++ b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.32bit.diff @@ -47,6 +47,7 @@ + Deinit(_8); + copy_nonoverlapping(dst = copy _9, src = copy _11, count = copy _7); + StorageDead(_4); ++ nop; StorageDead(_2); - _0 = move _1; + StorageLive(_12); @@ -61,6 +62,7 @@ + Deinit(_16); + copy_nonoverlapping(dst = copy _17, src = copy _19, count = copy _15); + StorageDead(_12); ++ nop; StorageDead(_1); return; } diff --git a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff index 8d0cd97f78661..8e5c403cd7e6b 100644 --- a/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff +++ b/tests/mir-opt/enum_opt.cand.EnumSizeOpt.64bit.diff @@ -47,6 +47,7 @@ + Deinit(_8); + copy_nonoverlapping(dst = copy _9, src = copy _11, count = copy _7); + StorageDead(_4); ++ nop; StorageDead(_2); - _0 = move _1; + StorageLive(_12); @@ -61,6 +62,7 @@ + Deinit(_16); + copy_nonoverlapping(dst = copy _17, src = copy _19, count = copy _15); + StorageDead(_12); ++ nop; StorageDead(_1); return; } diff --git a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff index 6d1e2a72fdb73..96c5aadd85fd4 100644 --- a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff +++ b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.32bit.diff @@ -47,6 +47,7 @@ + Deinit(_8); + copy_nonoverlapping(dst = copy _9, src = copy _11, count = copy _7); + StorageDead(_4); ++ nop; StorageDead(_2); - _0 = move _1; + StorageLive(_12); @@ -61,6 +62,7 @@ + Deinit(_16); + copy_nonoverlapping(dst = copy _17, src = copy _19, count = copy _15); + StorageDead(_12); ++ nop; StorageDead(_1); return; } diff --git a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff index 4b1406d0d623b..d20e2e08eaafd 100644 --- a/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff +++ b/tests/mir-opt/enum_opt.unin.EnumSizeOpt.64bit.diff @@ -47,6 +47,7 @@ + Deinit(_8); + copy_nonoverlapping(dst = copy _9, src = copy _11, count = copy _7); + StorageDead(_4); ++ nop; StorageDead(_2); - _0 = move _1; + StorageLive(_12); @@ -61,6 +62,7 @@ + Deinit(_16); + copy_nonoverlapping(dst = copy _17, src = copy _19, count = copy _15); + StorageDead(_12); ++ nop; StorageDead(_1); return; } From e3316ae453a86eed28840a85b12df2ea1917aac7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 12:48:27 +1100 Subject: [PATCH 16/28] Improve `MirPatch` documentation and naming. It's currently lacking comments. This commit adds some, which is useful because there are some methods with non-obvious behaviour. The commit also renames two things: - `patch_map` becomes `term_patch_map`, because it's only about terminators. - `is_patched` becomes `is_term_patched`, for the same reason. (I would guess that originally `MirPatch` only handled terminators, and then over time it expanded to allow other modifications, but these names weren't updated.) --- .../src/elaborate_drops.rs | 6 +-- compiler/rustc_mir_transform/src/patch.rs | 51 ++++++++++++++----- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index ab6aafab446bb..530c72ca549a6 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -417,7 +417,7 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { .. } = data.terminator().kind { - assert!(!self.patch.is_patched(bb)); + assert!(!self.patch.is_term_patched(bb)); let loc = Location { block: tgt, statement_index: 0 }; let path = self.move_data().rev_lookup.find(destination.as_ref()); @@ -462,7 +462,7 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { // a Goto; see `MirPatch::new`). } _ => { - assert!(!self.patch.is_patched(bb)); + assert!(!self.patch.is_term_patched(bb)); } } } @@ -486,7 +486,7 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { .. } = data.terminator().kind { - assert!(!self.patch.is_patched(bb)); + assert!(!self.patch.is_term_patched(bb)); let loc = Location { block: bb, statement_index: data.statements.len() }; let path = self.move_data().rev_lookup.find(destination.as_ref()); diff --git a/compiler/rustc_mir_transform/src/patch.rs b/compiler/rustc_mir_transform/src/patch.rs index b4f6fa514a487..d3d181f6cb2b1 100644 --- a/compiler/rustc_mir_transform/src/patch.rs +++ b/compiler/rustc_mir_transform/src/patch.rs @@ -4,11 +4,12 @@ use rustc_middle::ty::Ty; use rustc_span::Span; use tracing::debug; -/// This struct represents a patch to MIR, which can add -/// new statements and basic blocks and patch over block -/// terminators. +/// This struct lets you "patch" a MIR body, i.e. modify it. You can queue up +/// various changes, such as the addition of new statements and basic blocks +/// and replacement of terminators, and then apply the queued changes all at +/// once with `apply`. This is useful for MIR transformation passes. pub(crate) struct MirPatch<'tcx> { - patch_map: IndexVec>>, + term_patch_map: IndexVec>>, new_blocks: Vec>, new_statements: Vec<(Location, StatementKind<'tcx>)>, new_locals: Vec>, @@ -24,9 +25,10 @@ pub(crate) struct MirPatch<'tcx> { } impl<'tcx> MirPatch<'tcx> { + /// Creates a new, empty patch. pub(crate) fn new(body: &Body<'tcx>) -> Self { let mut result = MirPatch { - patch_map: IndexVec::from_elem(None, &body.basic_blocks), + term_patch_map: IndexVec::from_elem(None, &body.basic_blocks), new_blocks: vec![], new_statements: vec![], new_locals: vec![], @@ -141,10 +143,12 @@ impl<'tcx> MirPatch<'tcx> { bb } - pub(crate) fn is_patched(&self, bb: BasicBlock) -> bool { - self.patch_map[bb].is_some() + /// Has a replacement of this block's terminator been queued in this patch? + pub(crate) fn is_term_patched(&self, bb: BasicBlock) -> bool { + self.term_patch_map[bb].is_some() } + /// Queues the addition of a new temporary with additional local info. pub(crate) fn new_local_with_info( &mut self, ty: Ty<'tcx>, @@ -159,6 +163,7 @@ impl<'tcx> MirPatch<'tcx> { Local::new(index) } + /// Queues the addition of a new temporary. pub(crate) fn new_temp(&mut self, ty: Ty<'tcx>, span: Span) -> Local { let index = self.next_local; self.next_local += 1; @@ -174,29 +179,46 @@ impl<'tcx> MirPatch<'tcx> { self.new_locals[new_local_idx].ty } + /// Queues the addition of a new basic block. pub(crate) fn new_block(&mut self, data: BasicBlockData<'tcx>) -> BasicBlock { - let block = BasicBlock::new(self.patch_map.len()); + let block = BasicBlock::new(self.term_patch_map.len()); debug!("MirPatch: new_block: {:?}: {:?}", block, data); self.new_blocks.push(data); - self.patch_map.push(None); + self.term_patch_map.push(None); block } + /// Queues the replacement of a block's terminator. pub(crate) fn patch_terminator(&mut self, block: BasicBlock, new: TerminatorKind<'tcx>) { - assert!(self.patch_map[block].is_none()); + assert!(self.term_patch_map[block].is_none()); debug!("MirPatch: patch_terminator({:?}, {:?})", block, new); - self.patch_map[block] = Some(new); + self.term_patch_map[block] = Some(new); } + /// Queues the insertion of a statement at a given location. The statement + /// currently at that location, and all statements that follow, are shifted + /// down. If multiple statements are queued for addition at the same + /// location, the final statement order after calling `apply` will match + /// the queue insertion order. + /// + /// E.g. if we have `s0` at location `loc` and do these calls: + /// + /// p.add_statement(loc, s1); + /// p.add_statement(loc, s2); + /// p.apply(body); + /// + /// then the final order will be `s1, s2, s0`, with `s1` at `loc`. pub(crate) fn add_statement(&mut self, loc: Location, stmt: StatementKind<'tcx>) { debug!("MirPatch: add_statement({:?}, {:?})", loc, stmt); self.new_statements.push((loc, stmt)); } + /// Like `add_statement`, but specialized for assignments. pub(crate) fn add_assign(&mut self, loc: Location, place: Place<'tcx>, rv: Rvalue<'tcx>) { self.add_statement(loc, StatementKind::Assign(Box::new((place, rv)))); } + /// Applies the queued changes. pub(crate) fn apply(self, body: &mut Body<'tcx>) { debug!( "MirPatch: {:?} new temps, starting from index {}: {:?}", @@ -209,14 +231,14 @@ impl<'tcx> MirPatch<'tcx> { self.new_blocks.len(), body.basic_blocks.len() ); - let bbs = if self.patch_map.is_empty() && self.new_blocks.is_empty() { + let bbs = if self.term_patch_map.is_empty() && self.new_blocks.is_empty() { body.basic_blocks.as_mut_preserves_cfg() } else { body.basic_blocks.as_mut() }; bbs.extend(self.new_blocks); body.local_decls.extend(self.new_locals); - for (src, patch) in self.patch_map.into_iter_enumerated() { + for (src, patch) in self.term_patch_map.into_iter_enumerated() { if let Some(patch) = patch { debug!("MirPatch: patching block {:?}", src); bbs[src].terminator_mut().kind = patch; @@ -224,6 +246,9 @@ impl<'tcx> MirPatch<'tcx> { } let mut new_statements = self.new_statements; + + // This must be a stable sort to provide the ordering described in the + // comment for `add_statement`. new_statements.sort_by_key(|s| s.0); let mut delta = 0; From 627e08c909168aa451c613570a7f624e05f72766 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 13:05:41 +1100 Subject: [PATCH 17/28] Remove `BasicBlockData::expand_statements`. The previous commit removed its single use. `MirPatch` is a more flexible alternative. --- compiler/rustc_middle/src/mir/mod.rs | 49 ---------------------------- 1 file changed, 49 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 795cfcef2d36d..c78cde82e15d1 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1376,55 +1376,6 @@ impl<'tcx> BasicBlockData<'tcx> { } } - pub fn expand_statements(&mut self, mut f: F) - where - F: FnMut(&mut Statement<'tcx>) -> Option, - I: iter::TrustedLen>, - { - // Gather all the iterators we'll need to splice in, and their positions. - let mut splices: Vec<(usize, I)> = vec![]; - let mut extra_stmts = 0; - for (i, s) in self.statements.iter_mut().enumerate() { - if let Some(mut new_stmts) = f(s) { - if let Some(first) = new_stmts.next() { - // We can already store the first new statement. - *s = first; - - // Save the other statements for optimized splicing. - let remaining = new_stmts.size_hint().0; - if remaining > 0 { - splices.push((i + 1 + extra_stmts, new_stmts)); - extra_stmts += remaining; - } - } else { - s.make_nop(); - } - } - } - - // Splice in the new statements, from the end of the block. - // FIXME(eddyb) This could be more efficient with a "gap buffer" - // where a range of elements ("gap") is left uninitialized, with - // splicing adding new elements to the end of that gap and moving - // existing elements from before the gap to the end of the gap. - // For now, this is safe code, emulating a gap but initializing it. - let mut gap = self.statements.len()..self.statements.len() + extra_stmts; - self.statements.resize( - gap.end, - Statement { source_info: SourceInfo::outermost(DUMMY_SP), kind: StatementKind::Nop }, - ); - for (splice_start, new_stmts) in splices.into_iter().rev() { - let splice_end = splice_start + new_stmts.size_hint().0; - while gap.end > splice_end { - gap.start -= 1; - gap.end -= 1; - self.statements.swap(gap.start, gap.end); - } - self.statements.splice(splice_start..splice_end, new_stmts); - gap.end = splice_start; - } - } - pub fn visitable(&self, index: usize) -> &dyn MirVisitable<'tcx> { if index < self.statements.len() { &self.statements[index] } else { &self.terminator } } From 69f5e342bf179c991e325587fb9b16a75851b372 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 13:31:08 +1100 Subject: [PATCH 18/28] Inline and remove `BasicBlockData::retain_statements`. It has a single call site, and the code is clearer this way. --- compiler/rustc_middle/src/mir/mod.rs | 11 ----------- compiler/rustc_mir_transform/src/coroutine.rs | 11 ++++++----- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index c78cde82e15d1..582941e7e0493 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1365,17 +1365,6 @@ impl<'tcx> BasicBlockData<'tcx> { self.terminator.as_mut().expect("invalid terminator state") } - pub fn retain_statements(&mut self, mut f: F) - where - F: FnMut(&mut Statement<'_>) -> bool, - { - for s in &mut self.statements { - if !f(s) { - s.make_nop(); - } - } - } - pub fn visitable(&self, index: usize) -> &dyn MirVisitable<'tcx> { if index < self.statements.len() { &self.statements[index] } else { &self.terminator } } diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index afc49c5cc54af..f3f3a65cd805d 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -393,12 +393,13 @@ impl<'tcx> MutVisitor<'tcx> for TransformVisitor<'tcx> { fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) { // Remove StorageLive and StorageDead statements for remapped locals - data.retain_statements(|s| match s.kind { - StatementKind::StorageLive(l) | StatementKind::StorageDead(l) => { - !self.remap.contains(l) + for s in &mut data.statements { + if let StatementKind::StorageLive(l) | StatementKind::StorageDead(l) = s.kind + && self.remap.contains(l) + { + s.make_nop(); } - _ => true, - }); + } let ret_val = match data.terminator().kind { TerminatorKind::Return => { From 04eeda47abd07a5ba6f3f93e586ecf75d5574545 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 13:40:58 +1100 Subject: [PATCH 19/28] Inline and replace `Statement::replace_nop`. It has a single call site, and doesn't seem worth having as an API function. --- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_middle/src/mir/statement.rs | 9 --------- compiler/rustc_mir_transform/src/single_use_consts.rs | 8 +++++--- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 582941e7e0493..74ecff7208244 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -4,8 +4,8 @@ use std::borrow::Cow; use std::fmt::{self, Debug, Formatter}; +use std::iter; use std::ops::{Index, IndexMut}; -use std::{iter, mem}; pub use basic_blocks::BasicBlocks; use either::Either; diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index d345c99f902f2..690a907f9a335 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -19,15 +19,6 @@ impl Statement<'_> { pub fn make_nop(&mut self) { self.kind = StatementKind::Nop } - - /// Changes a statement to a nop and returns the original statement. - #[must_use = "If you don't need the statement, use `make_nop` instead"] - pub fn replace_nop(&mut self) -> Self { - Statement { - source_info: self.source_info, - kind: mem::replace(&mut self.kind, StatementKind::Nop), - } - } } impl<'tcx> StatementKind<'tcx> { diff --git a/compiler/rustc_mir_transform/src/single_use_consts.rs b/compiler/rustc_mir_transform/src/single_use_consts.rs index c5e951eb8b2c2..02caa92ad3fc8 100644 --- a/compiler/rustc_mir_transform/src/single_use_consts.rs +++ b/compiler/rustc_mir_transform/src/single_use_consts.rs @@ -48,9 +48,11 @@ impl<'tcx> crate::MirPass<'tcx> for SingleUseConsts { // We're only changing an operand, not the terminator kinds or successors let basic_blocks = body.basic_blocks.as_mut_preserves_cfg(); - let init_statement = - basic_blocks[init_loc.block].statements[init_loc.statement_index].replace_nop(); - let StatementKind::Assign(place_and_rvalue) = init_statement.kind else { + let init_statement_kind = std::mem::replace( + &mut basic_blocks[init_loc.block].statements[init_loc.statement_index].kind, + StatementKind::Nop, + ); + let StatementKind::Assign(place_and_rvalue) = init_statement_kind else { bug!("No longer an assign?"); }; let (place, rvalue) = *place_and_rvalue; From 693f7035f1a98cc9a640b045c9e996c8bb1606cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 18 Feb 2025 04:50:11 +0000 Subject: [PATCH 20/28] Make E0599 a structured error --- compiler/rustc_hir_analysis/messages.ftl | 2 ++ compiler/rustc_hir_analysis/src/errors.rs | 9 +++++++++ .../src/hir_ty_lowering/mod.rs | 17 ++++++++--------- compiler/rustc_hir_analysis/src/lib.rs | 1 + compiler/rustc_hir_typeck/src/expr.rs | 14 +++++--------- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 5560d087e96f0..47d5976be09ef 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -387,6 +387,8 @@ hir_analysis_must_implement_not_function_span_note = required by this annotation hir_analysis_must_implement_one_of_attribute = the `#[rustc_must_implement_one_of]` attribute must be used with at least 2 args +hir_analysis_no_variant_named = no variant named `{$ident}` found for enum `{$ty}` + hir_analysis_not_supported_delegation = {$descr} .label = callee defined here diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 14ea10461cbea..1a0b0edb25703 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1417,6 +1417,15 @@ pub(crate) struct CrossCrateTraitsDefined { pub traits: String, } +#[derive(Diagnostic)] +#[diag(hir_analysis_no_variant_named, code = E0599)] +pub struct NoVariantNamed<'tcx> { + #[primary_span] + pub span: Span, + pub ident: Ident, + pub ty: Ty<'tcx>, +} + // FIXME(fmease): Deduplicate: #[derive(Diagnostic)] diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 88323db6dda65..3b17d7fe6ffc0 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -53,7 +53,9 @@ use tracing::{debug, instrument}; use crate::bounds::Bounds; use crate::check::check_abi_fn_ptr; -use crate::errors::{AmbiguousLifetimeBound, BadReturnTypeNotation, InvalidBaseType}; +use crate::errors::{ + AmbiguousLifetimeBound, BadReturnTypeNotation, InvalidBaseType, NoVariantNamed, +}; use crate::hir_ty_lowering::errors::{GenericsArgsErrExtend, prohibit_assoc_item_constraint}; use crate::hir_ty_lowering::generics::{check_generic_arg_count, lower_generic_args}; use crate::middle::resolve_bound_vars as rbv; @@ -1188,14 +1190,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let msg = format!("expected type, found variant `{assoc_ident}`"); self.dcx().span_err(span, msg) } else if qself_ty.is_enum() { - let mut err = struct_span_code_err!( - self.dcx(), - assoc_ident.span, - E0599, - "no variant named `{}` found for enum `{}`", - assoc_ident, - qself_ty, - ); + let mut err = self.dcx().create_err(NoVariantNamed { + span: assoc_ident.span, + ident: assoc_ident, + ty: qself_ty, + }); let adt_def = qself_ty.ty_adt_def().expect("enum is not an ADT"); if let Some(variant_name) = find_best_match_for_name( diff --git a/compiler/rustc_hir_analysis/src/lib.rs b/compiler/rustc_hir_analysis/src/lib.rs index 8a529e9c686e0..2068b9f2dea6e 100644 --- a/compiler/rustc_hir_analysis/src/lib.rs +++ b/compiler/rustc_hir_analysis/src/lib.rs @@ -93,6 +93,7 @@ mod impl_wf_check; mod outlives; mod variance; +pub use errors::NoVariantNamed; use rustc_abi::ExternAbi; use rustc_hir as hir; use rustc_hir::def::DefKind; diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 9ccd674608744..9000868ec276d 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -19,6 +19,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; use rustc_hir::lang_items::LangItem; use rustc_hir::{ExprKind, HirId, QPath}; +use rustc_hir_analysis::NoVariantNamed; use rustc_hir_analysis::hir_ty_lowering::{FeedConstTy, HirTyLowerer as _}; use rustc_infer::infer; use rustc_infer::infer::{DefineOpaqueTypes, InferOk}; @@ -3837,15 +3838,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .iter_enumerated() .find(|(_, v)| v.ident(self.tcx).normalize_to_macros_2_0() == ident) else { - type_error_struct!( - self.dcx(), - ident.span, - container, - E0599, - "no variant named `{ident}` found for enum `{container}`", - ) - .with_span_label(field.span, "variant not found") - .emit(); + self.dcx() + .create_err(NoVariantNamed { span: ident.span, ident, ty: container }) + .with_span_label(field.span, "variant not found") + .emit_unless(container.references_error()); break; }; let Some(&subfield) = fields.next() else { From d0a5bbbb8e394095848e078a3fff72beaad14209 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 16 Feb 2025 00:03:10 +0100 Subject: [PATCH 21/28] document and test all `LayoutError` variants --- compiler/rustc_middle/src/ty/layout.rs | 21 ++++++++ tests/ui/layout/debug.rs | 5 ++ tests/ui/layout/debug.stderr | 8 ++- tests/ui/layout/normalization-failure.rs | 57 ++++++++++++++++++++ tests/ui/layout/normalization-failure.stderr | 12 +++++ 5 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/ui/layout/normalization-failure.rs create mode 100644 tests/ui/layout/normalization-failure.stderr diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index bbb8a9fa67149..2a058addf1597 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -229,11 +229,32 @@ impl fmt::Display for ValidityRequirement { #[derive(Copy, Clone, Debug, HashStable, TyEncodable, TyDecodable)] pub enum LayoutError<'tcx> { + /// A type doesn't have a sensible layout. + /// + /// This variant is used for layout errors that don't necessarily cause + /// compile errors. + /// + /// For example, this can happen if a struct contains an unsized type in a + /// non-tail field, but has an unsatisfiable bound like `str: Sized`. Unknown(Ty<'tcx>), + /// The size of a type exceeds [`TargetDataLayout::obj_size_bound`]. SizeOverflow(Ty<'tcx>), + /// The layout can vary due to a generic parameter. + /// + /// Unlike `Unknown`, this variant is a "soft" error and indicates that the layout + /// may become computable after further instantiating the generic parameter(s). TooGeneric(Ty<'tcx>), + /// An alias failed to normalize. + /// + /// This variant is necessary, because, due to trait solver incompleteness, it is + /// possible than an alias that was rigid during analysis fails to normalize after + /// revealing opaque types. + /// + /// See `tests/ui/layout/normalization-failure.rs` for an example. NormalizationFailure(Ty<'tcx>, NormalizationError<'tcx>), + /// A non-layout error is reported elsewhere. ReferencesError(ErrorGuaranteed), + /// A type has cyclic layout, i.e. the type contains itself without indirection. Cycle(ErrorGuaranteed), } diff --git a/tests/ui/layout/debug.rs b/tests/ui/layout/debug.rs index 81dc72852543d..c0adf9ec67763 100644 --- a/tests/ui/layout/debug.rs +++ b/tests/ui/layout/debug.rs @@ -82,3 +82,8 @@ type Impossible = (str, str); //~ ERROR: cannot be known at compilation time #[rustc_layout(debug)] union EmptyUnion {} //~ ERROR: has an unknown layout //~^ ERROR: unions cannot have zero fields + +// Test the error message of `LayoutError::TooGeneric` +// (this error is never emitted to users). +#[rustc_layout(debug)] +type TooGeneric = T; //~ ERROR: does not have a fixed size diff --git a/tests/ui/layout/debug.stderr b/tests/ui/layout/debug.stderr index 319c0de26a90e..d64dee4e27fc3 100644 --- a/tests/ui/layout/debug.stderr +++ b/tests/ui/layout/debug.stderr @@ -596,12 +596,18 @@ error: the type has an unknown layout LL | union EmptyUnion {} | ^^^^^^^^^^^^^^^^ +error: `T` does not have a fixed size + --> $DIR/debug.rs:89:1 + | +LL | type TooGeneric = T; + | ^^^^^^^^^^^^^^^^^^ + error: `#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases --> $DIR/debug.rs:75:5 | LL | const C: () = (); | ^^^^^^^^^^^ -error: aborting due to 19 previous errors +error: aborting due to 20 previous errors For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/layout/normalization-failure.rs b/tests/ui/layout/normalization-failure.rs new file mode 100644 index 0000000000000..c0f8710c03cb3 --- /dev/null +++ b/tests/ui/layout/normalization-failure.rs @@ -0,0 +1,57 @@ +//! This test demonstrates how `LayoutError::NormalizationFailure` can happen and why +//! it is necessary. +//! +//! This code does not cause an immediate normalization error in typeck, because we +//! don't reveal the hidden type returned by `opaque` in the analysis typing mode. +//! Instead, `<{opaque} as Project2>::Assoc2` is a *rigid projection*, because we know +//! that `{opaque}: Project2` holds, due to the opaque type's `impl Project2` bound, +//! but cannot normalize `<{opaque} as Project2>::Assoc2` any further. +//! +//! However, in the post-analysis typing mode, which is used for the layout computation, +//! the opaque's hidden type is revealed to be `PhantomData`, and now we fail to +//! normalize ` as Project2>::Assoc2` if there is a `T: Project1` bound +//! in the param env! This happens, because `PhantomData: Project2` only holds if +//! `::Assoc1 == ()` holds. This would usually be satisfied by the +//! blanket `impl Project1 for T`, but due to the `T: Project1` bound we do not +//! normalize `::Assoc1` via the impl and treat it as rigid instead. +//! Therefore, `PhantomData: Project2` does NOT hold and normalizing +//! ` as Project2>::Assoc2` fails. +//! +//! Note that this layout error can only happen when computing the layout in a generic +//! context, which is not required for codegen, but may happen for lints, MIR optimizations, +//! and the transmute check. + +use std::marker::PhantomData; + +trait Project1 { + type Assoc1; +} + +impl Project1 for T { + type Assoc1 = (); +} + +trait Project2 { + type Assoc2; + fn get(self) -> Self::Assoc2; +} + +impl> Project2 for PhantomData { + type Assoc2 = (); + fn get(self) -> Self::Assoc2 {} +} + +fn opaque() -> impl Project2 { + PhantomData:: +} + +fn check() { + unsafe { + std::mem::transmute::<_, ()>(opaque::().get()); + //~^ ERROR: cannot transmute + //~| NOTE: (unable to determine layout for `::Assoc2` because `::Assoc2` cannot be normalized) + //~| NOTE: (0 bits) + } +} + +fn main() {} diff --git a/tests/ui/layout/normalization-failure.stderr b/tests/ui/layout/normalization-failure.stderr new file mode 100644 index 0000000000000..5fe38d4403a20 --- /dev/null +++ b/tests/ui/layout/normalization-failure.stderr @@ -0,0 +1,12 @@ +error[E0512]: cannot transmute between types of different sizes, or dependently-sized types + --> $DIR/normalization-failure.rs:50:9 + | +LL | std::mem::transmute::<_, ()>(opaque::().get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: source type: `::Assoc2` (unable to determine layout for `::Assoc2` because `::Assoc2` cannot be normalized) + = note: target type: `()` (0 bits) + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0512`. From 802b7abab736166fcb12c2567c743e1da6d6806c Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 16 Feb 2025 20:26:07 +0100 Subject: [PATCH 22/28] clean up layout error diagnostics - group the fluent slugs together - reword (internal-only) "too generic" error to be more in line with the other errors --- compiler/rustc_middle/messages.ftl | 28 +++++++++++++------------- compiler/rustc_middle/src/error.rs | 10 ++++----- compiler/rustc_middle/src/ty/layout.rs | 12 +++++------ tests/ui/layout/debug.rs | 2 +- tests/ui/layout/debug.stderr | 2 +- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_middle/messages.ftl b/compiler/rustc_middle/messages.ftl index dcfa81dab2526..0b3c0be1a4e1a 100644 --- a/compiler/rustc_middle/messages.ftl +++ b/compiler/rustc_middle/messages.ftl @@ -37,9 +37,6 @@ middle_autodiff_unsafe_inner_const_ref = reading from a `Duplicated` const {$ty} middle_bounds_check = index out of bounds: the length is {$len} but the index is {$index} -middle_cannot_be_normalized = - unable to determine layout for `{$ty}` because `{$failure_ty}` cannot be normalized - middle_conflict_types = this expression supplies two conflicting concrete types for the same opaque type @@ -52,9 +49,6 @@ middle_const_eval_non_int = middle_const_not_used_in_type_alias = const parameter `{$ct}` is part of concrete type but not used in parameter list for the `impl Trait` type alias -middle_cycle = - a cycle occurred during layout computation - middle_deprecated = use of deprecated {$kind} `{$path}`{$has_note -> [true] : {$note} *[other] {""} @@ -78,9 +72,23 @@ middle_erroneous_constant = erroneous constant encountered middle_failed_writing_file = failed to write file {$path}: {$error}" +middle_layout_cycle = + a cycle occurred during layout computation + +middle_layout_normalization_failure = + unable to determine layout for `{$ty}` because `{$failure_ty}` cannot be normalized + middle_layout_references_error = the type has an unknown layout +middle_layout_size_overflow = + values of the type `{$ty}` are too big for the target architecture + +middle_layout_too_generic = the type `{$ty}` does not have a fixed layout + +middle_layout_unknown = + the type `{$ty}` has an unknown layout + middle_opaque_hidden_type_mismatch = concrete type differs from previous defining opaque type use .label = expected `{$self_ty}`, got `{$other_ty}` @@ -98,16 +106,8 @@ middle_strict_coherence_needs_negative_coherence = to use `strict_coherence` on this trait, the `with_negative_coherence` feature must be enabled .label = due to this attribute -middle_too_generic = `{$ty}` does not have a fixed size - middle_type_length_limit = reached the type-length limit while instantiating `{$shrunk}` -middle_unknown_layout = - the type `{$ty}` has an unknown layout - middle_unsupported_union = we don't support unions yet: '{$ty_name}' -middle_values_too_big = - values of the type `{$ty}` are too big for the target architecture - middle_written_to_path = the full type name has been written to '{$path}' diff --git a/compiler/rustc_middle/src/error.rs b/compiler/rustc_middle/src/error.rs index c53e3d54cc491..be8a3403ba956 100644 --- a/compiler/rustc_middle/src/error.rs +++ b/compiler/rustc_middle/src/error.rs @@ -132,19 +132,19 @@ impl fmt::Debug for CustomSubdiagnostic<'_> { #[derive(Diagnostic)] pub enum LayoutError<'tcx> { - #[diag(middle_unknown_layout)] + #[diag(middle_layout_unknown)] Unknown { ty: Ty<'tcx> }, - #[diag(middle_too_generic)] + #[diag(middle_layout_too_generic)] TooGeneric { ty: Ty<'tcx> }, - #[diag(middle_values_too_big)] + #[diag(middle_layout_size_overflow)] Overflow { ty: Ty<'tcx> }, - #[diag(middle_cannot_be_normalized)] + #[diag(middle_layout_normalization_failure)] NormalizationFailure { ty: Ty<'tcx>, failure_ty: String }, - #[diag(middle_cycle)] + #[diag(middle_layout_cycle)] Cycle, #[diag(middle_layout_references_error)] diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 2a058addf1597..19fa832357499 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -264,11 +264,11 @@ impl<'tcx> LayoutError<'tcx> { use crate::fluent_generated::*; match self { - Unknown(_) => middle_unknown_layout, - SizeOverflow(_) => middle_values_too_big, - TooGeneric(_) => middle_too_generic, - NormalizationFailure(_, _) => middle_cannot_be_normalized, - Cycle(_) => middle_cycle, + Unknown(_) => middle_layout_unknown, + SizeOverflow(_) => middle_layout_size_overflow, + TooGeneric(_) => middle_layout_too_generic, + NormalizationFailure(_, _) => middle_layout_normalization_failure, + Cycle(_) => middle_layout_cycle, ReferencesError(_) => middle_layout_references_error, } } @@ -297,7 +297,7 @@ impl<'tcx> fmt::Display for LayoutError<'tcx> { match *self { LayoutError::Unknown(ty) => write!(f, "the type `{ty}` has an unknown layout"), LayoutError::TooGeneric(ty) => { - write!(f, "`{ty}` does not have a fixed size") + write!(f, "the type `{ty}` does not have a fixed layout") } LayoutError::SizeOverflow(ty) => { write!(f, "values of the type `{ty}` are too big for the target architecture") diff --git a/tests/ui/layout/debug.rs b/tests/ui/layout/debug.rs index c0adf9ec67763..b87a1d2031df4 100644 --- a/tests/ui/layout/debug.rs +++ b/tests/ui/layout/debug.rs @@ -86,4 +86,4 @@ union EmptyUnion {} //~ ERROR: has an unknown layout // Test the error message of `LayoutError::TooGeneric` // (this error is never emitted to users). #[rustc_layout(debug)] -type TooGeneric = T; //~ ERROR: does not have a fixed size +type TooGeneric = T; //~ ERROR: does not have a fixed layout diff --git a/tests/ui/layout/debug.stderr b/tests/ui/layout/debug.stderr index d64dee4e27fc3..0daf2d3b9e7f2 100644 --- a/tests/ui/layout/debug.stderr +++ b/tests/ui/layout/debug.stderr @@ -596,7 +596,7 @@ error: the type has an unknown layout LL | union EmptyUnion {} | ^^^^^^^^^^^^^^^^ -error: `T` does not have a fixed size +error: the type `T` does not have a fixed layout --> $DIR/debug.rs:89:1 | LL | type TooGeneric = T; From 7a667d206c45b96676c260030a4e3d9be1cc8495 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 16 Feb 2025 00:19:56 +0100 Subject: [PATCH 23/28] remove unreachable cases `ty::Placeholder` is used by the trait solver and computing its layout was necessary, because the `PointerLike` trait used to be automatically implemented for all types with pointer-like layout. Nowadays, `PointerLike` requires user-written impls and the trait solver no longer computes any layouts, so this can be removed. Unevaluated constants that aren't generic should have caused a const eval error earlier during normalization. --- compiler/rustc_ty_utils/src/layout.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 8429e68b600da..f9ce709d07cb6 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -155,19 +155,12 @@ fn extract_const_value<'tcx>( ty::ConstKind::Error(guar) => { return Err(error(cx, LayoutError::ReferencesError(guar))); } - ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) => { + ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) | ty::ConstKind::Unevaluated(_) => { if !const_.has_param() { - bug!("no generic type found in the type: {ty:?}"); + bug!("failed to normalize const, but it is not generic: {const_:?}"); } return Err(error(cx, LayoutError::TooGeneric(ty))); } - ty::ConstKind::Unevaluated(_) => { - if !const_.has_param() { - return Err(error(cx, LayoutError::Unknown(ty))); - } else { - return Err(error(cx, LayoutError::TooGeneric(ty))); - } - } ty::ConstKind::Infer(_) | ty::ConstKind::Bound(..) | ty::ConstKind::Placeholder(_) => { bug!("unexpected type: {ty:?}"); } @@ -728,17 +721,17 @@ fn layout_of_uncached<'tcx>( return Err(error(cx, LayoutError::Unknown(ty))); } - ty::Bound(..) | ty::CoroutineWitness(..) | ty::Infer(_) | ty::Error(_) => { - bug!("Layout::compute: unexpected type `{}`", ty) + ty::Placeholder(..) + | ty::Bound(..) + | ty::CoroutineWitness(..) + | ty::Infer(_) + | ty::Error(_) => { + bug!("layout_of: unexpected type `{ty}`") } ty::Param(_) => { return Err(error(cx, LayoutError::TooGeneric(ty))); } - - ty::Placeholder(..) => { - return Err(error(cx, LayoutError::Unknown(ty))); - } }) } From 1d1ac3d310dfe90e1ec91c731f2cc306ddbd1eb4 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 16 Feb 2025 01:02:18 +0100 Subject: [PATCH 24/28] remove redundant code - we normalize before calling `layout_of_uncached`, so we don't need to normalize again later - we check for type/const errors at the top of `layout_of_uncached`, so we don't need to check again later --- compiler/rustc_ty_utils/src/layout.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index f9ce709d07cb6..5c5558053cc40 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -152,17 +152,19 @@ fn extract_const_value<'tcx>( ) -> Result, &'tcx LayoutError<'tcx>> { match const_.kind() { ty::ConstKind::Value(cv) => Ok(cv), - ty::ConstKind::Error(guar) => { - return Err(error(cx, LayoutError::ReferencesError(guar))); - } ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) | ty::ConstKind::Unevaluated(_) => { if !const_.has_param() { bug!("failed to normalize const, but it is not generic: {const_:?}"); } return Err(error(cx, LayoutError::TooGeneric(ty))); } - ty::ConstKind::Infer(_) | ty::ConstKind::Bound(..) | ty::ConstKind::Placeholder(_) => { - bug!("unexpected type: {ty:?}"); + ty::ConstKind::Infer(_) + | ty::ConstKind::Bound(..) + | ty::ConstKind::Placeholder(_) + | ty::ConstKind::Error(_) => { + // `ty::ConstKind::Error` is handled at the top of `layout_of_uncached` + // (via `ty.error_reported()`). + bug!("layout_of: unexpected const: {const_:?}"); } } } @@ -267,16 +269,11 @@ fn layout_of_uncached<'tcx>( data_ptr.valid_range_mut().start = 1; } - let pointee = tcx.normalize_erasing_regions(cx.typing_env, pointee); if pointee.is_sized(tcx, cx.typing_env) { return Ok(tcx.mk_layout(LayoutData::scalar(cx, data_ptr))); } - let metadata = if let Some(metadata_def_id) = tcx.lang_items().metadata_type() - // Projection eagerly bails out when the pointee references errors, - // fall back to structurally deducing metadata. - && !pointee.references_error() - { + let metadata = if let Some(metadata_def_id) = tcx.lang_items().metadata_type() { let pointee_metadata = Ty::new_projection(tcx, metadata_def_id, [pointee]); let metadata_ty = match tcx.try_normalize_erasing_regions(cx.typing_env, pointee_metadata) { @@ -726,6 +723,7 @@ fn layout_of_uncached<'tcx>( | ty::CoroutineWitness(..) | ty::Infer(_) | ty::Error(_) => { + // `ty::Error` is handled at the top of this function. bug!("layout_of: unexpected type `{ty}`") } From 67345f9203b451f21aa603329ab657f6e782267c Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 16 Feb 2025 01:17:08 +0100 Subject: [PATCH 25/28] remove useless parameter Remove the `repr` parameter from the wrappers around `calc.univariant`, because it's always defaulted. Only ADTs can have a repr and those call `calc.layout_of_struct_or_enum` and not `calc.univariant`. --- compiler/rustc_ty_utils/src/layout.rs | 46 ++++++--------------------- 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 5c5558053cc40..36dbddbb9d78a 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -134,15 +134,10 @@ fn univariant_uninterned<'tcx>( cx: &LayoutCx<'tcx>, ty: Ty<'tcx>, fields: &IndexSlice>, - repr: &ReprOptions, kind: StructKind, ) -> Result, &'tcx LayoutError<'tcx>> { - let pack = repr.pack; - if pack.is_some() && repr.align.is_some() { - cx.tcx().dcx().bug("struct cannot be packed and aligned"); - } - - cx.calc.univariant(fields, repr, kind).map_err(|err| map_error(cx, ty, err)) + let repr = ReprOptions::default(); + cx.calc.univariant(fields, &repr, kind).map_err(|err| map_error(cx, ty, err)) } fn extract_const_value<'tcx>( @@ -189,10 +184,9 @@ fn layout_of_uncached<'tcx>( }; let scalar = |value: Primitive| tcx.mk_layout(LayoutData::scalar(cx, scalar_unit(value))); - let univariant = - |fields: &IndexSlice>, repr: &ReprOptions, kind| { - Ok(tcx.mk_layout(univariant_uninterned(cx, ty, fields, repr, kind)?)) - }; + let univariant = |fields: &IndexSlice>, kind| { + Ok(tcx.mk_layout(univariant_uninterned(cx, ty, fields, kind)?)) + }; debug_assert!(!ty.has_non_region_infer()); Ok(match *ty.kind() { @@ -405,17 +399,10 @@ fn layout_of_uncached<'tcx>( }), // Odd unit types. - ty::FnDef(..) => { - univariant(IndexSlice::empty(), &ReprOptions::default(), StructKind::AlwaysSized)? - } + ty::FnDef(..) => univariant(IndexSlice::empty(), StructKind::AlwaysSized)?, ty::Dynamic(_, _, ty::Dyn) | ty::Foreign(..) => { - let mut unit = univariant_uninterned( - cx, - ty, - IndexSlice::empty(), - &ReprOptions::default(), - StructKind::AlwaysSized, - )?; + let mut unit = + univariant_uninterned(cx, ty, IndexSlice::empty(), StructKind::AlwaysSized)?; match unit.backend_repr { BackendRepr::Memory { ref mut sized } => *sized = false, _ => bug!(), @@ -429,7 +416,6 @@ fn layout_of_uncached<'tcx>( let tys = args.as_closure().upvar_tys(); univariant( &tys.iter().map(|ty| cx.layout_of(ty)).try_collect::>()?, - &ReprOptions::default(), StructKind::AlwaysSized, )? } @@ -438,7 +424,6 @@ fn layout_of_uncached<'tcx>( let tys = args.as_coroutine_closure().upvar_tys(); univariant( &tys.iter().map(|ty| cx.layout_of(ty)).try_collect::>()?, - &ReprOptions::default(), StructKind::AlwaysSized, )? } @@ -447,11 +432,7 @@ fn layout_of_uncached<'tcx>( let kind = if tys.len() == 0 { StructKind::AlwaysSized } else { StructKind::MaybeUnsized }; - univariant( - &tys.iter().map(|k| cx.layout_of(k)).try_collect::>()?, - &ReprOptions::default(), - kind, - )? + univariant(&tys.iter().map(|k| cx.layout_of(k)).try_collect::>()?, kind)? } // SIMD vector types. @@ -902,13 +883,7 @@ fn coroutine_layout<'tcx>( .chain(iter::once(Ok(tag_layout))) .chain(promoted_layouts) .try_collect::>()?; - let prefix = univariant_uninterned( - cx, - ty, - &prefix_layouts, - &ReprOptions::default(), - StructKind::AlwaysSized, - )?; + let prefix = univariant_uninterned(cx, ty, &prefix_layouts, StructKind::AlwaysSized)?; let (prefix_size, prefix_align) = (prefix.size, prefix.align); @@ -973,7 +948,6 @@ fn coroutine_layout<'tcx>( cx, ty, &variant_only_tys.map(|ty| cx.layout_of(ty)).try_collect::>()?, - &ReprOptions::default(), StructKind::Prefixed(prefix_size, prefix_align.abi), )?; variant.variants = Variants::Single { index }; From 2fbc413d832486106221253a2db7f4670473ee67 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 16 Feb 2025 01:32:38 +0100 Subject: [PATCH 26/28] cosmetic changes - change function parameter order to `cx, ty, ...` to match the other functions in this file - use `ct` identifier for `ty::Const` to match the majority of the compiler codebase - remove useless return - bring match arms in a more natural order --- compiler/rustc_ty_utils/src/layout.rs | 40 +++++++++++++++------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 36dbddbb9d78a..556512e0236e5 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -141,17 +141,17 @@ fn univariant_uninterned<'tcx>( } fn extract_const_value<'tcx>( - const_: ty::Const<'tcx>, - ty: Ty<'tcx>, cx: &LayoutCx<'tcx>, + ty: Ty<'tcx>, + ct: ty::Const<'tcx>, ) -> Result, &'tcx LayoutError<'tcx>> { - match const_.kind() { + match ct.kind() { ty::ConstKind::Value(cv) => Ok(cv), ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) | ty::ConstKind::Unevaluated(_) => { - if !const_.has_param() { - bug!("failed to normalize const, but it is not generic: {const_:?}"); + if !ct.has_param() { + bug!("failed to normalize const, but it is not generic: {ct:?}"); } - return Err(error(cx, LayoutError::TooGeneric(ty))); + Err(error(cx, LayoutError::TooGeneric(ty))) } ty::ConstKind::Infer(_) | ty::ConstKind::Bound(..) @@ -159,7 +159,7 @@ fn extract_const_value<'tcx>( | ty::ConstKind::Error(_) => { // `ty::ConstKind::Error` is handled at the top of `layout_of_uncached` // (via `ty.error_reported()`). - bug!("layout_of: unexpected const: {const_:?}"); + bug!("layout_of: unexpected const: {ct:?}"); } } } @@ -199,12 +199,12 @@ fn layout_of_uncached<'tcx>( &mut layout.backend_repr { if let Some(start) = start { - scalar.valid_range_mut().start = extract_const_value(start, ty, cx)? + scalar.valid_range_mut().start = extract_const_value(cx, ty, start)? .try_to_bits(tcx, cx.typing_env) .ok_or_else(|| error(cx, LayoutError::Unknown(ty)))?; } if let Some(end) = end { - let mut end = extract_const_value(end, ty, cx)? + let mut end = extract_const_value(cx, ty, end)? .try_to_bits(tcx, cx.typing_env) .ok_or_else(|| error(cx, LayoutError::Unknown(ty)))?; if !include_end { @@ -338,7 +338,7 @@ fn layout_of_uncached<'tcx>( // Arrays and slices. ty::Array(element, count) => { - let count = extract_const_value(count, ty, cx)? + let count = extract_const_value(cx, ty, count)? .try_to_target_usize(tcx) .ok_or_else(|| error(cx, LayoutError::Unknown(ty)))?; @@ -690,13 +690,21 @@ fn layout_of_uncached<'tcx>( } // Types with no meaningful known layout. + ty::Param(_) => { + return Err(error(cx, LayoutError::TooGeneric(ty))); + } + ty::Alias(..) => { - if ty.has_param() { - return Err(error(cx, LayoutError::TooGeneric(ty))); - } // NOTE(eddyb) `layout_of` query should've normalized these away, // if that was possible, so there's no reason to try again here. - return Err(error(cx, LayoutError::Unknown(ty))); + let err = if ty.has_param() { + LayoutError::TooGeneric(ty) + } else { + // This is only reachable with unsatisfiable predicates. For example, if we have + // `u8: Iterator`, then we can't compute the layout of `::Item`. + LayoutError::Unknown(ty) + }; + return Err(error(cx, err)); } ty::Placeholder(..) @@ -707,10 +715,6 @@ fn layout_of_uncached<'tcx>( // `ty::Error` is handled at the top of this function. bug!("layout_of: unexpected type `{ty}`") } - - ty::Param(_) => { - return Err(error(cx, LayoutError::TooGeneric(ty))); - } }) } From 35674eff6f69b04d55d22e31d879c8177abb3653 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Tue, 18 Feb 2025 17:26:33 +0100 Subject: [PATCH 27/28] Clarify that locking on Windows also works for files opened with `.read(true)` --- library/std/src/fs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 0707115cbdd93..df877a035a93f 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -649,7 +649,7 @@ impl File { /// this [may change in the future][changes]. /// /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, - /// open it with either `.read(true).append(true)` or `.write(true)`. + /// open it with one of `.read(true)`, `.read(true).append(true)`, or `.write(true)`. /// /// [changes]: io#platform-specific-behavior /// @@ -702,7 +702,7 @@ impl File { /// [may change in the future][changes]. /// /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, - /// open it with either `.read(true).append(true)` or `.write(true)`. + /// open it with one of `.read(true)`, `.read(true).append(true)`, or `.write(true)`. /// /// [changes]: io#platform-specific-behavior /// @@ -760,7 +760,7 @@ impl File { /// [may change in the future][changes]. /// /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, - /// open it with either `.read(true).append(true)` or `.write(true)`. + /// open it with one of `.read(true)`, `.read(true).append(true)`, or `.write(true)`. /// /// [changes]: io#platform-specific-behavior /// @@ -817,7 +817,7 @@ impl File { /// [may change in the future][changes]. /// /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, - /// open it with either `.read(true).append(true)` or `.write(true)`. + /// open it with one of `.read(true)`, `.read(true).append(true)`, or `.write(true)`. /// /// [changes]: io#platform-specific-behavior /// @@ -862,7 +862,7 @@ impl File { /// [may change in the future][changes]. /// /// On Windows, locking a file will fail if the file is opened only for append. To lock a file, - /// open it with either `.read(true).append(true)` or `.write(true)`. + /// open it with one of `.read(true)`, `.read(true).append(true)`, or `.write(true)`. /// /// [changes]: io#platform-specific-behavior /// From ec2034d53d83379abaaa57e395d4d8827ff7c432 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Tue, 18 Feb 2025 17:31:10 +0100 Subject: [PATCH 28/28] Reorder "This lock may be advisory or mandatory." earlier in the lock docs --- library/std/src/fs.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index df877a035a93f..36432b11f2049 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -628,17 +628,17 @@ impl File { /// /// This acquires an exclusive lock; no other file handle to this file may acquire another lock. /// + /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], + /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with + /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not + /// cause non-lockholders to block. + /// /// If this file handle/descriptor, or a clone of it, already holds an lock the exact behavior /// is unspecified and platform dependent, including the possibility that it will deadlock. /// However, if this method returns, then an exclusive lock is held. /// /// If the file not open for writing, it is unspecified whether this function returns an error. /// - /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], - /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with - /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not - /// cause non-lockholders to block. - /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. /// @@ -683,15 +683,15 @@ impl File { /// This acquires a shared lock; more than one file handle may hold a shared lock, but none may /// hold an exclusive lock at the same time. /// - /// If this file handle/descriptor, or a clone of it, already holds an lock, the exact behavior - /// is unspecified and platform dependent, including the possibility that it will deadlock. - /// However, if this method returns, then a shared lock is held. - /// /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not /// cause non-lockholders to block. /// + /// If this file handle/descriptor, or a clone of it, already holds an lock, the exact behavior + /// is unspecified and platform dependent, including the possibility that it will deadlock. + /// However, if this method returns, then a shared lock is held. + /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. /// @@ -738,17 +738,17 @@ impl File { /// /// This acquires an exclusive lock; no other file handle to this file may acquire another lock. /// + /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], + /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with + /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not + /// cause non-lockholders to block. + /// /// If this file handle/descriptor, or a clone of it, already holds an lock, the exact behavior /// is unspecified and platform dependent, including the possibility that it will deadlock. /// However, if this method returns `Ok(true)`, then it has acquired an exclusive lock. /// /// If the file not open for writing, it is unspecified whether this function returns an error. /// - /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], - /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with - /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not - /// cause non-lockholders to block. - /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. /// @@ -797,15 +797,15 @@ impl File { /// This acquires a shared lock; more than one file handle may hold a shared lock, but none may /// hold an exclusive lock at the same time. /// - /// If this file handle, or a clone of it, already holds an lock, the exact behavior is - /// unspecified and platform dependent, including the possibility that it will deadlock. - /// However, if this method returns `Ok(true)`, then it has acquired a shared lock. - /// /// This lock may be advisory or mandatory. This lock is meant to interact with [`lock`], /// [`try_lock`], [`lock_shared`], [`try_lock_shared`], and [`unlock`]. Its interactions with /// other methods, such as [`read`] and [`write`] are platform specific, and it may or may not /// cause non-lockholders to block. /// + /// If this file handle, or a clone of it, already holds an lock, the exact behavior is + /// unspecified and platform dependent, including the possibility that it will deadlock. + /// However, if this method returns `Ok(true)`, then it has acquired a shared lock. + /// /// The lock will be released when this file (along with any other file descriptors/handles /// duplicated or inherited from it) is closed, or if the [`unlock`] method is called. ///