From af62890a73ad67cbde98e88bdd4f33c4ccb67381 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Fri, 10 May 2024 19:00:06 +0200 Subject: [PATCH 01/21] Stabilization RFC for `core::marker::Freeze` in bounds --- text/0000-stabilize-marker-freeze.md | 66 ++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 text/0000-stabilize-marker-freeze.md diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md new file mode 100644 index 00000000000..59347e6c185 --- /dev/null +++ b/text/0000-stabilize-marker-freeze.md @@ -0,0 +1,66 @@ +- Feature Name: `stabilize_marker_freeze` +- Start Date: 2024-05-10 +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary +[summary]: #summary + +Stabilize `core::marker::Freeze` in trait bounds. + +# Motivation +[motivation]: #motivation + +In some contexts, guaranteeing that a value cannot be modified is a requirement to a construct behaving properly (`const` references, keys of a structured map...). + +While this looks achievable by only exposing immutable references, this is actually insufficient due to the (necessary) existence of interior mutability. + +Internally, the compiler uses `core::marker::Freeze` to identify types that are known not to have interior mutability. + +This RFC seeks to stabilize this trait for trait bounds for the following reasons: +- As explained above, some existing constructs are left to "trust" that the values they'd need to freeze won't be mutated through interior mutability. This is a potential bug source for these constructs. +- With [this PR](https://github.com/rust-lang/rust/issues/121250), a breaking change was introduced (and stabilized in 1.78): + - This change prevents associated consts from containing references to values that don't implement `core::marker::Freeze`. Since this bound cannot be added to generics, this prevents associated consts from containing references to generics altogether. + - This change was done in order to prevent potential unsoundness, but it also completely prevents sound uses, such as the ones reported in [this regression issue](https://github.com/rust-lang/rust/issues/123281). + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +This RFC simply seeks to act as the stabilization RFC for `core::marker::Freeze` in trait bounds. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +The work necessary for this RFC has already been done and merged in [this PR](https://github.com/rust-lang/rust/issues/121675), and a [tracking issue](https://github.com/rust-lang/rust/issues/121675) was opened. + +However, it was deemed that adding an Auto Trait to Rust's public API should go through the proper RFC process, but the RFC was never created. + +# Drawbacks +[drawbacks]: #drawbacks + +- Some people have previously argued that this would be akin to exposing compiler internals. + - The RFC author disagrees, viewing `Freeze` in a similar light as `Send` and `Sync`: a trait that allows soundness requirements to be proven at compile time. + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +- This trait has existed for 7 years. Over this course, it was made a compiler internal temporarily, but its usefulness lead to it being re-exposed soon after. +- The benefits of stabilizing `core::mem::Freeze` have been highlighted in [Motivation](#motivation). +- By not stabilizing `core::mem::Freeze` in trait bounds, we are preventing useful and sound code patterns from existing which were previously supported. + +# Prior art +[prior-art]: #prior-art + +This feature has been available in `nightly` for 7 years, and is used internally by the compiler. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +[Should the trait be exposed under a different name?](https://github.com/rust-lang/rust/pull/121501#issuecomment-1962900148) + +# Future possibilities +[future-possibilities]: #future-possibilities + +One might later consider whether `core::mem::Freeze` should be allowed to be `unsafe impl`'d like `Send` and `Sync` are, possibly allowing wrappers around interiorly mutable data to hide this interior mutability from constructs that require `Freeze` if the logic surrounding it guarantees that the interior mutability will never be used. + +This consideration is purposedly left out of scope for this RFC to allow the stabilization of its core interest to go more smoothly; these two debates being completely orthogonal. \ No newline at end of file From 106bc46650c484da2231e65cd24951fdb0799bcf Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Mon, 13 May 2024 20:42:06 +0200 Subject: [PATCH 02/21] Make this a proper RFC --- text/0000-stabilize-marker-freeze.md | 110 ++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 17 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 59347e6c185..52798b38dc8 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -11,47 +11,119 @@ Stabilize `core::marker::Freeze` in trait bounds. # Motivation [motivation]: #motivation -In some contexts, guaranteeing that a value cannot be modified is a requirement to a construct behaving properly (`const` references, keys of a structured map...). +With 1.78, Rust [stabilized](https://github.com/rust-lang/rust/issues/121250) the requirement that `T: core::marker::Freeze` for `const REF: &'a T = T::const_new();` (a pattern referred to as "static-promotion" regardless of whether `'a = 'static`) to be legal. `T: core::marker::Freeze` indicates that `T` doesn't contain any un-indirected `UnsafeCell`, meaning that `T`'s memory cannot be modified through a shared reference. -While this looks achievable by only exposing immutable references, this is actually insufficient due to the (necessary) existence of interior mutability. +The purpose of this change was to ensure that interior mutability cannot affect content that may have been static-promoted in read-only memory, which would be a soundness issue. -Internally, the compiler uses `core::marker::Freeze` to identify types that are known not to have interior mutability. +However, this new requirement also prevents using static-promotion to allow generics to provide a generic equivalent to `static` (with the distinction that static-promotion doesn't guarantee a unique address for the promoted content). An example of this pattern can be found in `stabby` and `equator`'s shared way of constructing v-tables: +```rust +pub trait VTable<'a>: Copy { + const VT: &'a Self; +} +pub struct VtAccumulator { + tail: Tail, + head: Head, +} +impl, Head: VTable<'a>> VTable<'a> for VtAccumulator { + const VT: &'a Self = &Self {tail: *Tail::VT, head: *Head::VT}; // Doesn't compile since 1.78 +} +``` -This RFC seeks to stabilize this trait for trait bounds for the following reasons: -- As explained above, some existing constructs are left to "trust" that the values they'd need to freeze won't be mutated through interior mutability. This is a potential bug source for these constructs. -- With [this PR](https://github.com/rust-lang/rust/issues/121250), a breaking change was introduced (and stabilized in 1.78): - - This change prevents associated consts from containing references to values that don't implement `core::marker::Freeze`. Since this bound cannot be added to generics, this prevents associated consts from containing references to generics altogether. - - This change was done in order to prevent potential unsoundness, but it also completely prevents sound uses, such as the ones reported in [this regression issue](https://github.com/rust-lang/rust/issues/123281). +Making `VTable` a subtrait of `core::marker::Freeze` in this example is sufficient to allow this example to compile again, as static-promotion becomes legal again. This is however impossible as of today due to `core::marker::Freeze` being restricted to `nightly`. + +Orthogonally to static-promotion, `core::marker::Freeze` can also be used to ensure that transmuting `&T` to a reference to an interior-immutable type (such as `[T; core::mem::size_of::()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which currently use their own equivalents of `core::marker::Freeze` to ensure that requirement, which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They would notably benefit from `core::marker::Freeze`'s status as an auto-trait. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -This RFC simply seeks to act as the stabilization RFC for `core::marker::Freeze` in trait bounds. +`core::marker::Freeze` is a trait that indicates the shallow lack of interior mutability in a type: it indicates that the memory referenced by `&T` is guaranteed not to change under defined behaviour. + +It is automatically implemented by the compiler for any type that doesn't shallowly contain a `core::cell::UnsafeCell`. + +Notably, a `const` can only store a reference to a value of type `T` if `T: core::marker::Freeze`, in a pattern named "static-promotion". # Reference-level explanation [reference-level-explanation]: #reference-level-explanation -The work necessary for this RFC has already been done and merged in [this PR](https://github.com/rust-lang/rust/issues/121675), and a [tracking issue](https://github.com/rust-lang/rust/issues/121675) was opened. +The following documentation is lifted from the current nightly documentation. +```markdown +Used to determine whether a type contains +any `UnsafeCell` internally, but not through an indirection. +This affects, for example, whether a `static` of that type is +placed in read-only static memory or writable static memory. +This can be used to declare that a constant with a generic type +will not contain interior mutability, and subsequently allow +placing the constant behind references. +# Safety +This trait is a core part of the language, it is just expressed as a trait in libcore for +convenience. Do *not* implement it for other types. +``` + +The current _Safety_ section may be removed once manual implementation of this trait is forbidden. + +From a cursary review, the following documentation improvements may be considered: + +```markdown +[`Freeze`](core::marker::Freeze) marks all types that do not contain any un-indirected interior mutability. +This means that their byte representation cannot change as long as a reference to them exists. -However, it was deemed that adding an Auto Trait to Rust's public API should go through the proper RFC process, but the RFC was never created. +Note that `T: Freeze` is a shallow property: `T` is still allowed to contain interior mutability, +provided that it is behind an indirection (such as `Box>`). + +Notable interior mutability sources are [`UnsafeCell`](core::cell::UnsafeCell) (and any of its safe wrappers such the types in the [`cell` module](core::cell) or [`Mutex`](std::sync::Mutex)) and [atomics](core::sync::atomic). + +`T: Freeze` is notably a requirement for static promotion (`const REF: &'a T;`) to be legal. Note that static promotion doesn't guarantee a single address: if `REF` is assigned to multiple variables, they may still refer to distinct addresses. + +Whether or not `T: Freeze` may also affect whether `static STATIC: T` is placed in read-only static memory or writeable static memory, or the optimizations that may be performed in code that holds an immutable reference to `T`. +``` + +Mention could be added to `UnsafeCell` and atomics that adding one to a previously `Freeze` type without an indirection (such as a `Box`) is a SemVer hazard, as it will revoque its implementation of `Freeze`. # Drawbacks [drawbacks]: #drawbacks - Some people have previously argued that this would be akin to exposing compiler internals. - The RFC author disagrees, viewing `Freeze` in a similar light as `Send` and `Sync`: a trait that allows soundness requirements to be proven at compile time. +- `Freeze` being an auto-trait, it is, like `Send` and `Sync` a sneaky SemVer hazard. + - Note that this SemVer hazard already exists through the existence of static-promotion, as examplified by the following example: + ```rust + // old version of the crate. + mod v1 { + pub struct S(i32); + impl S { + pub const fn new() -> Self { S(42) } + } + } + + // new version of the crate, adding interior mutability. + mod v2 { + use std::cell::Cell; + pub struct S(Cell); + impl S { + pub const fn new() -> Self { S(Cell::new(42)) } + } + } + + // Old version: builds + const C1: &v1::S = &v1::S::new(); + // New version: does not build + const C2: &v2::S = &v2::S::new(); + ``` # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -- This trait has existed for 7 years. Over this course, it was made a compiler internal temporarily, but its usefulness lead to it being re-exposed soon after. - The benefits of stabilizing `core::mem::Freeze` have been highlighted in [Motivation](#motivation). -- By not stabilizing `core::mem::Freeze` in trait bounds, we are preventing useful and sound code patterns from existing which were previously supported. + - By not stabilizing `core::mem::Freeze` in trait bounds, we are preventing useful and sound code patterns from existing which were previously supported. +- Alternatively, a non-auto sub-trait of `core::mem::Freeze` may be defined: + - While this reduces the SemVer hazard by making its breakage more obvious, this does lose part of the usefulness that `core::mem::Freeze` would provide to projects such as `zerocopy`. + - A "perfect" derive macro should then be introduced to ease the implementation of this trait. A lint may be introduced in `clippy` to inform users of the existence and applicability of this new trait. # Prior art [prior-art]: #prior-art -This feature has been available in `nightly` for 7 years, and is used internally by the compiler. +- This feature has been available in `nightly` for 7 years, and is used internally by the compiler. +- The work necessary for this RFC has already been done and merged in [this PR](https://github.com/rust-lang/rust/issues/121675), and a [tracking issue](https://github.com/rust-lang/rust/issues/121675) was opened. # Unresolved questions [unresolved-questions]: #unresolved-questions @@ -61,6 +133,10 @@ This feature has been available in `nightly` for 7 years, and is used internally # Future possibilities [future-possibilities]: #future-possibilities -One might later consider whether `core::mem::Freeze` should be allowed to be `unsafe impl`'d like `Send` and `Sync` are, possibly allowing wrappers around interiorly mutable data to hide this interior mutability from constructs that require `Freeze` if the logic surrounding it guarantees that the interior mutability will never be used. - -This consideration is purposedly left out of scope for this RFC to allow the stabilization of its core interest to go more smoothly; these two debates being completely orthogonal. \ No newline at end of file +- One might later consider whether `core::mem::Freeze` should be allowed to be `unsafe impl`'d like `Send` and `Sync` are, possibly allowing wrappers around interiorly mutable data to hide this interior mutability from constructs that require `Freeze` if the logic surrounding it guarantees that the interior mutability will never be used. + - The current status-quo is that it cannot be implemented manually (experimentally verified with 2024-05-12's nightly). + - The RFC author is unable to tell whether allowing manual implementation may cause the compiler to generate unsound code (even if the wrapper correctly prevents interior mutation), but judges that the gains of allowing these implementations are too small to justify allowing the risk. + - This consideration is purposedly left out of scope for this RFC to allow the stabilization of its core interest to go more smoothly; these two debates being completely orthogonal. +- Adding a `trait Pure: Freeze` which extends the interior immutability guarantee to indirected data could be valuable: + - This is however likely to be a fool's errand, as indirections could (for example) be hidden behind keys to global collections. + - Providing such a trait could be left to the ecosystem unless we'd want it to be an auto-trait also (unlikely). \ No newline at end of file From 902b79dbc8b386d07da5b1023fe8ea8cb4db9686 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Mon, 13 May 2024 20:48:11 +0200 Subject: [PATCH 03/21] Add line wraps for legibility. --- text/0000-stabilize-marker-freeze.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 52798b38dc8..6ee54c8424c 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -59,7 +59,7 @@ This trait is a core part of the language, it is just expressed as a trait in li convenience. Do *not* implement it for other types. ``` -The current _Safety_ section may be removed once manual implementation of this trait is forbidden. +The current _Safety_ section may be removed, as manual implementation of this trait is forbidden. From a cursary review, the following documentation improvements may be considered: @@ -70,11 +70,17 @@ This means that their byte representation cannot change as long as a reference t Note that `T: Freeze` is a shallow property: `T` is still allowed to contain interior mutability, provided that it is behind an indirection (such as `Box>`). -Notable interior mutability sources are [`UnsafeCell`](core::cell::UnsafeCell) (and any of its safe wrappers such the types in the [`cell` module](core::cell) or [`Mutex`](std::sync::Mutex)) and [atomics](core::sync::atomic). +Notable interior mutability sources are [`UnsafeCell`](core::cell::UnsafeCell) (and any of its safe wrappers +such the types in the [`cell` module](core::cell) or [`Mutex`](std::sync::Mutex)) and [atomics](core::sync::atomic). -`T: Freeze` is notably a requirement for static promotion (`const REF: &'a T;`) to be legal. Note that static promotion doesn't guarantee a single address: if `REF` is assigned to multiple variables, they may still refer to distinct addresses. +`T: Freeze` is notably a requirement for static promotion (`const REF: &'a T;`) to be legal. -Whether or not `T: Freeze` may also affect whether `static STATIC: T` is placed in read-only static memory or writeable static memory, or the optimizations that may be performed in code that holds an immutable reference to `T`. +Note that static promotion doesn't guarantee a single address: if `REF` is assigned to multiple variables, +they may still refer to distinct addresses. + +Whether or not `T: Freeze` may also affect whether `static STATIC: T` is placed +in read-only static memory or writeable static memory, or the optimizations that may be performed +in code that holds an immutable reference to `T`. ``` Mention could be added to `UnsafeCell` and atomics that adding one to a previously `Freeze` type without an indirection (such as a `Box`) is a SemVer hazard, as it will revoque its implementation of `Freeze`. From 126f8f008643ed2e30975413e7783b7cbbff3b48 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Mon, 13 May 2024 22:34:58 +0200 Subject: [PATCH 04/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Joshua Liebow-Feeser --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 6ee54c8424c..334a09507bc 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -31,7 +31,7 @@ impl, Head: VTable<'a>> VTable<'a> for VtAccumulator()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which currently use their own equivalents of `core::marker::Freeze` to ensure that requirement, which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They would notably benefit from `core::marker::Freeze`'s status as an auto-trait. +Orthogonally to static-promotion, `core::marker::Freeze` can also be used to ensure that transmuting `&T` to a reference to an interior-immutable type (such as `[u8; core::mem::size_of::()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which currently use their own equivalents of `core::marker::Freeze` to ensure that requirement, which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They would notably benefit from `core::marker::Freeze`'s status as an auto-trait. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation From 94ef594606d01cdd356209ac8427acdc68f0f768 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 02:01:58 +0200 Subject: [PATCH 05/21] Update 0000-stabilize-marker-freeze.md --- text/0000-stabilize-marker-freeze.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 334a09507bc..a0e076be870 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -31,7 +31,7 @@ impl, Head: VTable<'a>> VTable<'a> for VtAccumulator()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which currently use their own equivalents of `core::marker::Freeze` to ensure that requirement, which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They would notably benefit from `core::marker::Freeze`'s status as an auto-trait. +Orthogonally to static-promotion, `core::marker::Freeze` can also be used to ensure that transmuting `&T` to a reference to an interior-immutable type (such as `[u8; core::mem::size_of::()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which are currently evaluating the use of `core::marker::Freeze` to ensure that requirement; or rolling out their own equivalents (such as zerocopy's `Immutable`) which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They could stand to benefit from `core::marker::Freeze`'s status as an auto-trait, but this isn't a requirement for them, and explicit derivation is considered suitable (at least to zerocopy). # Guide-level explanation [guide-level-explanation]: #guide-level-explanation @@ -130,6 +130,7 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous - This feature has been available in `nightly` for 7 years, and is used internally by the compiler. - The work necessary for this RFC has already been done and merged in [this PR](https://github.com/rust-lang/rust/issues/121675), and a [tracking issue](https://github.com/rust-lang/rust/issues/121675) was opened. +- zerocopy's [`Immutable`](https://docs.rs/zerocopy/0.8.0-alpha.11/zerocopy/trait.Immutable.html) seeks to provide the same guarantees as `core::marker::Freeze`. # Unresolved questions [unresolved-questions]: #unresolved-questions @@ -145,4 +146,4 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous - This consideration is purposedly left out of scope for this RFC to allow the stabilization of its core interest to go more smoothly; these two debates being completely orthogonal. - Adding a `trait Pure: Freeze` which extends the interior immutability guarantee to indirected data could be valuable: - This is however likely to be a fool's errand, as indirections could (for example) be hidden behind keys to global collections. - - Providing such a trait could be left to the ecosystem unless we'd want it to be an auto-trait also (unlikely). \ No newline at end of file + - Providing such a trait could be left to the ecosystem unless we'd want it to be an auto-trait also (unlikely). From 34b97754ff59f1f0e3ea5f38553139ef561a503e Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 14:58:56 +0200 Subject: [PATCH 06/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Andres O. Vela --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index a0e076be870..faacd3f203b 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -18,7 +18,7 @@ The purpose of this change was to ensure that interior mutability cannot affect However, this new requirement also prevents using static-promotion to allow generics to provide a generic equivalent to `static` (with the distinction that static-promotion doesn't guarantee a unique address for the promoted content). An example of this pattern can be found in `stabby` and `equator`'s shared way of constructing v-tables: ```rust pub trait VTable<'a>: Copy { - const VT: &'a Self; + const VT: &'a Self; } pub struct VtAccumulator { tail: Tail, From 3a104b1d3a2a525b4184d71ca2b76189ba88042e Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 14:59:04 +0200 Subject: [PATCH 07/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Andres O. Vela --- text/0000-stabilize-marker-freeze.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index faacd3f203b..da7d7501bf2 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -21,8 +21,8 @@ pub trait VTable<'a>: Copy { const VT: &'a Self; } pub struct VtAccumulator { - tail: Tail, - head: Head, + tail: Tail, + head: Head, } impl, Head: VTable<'a>> VTable<'a> for VtAccumulator { const VT: &'a Self = &Self {tail: *Tail::VT, head: *Head::VT}; // Doesn't compile since 1.78 From 6e15d069c9937eb25608b40b0beb0e3090c0715b Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 14:59:10 +0200 Subject: [PATCH 08/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Andres O. Vela --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index da7d7501bf2..dc0c555f473 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -83,7 +83,7 @@ in read-only static memory or writeable static memory, or the optimizations that in code that holds an immutable reference to `T`. ``` -Mention could be added to `UnsafeCell` and atomics that adding one to a previously `Freeze` type without an indirection (such as a `Box`) is a SemVer hazard, as it will revoque its implementation of `Freeze`. +Mention could be added to `UnsafeCell` and atomics that adding one to a previously `Freeze` type without an indirection (such as a `Box`) is a SemVer hazard, as it will revoke its implementation of `Freeze`. # Drawbacks [drawbacks]: #drawbacks From c5b3fe858419681fa11098671421ac6ea47ec9a8 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 14:59:16 +0200 Subject: [PATCH 09/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Andres O. Vela --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index dc0c555f473..199d5df88a9 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -61,7 +61,7 @@ convenience. Do *not* implement it for other types. The current _Safety_ section may be removed, as manual implementation of this trait is forbidden. -From a cursary review, the following documentation improvements may be considered: +From a cursory review, the following documentation improvements may be considered: ```markdown [`Freeze`](core::marker::Freeze) marks all types that do not contain any un-indirected interior mutability. From 2b4f996d74355ff02b92e7a896740b9ef3412be4 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 14:59:23 +0200 Subject: [PATCH 10/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Andres O. Vela --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 199d5df88a9..a886c0567ee 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -91,7 +91,7 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous - Some people have previously argued that this would be akin to exposing compiler internals. - The RFC author disagrees, viewing `Freeze` in a similar light as `Send` and `Sync`: a trait that allows soundness requirements to be proven at compile time. - `Freeze` being an auto-trait, it is, like `Send` and `Sync` a sneaky SemVer hazard. - - Note that this SemVer hazard already exists through the existence of static-promotion, as examplified by the following example: + - Note that this SemVer hazard already exists through the existence of static-promotion, as exemplified by the following example: ```rust // old version of the crate. mod v1 { From 33ffcc65e0e0bd31ff5dd215a69a6fa96549fcf2 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 14 May 2024 14:59:31 +0200 Subject: [PATCH 11/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Andres O. Vela --- text/0000-stabilize-marker-freeze.md | 42 ++++++++++++++-------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index a886c0567ee..3ce34fcc13d 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -93,27 +93,27 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous - `Freeze` being an auto-trait, it is, like `Send` and `Sync` a sneaky SemVer hazard. - Note that this SemVer hazard already exists through the existence of static-promotion, as exemplified by the following example: ```rust - // old version of the crate. - mod v1 { - pub struct S(i32); - impl S { - pub const fn new() -> Self { S(42) } - } - } - - // new version of the crate, adding interior mutability. - mod v2 { - use std::cell::Cell; - pub struct S(Cell); - impl S { - pub const fn new() -> Self { S(Cell::new(42)) } - } - } - - // Old version: builds - const C1: &v1::S = &v1::S::new(); - // New version: does not build - const C2: &v2::S = &v2::S::new(); + // old version of the crate. + mod v1 { + pub struct S(i32); + impl S { + pub const fn new() -> Self { S(42) } + } + } + + // new version of the crate, adding interior mutability. + mod v2 { + use std::cell::Cell; + pub struct S(Cell); + impl S { + pub const fn new() -> Self { S(Cell::new(42)) } + } + } + + // Old version: builds + const C1: &v1::S = &v1::S::new(); + // New version: does not build + const C2: &v2::S = &v2::S::new(); ``` # Rationale and alternatives From b339b0d364969c6c60a422f97b9f216042c602d7 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 22 May 2024 20:23:20 +0200 Subject: [PATCH 12/21] Address a batch of comments with actionnable suggestions --- text/0000-stabilize-marker-freeze.md | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 3ce34fcc13d..359816b3a5e 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -11,10 +11,11 @@ Stabilize `core::marker::Freeze` in trait bounds. # Motivation [motivation]: #motivation -With 1.78, Rust [stabilized](https://github.com/rust-lang/rust/issues/121250) the requirement that `T: core::marker::Freeze` for `const REF: &'a T = T::const_new();` (a pattern referred to as "static-promotion" regardless of whether `'a = 'static`) to be legal. `T: core::marker::Freeze` indicates that `T` doesn't contain any un-indirected `UnsafeCell`, meaning that `T`'s memory cannot be modified through a shared reference. +With 1.78, Rust [changed behavior](https://github.com/rust-lang/rust/issues/121250): previously, `const REF: &T = &expr;` was (accidentally) accepted even when `expr` may contain interior mutability. +Now this requires that the type of `expr` satisfies `T: core::marker::Freeze`, which indicates that `T` doesn't contain any un-indirected `UnsafeCell`, meaning that `T`'s memory cannot be modified through a shared reference. The purpose of this change was to ensure that interior mutability cannot affect content that may have been static-promoted in read-only memory, which would be a soundness issue. - +However, this new requirement also prevents using static-promotion to create constant references to data of generic type. This pattern can be used to approximate "generic `static`s" (with the distinction that static-promotion doesn't guarantee a unique address for the promoted content). An example of this pattern can be found in `stabby` and `equator`'s shared way of constructing v-tables: However, this new requirement also prevents using static-promotion to allow generics to provide a generic equivalent to `static` (with the distinction that static-promotion doesn't guarantee a unique address for the promoted content). An example of this pattern can be found in `stabby` and `equator`'s shared way of constructing v-tables: ```rust pub trait VTable<'a>: Copy { @@ -31,14 +32,16 @@ impl, Head: VTable<'a>> VTable<'a> for VtAccumulator()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which are currently evaluating the use of `core::marker::Freeze` to ensure that requirement; or rolling out their own equivalents (such as zerocopy's `Immutable`) which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They could stand to benefit from `core::marker::Freeze`'s status as an auto-trait, but this isn't a requirement for them, and explicit derivation is considered suitable (at least to zerocopy). +Orthogonally to static-promotion, `core::marker::Freeze` can also be used to ensure that transmuting `&T` to a reference to an interior-immutable type (such as `[u8; core::mem::size_of::()]`) is sound (as interior-mutation of a `&T` may lead to UB in code using the transmuted reference, as it expects that reference's pointee to never change). This is notably a safety requirement for `zerocopy` and `bytemuck` which are currently evaluating the use of `core::marker::Freeze` to ensure that requirement; or rolling out their own equivalents (such as zerocopy's `Immutable`) which imposes great maintenance pressure on these crates to ensure they support as many types as possible. They could stand to benefit from `core::marker::Freeze`'s status as an auto-trait, and `zerocopy` intends to replace its bespoke trait with a re-export of `core::marker::Freeze`. + +Note that for this latter use-case, `core::marker::Freeze` isn't entirely sufficient, as an additional proof that `T` doesn't contain padding bytes is necessary to allow this transmutation to be safe, as reading one of `T`'s padding bytes as a `u8` would be UB. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -`core::marker::Freeze` is a trait that indicates the shallow lack of interior mutability in a type: it indicates that the memory referenced by `&T` is guaranteed not to change under defined behaviour. +`core::marker::Freeze` is a trait that is implemented for any type whose memory layout doesn't contain any `UnsafeCell`: it indicates that the memory referenced by `&T` is guaranteed not to change while the reference is live. -It is automatically implemented by the compiler for any type that doesn't shallowly contain a `core::cell::UnsafeCell`. +It is automatically implemented by the compiler for any type that doesn't contain an un-indirected `core::cell::UnsafeCell`. Notably, a `const` can only store a reference to a value of type `T` if `T: core::marker::Freeze`, in a pattern named "static-promotion". @@ -59,8 +62,6 @@ This trait is a core part of the language, it is just expressed as a trait in li convenience. Do *not* implement it for other types. ``` -The current _Safety_ section may be removed, as manual implementation of this trait is forbidden. - From a cursory review, the following documentation improvements may be considered: ```markdown @@ -69,9 +70,9 @@ This means that their byte representation cannot change as long as a reference t Note that `T: Freeze` is a shallow property: `T` is still allowed to contain interior mutability, provided that it is behind an indirection (such as `Box>`). - -Notable interior mutability sources are [`UnsafeCell`](core::cell::UnsafeCell) (and any of its safe wrappers -such the types in the [`cell` module](core::cell) or [`Mutex`](std::sync::Mutex)) and [atomics](core::sync::atomic). +Notable `!Freeze` types are [`UnsafeCell`](core::cell::UnsafeCell) and its safe wrappers +such as the types in the [`cell` module](core::cell), [`Mutex`](std::sync::Mutex), and [atomics](core::sync::atomic). +Any type which contains any one of these without indirection is also `!Freeze`. `T: Freeze` is notably a requirement for static promotion (`const REF: &'a T;`) to be legal. @@ -81,6 +82,10 @@ they may still refer to distinct addresses. Whether or not `T: Freeze` may also affect whether `static STATIC: T` is placed in read-only static memory or writeable static memory, or the optimizations that may be performed in code that holds an immutable reference to `T`. + +# Safety +This trait is a core part of the language, it is just expressed as a trait in libcore for +convenience. Do *not* implement it for other types. ``` Mention could be added to `UnsafeCell` and atomics that adding one to a previously `Freeze` type without an indirection (such as a `Box`) is a SemVer hazard, as it will revoke its implementation of `Freeze`. @@ -125,7 +130,9 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous - While this reduces the SemVer hazard by making its breakage more obvious, this does lose part of the usefulness that `core::mem::Freeze` would provide to projects such as `zerocopy`. - A "perfect" derive macro should then be introduced to ease the implementation of this trait. A lint may be introduced in `clippy` to inform users of the existence and applicability of this new trait. -# Prior art +- This trait has a long history: it existed in ancient times but got [removed](https://github.com/rust-lang/rust/pull/13076) before Rust 1.0. + In 2017 it got [added back](https://github.com/rust-lang/rust/pull/41349) as a way to simplify the implementation of the `interior_unsafe` query, but it was kept private to the standard library. + In 2019, a [request](https://github.com/rust-lang/rust/issues/60715) was filed to publicly expose the trait, but not a lot happened until recently when the issue around static promotion led to it being [exposed unstably](https://github.com/rust-lang/rust/pull/121840). [prior-art]: #prior-art - This feature has been available in `nightly` for 7 years, and is used internally by the compiler. From 30a03fcdaba92430e122fecdd95bfe587c0226f8 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 22 May 2024 20:44:10 +0200 Subject: [PATCH 13/21] Update remaining questions --- text/0000-stabilize-marker-freeze.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 359816b3a5e..c862f575d2b 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -142,7 +142,9 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous # Unresolved questions [unresolved-questions]: #unresolved-questions -[Should the trait be exposed under a different name?](https://github.com/rust-lang/rust/pull/121501#issuecomment-1962900148) +- [Should the trait be exposed under a different name?](https://github.com/rust-lang/rust/pull/121501#issuecomment-1962900148) + - An appealing proposition is `ShallowImmutable` to avoid collision with `llvm`'s `freeze`, while highlighting that the property is "shallow". +- Should an explicit `PhantomNotFreeze` (`PhantomNotShallowImmut`?) be provided in the same stride as a more explicit way for maintainers to opt out of `Freeze` without resorting to `UnsafeCell<()>` which can currently provide that function, but whose design is still [under question](https://github.com/rust-lang/unsafe-code-guidelines/issues/236) # Future possibilities [future-possibilities]: #future-possibilities From c8fa805d07418aead4f6688faa8bc119788a9633 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 22 May 2024 21:44:43 +0200 Subject: [PATCH 14/21] Propose Freeze->ShallowImmutable and PhantomNotFreeze --- text/0000-stabilize-marker-freeze.md | 43 ++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index c862f575d2b..025b2d68c93 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -6,7 +6,9 @@ # Summary [summary]: #summary -Stabilize `core::marker::Freeze` in trait bounds. +- Stabilize `core::marker::Freeze` in trait bounds. + - Rename `core::marker::Freeze` to `core::marker::ShallowImmutable`. This proposition is tentative, the RFC will keep on using the historical `core::marker::Freeze` name. + - Provide a marker type to opt out of `core::marker::Freeze` for the most semver-conscious maintainers. Tentatively named `core::marker::PhantomNotFreeze` (or `core::marker::PhantomNotShallowImmutable` to go with the proposed rename) # Motivation [motivation]: #motivation @@ -45,9 +47,13 @@ It is automatically implemented by the compiler for any type that doesn't contai Notably, a `const` can only store a reference to a value of type `T` if `T: core::marker::Freeze`, in a pattern named "static-promotion". +As `core::marker::Freeze` is an auto-trait, it poses an inherent semver-hazard (which is already exposed through static-promotion): this RFC proposes the simultaneous addition and stabilization of a `core::marker::PhantomNotFreeze` type to provide a stable mean for maintainers to reliably opt out of `Freeze` without forbidding zero-sized types that are currently `!Freeze` due to the conservativeness of `Freeze`'s implementation being locked into remaining `!Freeze`. + # Reference-level explanation [reference-level-explanation]: #reference-level-explanation +## `core::marker::Freeze` + The following documentation is lifted from the current nightly documentation. ```markdown Used to determine whether a type contains @@ -65,7 +71,7 @@ convenience. Do *not* implement it for other types. From a cursory review, the following documentation improvements may be considered: ```markdown -[`Freeze`](core::marker::Freeze) marks all types that do not contain any un-indirected interior mutability. +[`Freeze`] marks all types that do not contain any un-indirected interior mutability. This means that their byte representation cannot change as long as a reference to them exists. Note that `T: Freeze` is a shallow property: `T` is still allowed to contain interior mutability, @@ -83,6 +89,18 @@ Whether or not `T: Freeze` may also affect whether `static STATIC: T` is placed in read-only static memory or writeable static memory, or the optimizations that may be performed in code that holds an immutable reference to `T`. +# Semver hazard +`Freeze` being an auto-trait, it may leak private properties of your types to semver. +Specifically, adding an `UnsafeCell` to a type's layout is a _major_ breaking change, +as it removes a trait implementation from it. + +## The ZST caveat +While `UnsafeCell` is currently `!Freeze` regardless of `T`, allowing `UnsafeCell: Freeze` iff `T` is +a Zero-Sized-Type is currently under consideration. + +Therefore, the advised way to make your types `!Freeze` regardless of their actual contents is to add a +[`PhantomNotFreeze`](core::marker::PhantomNotFreeze) field to it. + # Safety This trait is a core part of the language, it is just expressed as a trait in libcore for convenience. Do *not* implement it for other types. @@ -90,6 +108,26 @@ convenience. Do *not* implement it for other types. Mention could be added to `UnsafeCell` and atomics that adding one to a previously `Freeze` type without an indirection (such as a `Box`) is a SemVer hazard, as it will revoke its implementation of `Freeze`. +## `core::marker::PhantomNotFreeze` + +This ZST is proposed as a means for maintainers to reliably opt out of `Freeze` without constraining currently `!Freeze` ZSTs to remain so. While the RFC author doesn't have the expertise to produce its code, +here's its propsed documentation: + +```markdown +[`PhantomNotFreeze`] is type with the following guarantees: +- It is guaranteed not to affect the layout of a type containing it as a field. +- Any type including it in its fields (including nested fields) without indirection is guaranteed to be `!Freeze`. + +This latter property is [`PhantomNotFreeze`]'s raison-d'être: while other Zero-Sized-Types may currently be `!Freeze`, +[`PhantomNotFreeze`] is the only ZST that's guaranteed to keep that bound. + +Notable types that are currently `!Freeze` but might not remain so in the future are: +- `UnsafeCell` where `core::mem::size_of::() == 0` +- `[T; 0]` where `T: !Freeze`. + +Note that `core::marker::PhantomData` if `Freeze` regardless of `T`'s `Freeze`ness. +``` + # Drawbacks [drawbacks]: #drawbacks @@ -144,7 +182,6 @@ Mention could be added to `UnsafeCell` and atomics that adding one to a previous - [Should the trait be exposed under a different name?](https://github.com/rust-lang/rust/pull/121501#issuecomment-1962900148) - An appealing proposition is `ShallowImmutable` to avoid collision with `llvm`'s `freeze`, while highlighting that the property is "shallow". -- Should an explicit `PhantomNotFreeze` (`PhantomNotShallowImmut`?) be provided in the same stride as a more explicit way for maintainers to opt out of `Freeze` without resorting to `UnsafeCell<()>` which can currently provide that function, but whose design is still [under question](https://github.com/rust-lang/unsafe-code-guidelines/issues/236) # Future possibilities [future-possibilities]: #future-possibilities From 492a594923815feef22ef7ee03b9aedc75d8f159 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 22 May 2024 22:24:43 +0200 Subject: [PATCH 15/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 025b2d68c93..b98b116943f 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -125,7 +125,7 @@ Notable types that are currently `!Freeze` but might not remain so in the future - `UnsafeCell` where `core::mem::size_of::() == 0` - `[T; 0]` where `T: !Freeze`. -Note that `core::marker::PhantomData` if `Freeze` regardless of `T`'s `Freeze`ness. +Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze`ness. ``` # Drawbacks From c01e96cb5ae64f6109b0bbff8b503b8626399728 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 22 May 2024 23:34:40 +0200 Subject: [PATCH 16/21] Update 0000-stabilize-marker-freeze.md --- text/0000-stabilize-marker-freeze.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index b98b116943f..1778480c3aa 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -168,12 +168,11 @@ Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze - While this reduces the SemVer hazard by making its breakage more obvious, this does lose part of the usefulness that `core::mem::Freeze` would provide to projects such as `zerocopy`. - A "perfect" derive macro should then be introduced to ease the implementation of this trait. A lint may be introduced in `clippy` to inform users of the existence and applicability of this new trait. +# Prior Art +[prior-art]: #prior-art - This trait has a long history: it existed in ancient times but got [removed](https://github.com/rust-lang/rust/pull/13076) before Rust 1.0. In 2017 it got [added back](https://github.com/rust-lang/rust/pull/41349) as a way to simplify the implementation of the `interior_unsafe` query, but it was kept private to the standard library. In 2019, a [request](https://github.com/rust-lang/rust/issues/60715) was filed to publicly expose the trait, but not a lot happened until recently when the issue around static promotion led to it being [exposed unstably](https://github.com/rust-lang/rust/pull/121840). -[prior-art]: #prior-art - -- This feature has been available in `nightly` for 7 years, and is used internally by the compiler. - The work necessary for this RFC has already been done and merged in [this PR](https://github.com/rust-lang/rust/issues/121675), and a [tracking issue](https://github.com/rust-lang/rust/issues/121675) was opened. - zerocopy's [`Immutable`](https://docs.rs/zerocopy/0.8.0-alpha.11/zerocopy/trait.Immutable.html) seeks to provide the same guarantees as `core::marker::Freeze`. From 405b3226e403ede663397378d3b83ee971a92089 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Sat, 25 May 2024 11:27:35 +0200 Subject: [PATCH 17/21] Add motivation for the trait renaming --- text/0000-stabilize-marker-freeze.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 1778480c3aa..ee195c765b9 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -38,6 +38,8 @@ Orthogonally to static-promotion, `core::marker::Freeze` can also be used to ens Note that for this latter use-case, `core::marker::Freeze` isn't entirely sufficient, as an additional proof that `T` doesn't contain padding bytes is necessary to allow this transmutation to be safe, as reading one of `T`'s padding bytes as a `u8` would be UB. +Renaming the trait to `core::marker::ShallowImmutable` is desirable because `freeze` is already a term used in `llvm` to refer to an intrinsic which allows to safely read from uninitialized memory. [Another RFC](https://github.com/rust-lang/rfcs/pull/3605) is currently open to expose this intrinsic in Rust. + # Guide-level explanation [guide-level-explanation]: #guide-level-explanation From 14abf77518e9284a222c57145f57e17f3c32d113 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Sun, 26 May 2024 09:23:24 +0200 Subject: [PATCH 18/21] Update text/0000-stabilize-marker-freeze.md Co-authored-by: Ralf Jung --- text/0000-stabilize-marker-freeze.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index ee195c765b9..7b7c694f99b 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -189,7 +189,8 @@ Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze - One might later consider whether `core::mem::Freeze` should be allowed to be `unsafe impl`'d like `Send` and `Sync` are, possibly allowing wrappers around interiorly mutable data to hide this interior mutability from constructs that require `Freeze` if the logic surrounding it guarantees that the interior mutability will never be used. - The current status-quo is that it cannot be implemented manually (experimentally verified with 2024-05-12's nightly). - - The RFC author is unable to tell whether allowing manual implementation may cause the compiler to generate unsound code (even if the wrapper correctly prevents interior mutation), but judges that the gains of allowing these implementations are too small to justify allowing the risk. + - An `unsafe impl Freeze for T` would have very subtle soundness constraints: with such a declaration, performing mutation through any `&T` *or any pointer derived from it* would be UB. So this completely disables any interior mutability on fields of `T` with absolutely no way of ever recovering mutability. + - Given these tight constraints, it is unclear what a concrete use-case for `unsafe impl Freeze` would be. So far, none has been found. - This consideration is purposedly left out of scope for this RFC to allow the stabilization of its core interest to go more smoothly; these two debates being completely orthogonal. - Adding a `trait Pure: Freeze` which extends the interior immutability guarantee to indirected data could be valuable: - This is however likely to be a fool's errand, as indirections could (for example) be hidden behind keys to global collections. From c1fedd562c7bd4c5d02c5e9904ad909852970872 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Sat, 1 Jun 2024 00:37:45 +0200 Subject: [PATCH 19/21] Update text/0000-stabilize-marker-freeze.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gábor Lehel --- text/0000-stabilize-marker-freeze.md | 1 - 1 file changed, 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 7b7c694f99b..0574a2d2b89 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -18,7 +18,6 @@ Now this requires that the type of `expr` satisfies `T: core::marker::Freeze`, w The purpose of this change was to ensure that interior mutability cannot affect content that may have been static-promoted in read-only memory, which would be a soundness issue. However, this new requirement also prevents using static-promotion to create constant references to data of generic type. This pattern can be used to approximate "generic `static`s" (with the distinction that static-promotion doesn't guarantee a unique address for the promoted content). An example of this pattern can be found in `stabby` and `equator`'s shared way of constructing v-tables: -However, this new requirement also prevents using static-promotion to allow generics to provide a generic equivalent to `static` (with the distinction that static-promotion doesn't guarantee a unique address for the promoted content). An example of this pattern can be found in `stabby` and `equator`'s shared way of constructing v-tables: ```rust pub trait VTable<'a>: Copy { const VT: &'a Self; From 04e39d48ffa6debe91f776d537f16c864a50fca1 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Sat, 27 Jul 2024 16:18:00 +0200 Subject: [PATCH 20/21] Update RFC following the 2024-07-24 design meeting --- text/0000-stabilize-marker-freeze.md | 36 +++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index 0574a2d2b89..e8416375193 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -79,21 +79,22 @@ Note that `T: Freeze` is a shallow property: `T` is still allowed to contain int provided that it is behind an indirection (such as `Box>`). Notable `!Freeze` types are [`UnsafeCell`](core::cell::UnsafeCell) and its safe wrappers such as the types in the [`cell` module](core::cell), [`Mutex`](std::sync::Mutex), and [atomics](core::sync::atomic). -Any type which contains any one of these without indirection is also `!Freeze`. +Any type which contains a `!Freeze` type without indirection is also `!Freeze`. `T: Freeze` is notably a requirement for static promotion (`const REF: &'a T;`) to be legal. Note that static promotion doesn't guarantee a single address: if `REF` is assigned to multiple variables, they may still refer to distinct addresses. -Whether or not `T: Freeze` may also affect whether `static STATIC: T` is placed +Whether or not `T` implements `Freeze` may also affect whether `static STATIC: T` is placed in read-only static memory or writeable static memory, or the optimizations that may be performed in code that holds an immutable reference to `T`. # Semver hazard `Freeze` being an auto-trait, it may leak private properties of your types to semver. -Specifically, adding an `UnsafeCell` to a type's layout is a _major_ breaking change, -as it removes a trait implementation from it. +Specifically, adding a non-`Freeze` field to a type's layout is a _major_ breaking change, +as it removes a trait implementation from it. Removing the last non-`Freeeze` field of a type's +layout is a _minor_ breaking change, as it adds a trait implementation to that type. ## The ZST caveat While `UnsafeCell` is currently `!Freeze` regardless of `T`, allowing `UnsafeCell: Freeze` iff `T` is @@ -129,6 +130,8 @@ Notable types that are currently `!Freeze` but might not remain so in the future Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze`ness. ``` +The note on `PhantomData` is part of the RFC to reflect the current status, which cannot be changed without causing breakage. + # Drawbacks [drawbacks]: #drawbacks @@ -159,6 +162,7 @@ Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze // New version: does not build const C2: &v2::S = &v2::S::new(); ``` + - The provided example is also, in RFC author's estimation, the main way in which `Freeze` is likely to be depended upon: allowing bounds on it will likely not expand the hazard much. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives @@ -180,8 +184,27 @@ Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze # Unresolved questions [unresolved-questions]: #unresolved-questions -- [Should the trait be exposed under a different name?](https://github.com/rust-lang/rust/pull/121501#issuecomment-1962900148) - - An appealing proposition is `ShallowImmutable` to avoid collision with `llvm`'s `freeze`, while highlighting that the property is "shallow". +- [Should the trait be exposed under a different name?](https://github.com/rust-lang/rust/pull/121501#issuecomment-1962900148) to avoid collisions with `llvm`'s `freeze`? Here are the options cited during the design meeting: + - `Freeze` has become part of the Rustacean jargon, should `llvm`'s `freeze` get a new name in the Rust jargon instead and this trait keep its current name? + - `ShallowImmutable` + - `LocalImmutable` + - `InlineImmutable` + - `ValueImmutable` + - `DirectImmutable` + - `InteriorImmutable` +- How concerned are we about `Freeze` as a semver hazard? + - `Freeze` and `PhantomNotFreeze` could be stabilized at the same time. + - `PhantomNotFreeze` could be stabilized first, offering library authors a grace period to include it in their types before `Freeze` gets stabilized, adding new ways to depend on a type implementing it. + - Regardless, semver compliance guides and tools should be updated to ensure they handle it properly. +- What should be done about `PhantomData` historically implementing `Freeze` regardless of whether or not `T` does? + - We could simply ship `PhantomNotFreeze` (alternative names: `PhantomMutable`, `PhantomInteriorMutable`) as the RFC proposes. + - We could provide `PhantomFreezeIf` which would be equivalent to `PhantomData`, except it would only impl `Freeze` if `T` does. + - While slightly more complex than `PhantomNotFreeze` on the surface, this option does grant more flexibility. `type PhantomNotFreeze = PhantomFrezeIf>` could still be defined for convenience. + - We could make `PhantomData` communicate `Freeze` the way it does `Send`, `Sync` and `Unpin`. + - While this makes the behaviour more consistent, it may cause substantial breakage. + - If such an option was taken, crates that use `PhantomData` to keep information about a type next to an indirected representation of it would need to be guided to modify it to `PhantomData<&'static T>`. +- Does `PhantomNotFreeze` have any operation semantics consequences? As in: is the exception of "mutation allowed behind shared references" tied to `UnsafeCell` or to the newly public trait? + - If tied to `UnsafeCell`, the guarantee that any type containing `UnsafeCell` must never implement the trait should be ensured; this is the case as long as `unsafe impl Freeze` is disallowed. # Future possibilities [future-possibilities]: #future-possibilities @@ -194,3 +217,4 @@ Note that `core::marker::PhantomData` is `Freeze` regardless of `T`'s `Freeze - Adding a `trait Pure: Freeze` which extends the interior immutability guarantee to indirected data could be valuable: - This is however likely to be a fool's errand, as indirections could (for example) be hidden behind keys to global collections. - Providing such a trait could be left to the ecosystem unless we'd want it to be an auto-trait also (unlikely). +- `impl !Freeze for T` could be a nice alternative to `PhantomNotFreeze`. However, this would be a significant language change that needs much deeper consideration. From c7eab7925abb14ffd9a37a3a992896cc0330f43e Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Sun, 28 Jul 2024 10:52:04 +0200 Subject: [PATCH 21/21] Apply suggestions from code review Co-authored-by: zachs18 <8355914+zachs18@users.noreply.github.com> --- text/0000-stabilize-marker-freeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-stabilize-marker-freeze.md b/text/0000-stabilize-marker-freeze.md index e8416375193..73eb9a0cb58 100644 --- a/text/0000-stabilize-marker-freeze.md +++ b/text/0000-stabilize-marker-freeze.md @@ -79,7 +79,7 @@ Note that `T: Freeze` is a shallow property: `T` is still allowed to contain int provided that it is behind an indirection (such as `Box>`). Notable `!Freeze` types are [`UnsafeCell`](core::cell::UnsafeCell) and its safe wrappers such as the types in the [`cell` module](core::cell), [`Mutex`](std::sync::Mutex), and [atomics](core::sync::atomic). -Any type which contains a `!Freeze` type without indirection is also `!Freeze`. +Any type which contains a non-`Freeze` type without indirection also does not implement `Freeze`. `T: Freeze` is notably a requirement for static promotion (`const REF: &'a T;`) to be legal.