From b9fa34754b4e8596821c08d272b8d2678dd6138f Mon Sep 17 00:00:00 2001 From: tyler Date: Fri, 12 Jun 2020 18:14:50 -0700 Subject: [PATCH 1/7] rfc to add the Freeze trait to libcore/libstd --- text/0000-freeze.md | 155 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 text/0000-freeze.md diff --git a/text/0000-freeze.md b/text/0000-freeze.md new file mode 100644 index 00000000000..a93f53da372 --- /dev/null +++ b/text/0000-freeze.md @@ -0,0 +1,155 @@ +- Feature Name: `freeze` +- Start Date: 2020-06-12 +- RFC PR: [rust-lang/rfcs#2944](https://github.com/rust-lang/rfcs/pull/2944) +- Rust Issue: TBD + +# Summary +[summary]: #summary + +This RFC introduces new APIs to libcore/libstd to serve as safe abstractions for data which has no "shallow" interior mutability. + +```rust +pub unsafe auto trait Freeze {} +pub struct PhantomUnfrozen; +``` + +# Motivation +[motivation]: #motivation + +It is occasionally necessary in systems programming to know whether the range of bytes occupied by a value is truly immutable. Given that rust has interior mutability, there is currently no way to represent this immutability in the type system. + +## Read Only Memory + +If a type is suitable for read only memory, then it cannot have any interior mutability. For example, an `AtomicU8` is a poor candidate for being put into read only memory because the type system has no way to ensure that type is not mutated. It is, however, allowed to put a `Box` in read only memory as long as the heap allocation remains in writable memory. + +The [main reason](https://github.com/rust-lang/rust/blob/84ec8238b14b4cf89e82eae11907b59629baff2c/src/libcore/marker.rs#L702) libcore has a private version of `Freeze` is to decide: +> whether a `static` of that type is placed in read-only static memory or writable static memory + +Another example of read only memory includes read only memory mappings. + +## Optimistic Concurrency + +Optimistic concurrency (e.g. seqlocks, software transactional memory) relies heavily on retrieving shallow snapshots of memory. These snapshots can then be treated as read only references to the original data as long as no mutation occurs. In the case of interior mutability (e.g. `Mutex`), this falls apart. + +One example coming from [`swym`](https://docs.rs/swym/0.1.0-preview/swym/tcell/struct.TCell.html#method.borrow) is the method `borrow`. `borrow` returns snapshots of data - shallow memcpys - that are guaranteed to not be torn, and be valid for the duration of the containing transaction. These snapshots hold on to the lifetime of the `TCell` in order to act like a true reference, without blocking updates to the `TCell` from other threads. Other threads promise to not mutate the value that had its snapshot taken until the transaction has finished, but are permitted to move the value in memory. In the presence of interior mutability, these snapshots differ significantly from a true reference. + +The following example uses a `Mutex` (a `Send`/`Sync`, but not `Freeze` type to create UB): + +```rust +let x = TCell::new(Mutex::new("hello there".to_owned())); + +// .. inside a transaction +let shallow_copy = x.borrow(tx, Default::default())?; +// Locking a shallow copy of a lock... is not really a lock at all! +// The original String is deallocated here, likely leading to double-frees. +*shallow_copy.lock().unwrap() = "uh oh".to_owned(); +``` + +By having snapshotting functions like `borrow` require `Freeze`, such disastrous situations are prevented at compile time, without being overly restrictive, or requiring slower heap allocation based workarounds. + +Similarly to the above example, `crossbeam` would be able to expand `Atomic` to include non-copy types. See [this](https://github.com/crossbeam-rs/crossbeam/issues/379) issue. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +`Freeze` is a new marker trait, similar to `Send` and `Sync`, that is intended to only be implemented for types which have no direct interior mutability, and are therefore safe to place in read only memory. + +## What types are `Freeze`? + +The list of `Freeze` types is long, including primitives, `String`, `Vec`, `Option`, `Box`, `Arc`, `Rc`, etc. This is because you cannot modify the memory contained directly within these types through an immutable reference. + +Types that do not implement `Freeze` include types used in parallel programming such as, `Mutex`, `AtomicUsize`, etc, as well as `Cell`, `RefCell`, and `UnsafeCell`. This is because their memory can be modified via an immutable reference. + +## My type doesn't implement `Freeze`, but I need it to be `Freeze`. + +To convert a type which is not `Freeze`, into a `Freeze` type, all that is required is to stick it on the heap. For example, `Box` is `Freeze` even if `T` is an `UnsafeCell`. + +If you really know what you are doing, and promise not to mutate any data in your type through an immutable reference, then you can implement `Freeze` like so: + +```rust +struct MyType { /* .. */ } +unsafe impl Freeze for MyType {} +``` + +This requires `unsafe`, because UB is possible if in fact the memory occupied by `MyType` is mutable through an immutable reference to `MyType`. + +## How do I opt-out of `Freeze`? + +This is only useful when you suspect your type might, at some point in the future, include a non-`Freeze` type. To protect your users from relying on the current implementation of your type, simply add `PhantomUnfrozen` as a member to your type. + +```rust +struct MyType { + _dont_rely_on_freeze: PhantomUnfrozen, +} +``` + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +`Freeze` has been privately implemented in libcore for 3 years, and has not had major changes during that time. In that time it has been relied upon for deciding whether a `static` of a type is placed in read-only static memory or writable static memory. + +`Freeze` needs to be made `pub` instead of `pub(crate)`. `PhantomUnfrozen` would be a new addition. + +## Implementation + +`libcore/marker.rs`: +```rust +#[lang = "freeze"] +pub unsafe auto trait Freeze {} + +impl !Freeze for UnsafeCell {} +unsafe impl Freeze for PhantomData {} +unsafe impl Freeze for *const T {} +unsafe impl Freeze for *mut T {} +unsafe impl Freeze for &T {} +unsafe impl Freeze for &mut T {} + +pub struct PhantomUnfrozen; +impl !Freeze for PhantomUnfrozen {} +``` + +# Drawbacks +[drawbacks]: #drawbacks + +Adding a new `auto` trait typically complicates the language and adds cognitive overhead for public crates, `Freeze` is no exception. Crate owners have to now commit to an interior mutability story, or risk breaking changes in the future. + +The community desire for `Freeze` is also currently small. + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +This design has been relied on by rustc for 3 years, and has not required any significant maintenence, nor does this author expect there to be much maintenence after making it `pub`. + +Crate owners who incidentally have `Freeze` types in their API, and wish to add in interior mutability at a later date, can do so by simply `Box`ing up any parts of their type which may be modified through an immutable reference to avoid breaking changes. + +No other designs have been considered. + +The impact of not doing this would be admittedly small. Users who want this feature would have to wait for `optin-builtin-traits`, use nightly rust, `Box` up data they intend to `Freeze`, or rely on `unsafe` code. This RFC author would elect to keep [`swym`](https://github.com/mtak-/swym) on nightly rust rather than pay the performance overhead of heap allocation. + +# Prior art +[prior-art]: #prior-art + +This feature has existed internally in libcore for 3 years without any fuss. + +The D programming language has a similar feature known as [immutable references](https://dlang.org/spec/const3.html#const_and_immutable). The main difference is that `Freeze`'s immutability is not tracked across any contained pointers, like it is in D; however, they use it for similar purposes: +> Immutable data can be placed in ROM (Read Only Memory) or in memory pages marked by the hardware as read only. Since immutable data does not change, it enables many opportunities for program optimization, and has applications in functional style programming. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +## Design questions +- Is `PhantomUnfrozen` desirable? Users can write their own `PhantomUnfrozen` like so: +```rust +#[repr(transparent)] +struct PhantomUnfrozen(UnsafeCell<()>); +unsafe impl Sync for PhantomUnfrozen {} +``` +- Should `UnsafeCell` implement `Freeze`? It's a situation that might possibly occur in the wild, and could be supported. + +## Out of Scope +- Discussions of whether `UnsafeCell` should or could implement `Copy`. + +# Future possibilities +[future-possibilities]: #future-possibilities + +It's possible that the community might want a feature similar to D's "immutable references". Basically this would be `Freeze` but transitive across pointers; however, I am unsure what the use case would be. From e6a10659cdd0e5b371e493d1e2b5c322a4014a05 Mon Sep 17 00:00:00 2001 From: tyler Date: Sat, 13 Jun 2020 10:50:20 -0700 Subject: [PATCH 2/7] RFC freeze trait - add bikeshed question --- text/0000-freeze.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0000-freeze.md b/text/0000-freeze.md index a93f53da372..4b0a56324b9 100644 --- a/text/0000-freeze.md +++ b/text/0000-freeze.md @@ -138,6 +138,7 @@ The D programming language has a similar feature known as [immutable references] [unresolved-questions]: #unresolved-questions ## Design questions +- Should this trait have a different name besides `Freeze`? `Freeze` was a [public API](https://github.com/rust-lang/rust/pull/13076) long ago, and its meaning has somewhat changed. This may be confusing for oldtimers and/or newcomers who are googling the trait. Additionally, `freeze` is the name of an LLVM instruction used for turning uninitialized data into a fixed-but-arbitrary data value. - Is `PhantomUnfrozen` desirable? Users can write their own `PhantomUnfrozen` like so: ```rust #[repr(transparent)] From 8612bee5ea376cc192a641a2316ee64e184fc35c Mon Sep 17 00:00:00 2001 From: tyler Date: Sat, 13 Jun 2020 12:10:57 -0700 Subject: [PATCH 3/7] fix poor usage of paranthesis. edit ROM section to be less confusing. emphasize that `UnsafeCell` is the reason a type is not `Freeze` --- text/0000-freeze.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/0000-freeze.md b/text/0000-freeze.md index 4b0a56324b9..4597429d165 100644 --- a/text/0000-freeze.md +++ b/text/0000-freeze.md @@ -20,7 +20,7 @@ It is occasionally necessary in systems programming to know whether the range of ## Read Only Memory -If a type is suitable for read only memory, then it cannot have any interior mutability. For example, an `AtomicU8` is a poor candidate for being put into read only memory because the type system has no way to ensure that type is not mutated. It is, however, allowed to put a `Box` in read only memory as long as the heap allocation remains in writable memory. +If a type is suitable for read only memory, then it cannot have any interior mutability. For example, an `AtomicU8` cannot be placed in read-only memory, since it's possible to modify it via `.store` using only an immutable reference. On the other hand, a `Box` _can_ be placed in read only memory as long as the heap allocation remains in writable memory. The [main reason](https://github.com/rust-lang/rust/blob/84ec8238b14b4cf89e82eae11907b59629baff2c/src/libcore/marker.rs#L702) libcore has a private version of `Freeze` is to decide: > whether a `static` of that type is placed in read-only static memory or writable static memory @@ -33,7 +33,7 @@ Optimistic concurrency (e.g. seqlocks, software transactional memory) relies hea One example coming from [`swym`](https://docs.rs/swym/0.1.0-preview/swym/tcell/struct.TCell.html#method.borrow) is the method `borrow`. `borrow` returns snapshots of data - shallow memcpys - that are guaranteed to not be torn, and be valid for the duration of the containing transaction. These snapshots hold on to the lifetime of the `TCell` in order to act like a true reference, without blocking updates to the `TCell` from other threads. Other threads promise to not mutate the value that had its snapshot taken until the transaction has finished, but are permitted to move the value in memory. In the presence of interior mutability, these snapshots differ significantly from a true reference. -The following example uses a `Mutex` (a `Send`/`Sync`, but not `Freeze` type to create UB): +The following example uses a `Mutex`, a `Send`/`Sync` but not `Freeze` type, to create UB: ```rust let x = TCell::new(Mutex::new("hello there".to_owned())); @@ -56,9 +56,9 @@ Similarly to the above example, `crossbeam` would be able to expand `Atomic` to ## What types are `Freeze`? -The list of `Freeze` types is long, including primitives, `String`, `Vec`, `Option`, `Box`, `Arc`, `Rc`, etc. This is because you cannot modify the memory contained directly within these types through an immutable reference. +Types that contain an `UnsafeCell` in their memory layout, either directly or transitively, are not `Freeze` (modulo `unsafe impl Freeze for MyType`). This includes `Cell`, `RefCell`, `Mutex`, `AtomicUsize`, etc, as well as any types with a non-`Freeze` member. -Types that do not implement `Freeze` include types used in parallel programming such as, `Mutex`, `AtomicUsize`, etc, as well as `Cell`, `RefCell`, and `UnsafeCell`. This is because their memory can be modified via an immutable reference. +All other types are `Freeze`. This includes all primitives, `String`, `Vec`, `Option`, `Box`, `Arc`, `Rc`, etc. ## My type doesn't implement `Freeze`, but I need it to be `Freeze`. From 053cb6ce2a4e6fac0d1f0fa6fe48fe9b7a9f21a4 Mon Sep 17 00:00:00 2001 From: tyler Date: Sat, 13 Jun 2020 13:49:42 -0700 Subject: [PATCH 4/7] forbid explicit implementations of `Freeze` --- text/0000-freeze.md | 52 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/text/0000-freeze.md b/text/0000-freeze.md index 4597429d165..1d4ed534bc6 100644 --- a/text/0000-freeze.md +++ b/text/0000-freeze.md @@ -56,7 +56,7 @@ Similarly to the above example, `crossbeam` would be able to expand `Atomic` to ## What types are `Freeze`? -Types that contain an `UnsafeCell` in their memory layout, either directly or transitively, are not `Freeze` (modulo `unsafe impl Freeze for MyType`). This includes `Cell`, `RefCell`, `Mutex`, `AtomicUsize`, etc, as well as any types with a non-`Freeze` member. +Types that contain an `UnsafeCell` in their memory layout, either directly or transitively, are not `Freeze`. This includes `Cell`, `RefCell`, `Mutex`, `AtomicUsize`, etc, as well as any types with a non-`Freeze` member. All other types are `Freeze`. This includes all primitives, `String`, `Vec`, `Option`, `Box`, `Arc`, `Rc`, etc. @@ -64,14 +64,7 @@ All other types are `Freeze`. This includes all primitives, `String`, `Vec`, `Op To convert a type which is not `Freeze`, into a `Freeze` type, all that is required is to stick it on the heap. For example, `Box` is `Freeze` even if `T` is an `UnsafeCell`. -If you really know what you are doing, and promise not to mutate any data in your type through an immutable reference, then you can implement `Freeze` like so: - -```rust -struct MyType { /* .. */ } -unsafe impl Freeze for MyType {} -``` - -This requires `unsafe`, because UB is possible if in fact the memory occupied by `MyType` is mutable through an immutable reference to `MyType`. +Explicit implementations of `Freeze`, `unsafe impl Freeze for ..`, are forbidden by the compiler. ## How do I opt-out of `Freeze`? @@ -88,9 +81,9 @@ struct MyType { `Freeze` has been privately implemented in libcore for 3 years, and has not had major changes during that time. In that time it has been relied upon for deciding whether a `static` of a type is placed in read-only static memory or writable static memory. -`Freeze` needs to be made `pub` instead of `pub(crate)`. `PhantomUnfrozen` would be a new addition. +`Freeze` needs to be made `pub` instead of `pub(crate)`. `PhantomUnfrozen` would be a new addition. The compiler requires some changes to forbid explicit implementations of `Freeze`. -## Implementation +## libcore Implementation `libcore/marker.rs`: ```rust @@ -108,6 +101,40 @@ pub struct PhantomUnfrozen; impl !Freeze for PhantomUnfrozen {} ``` +## Compiler + +The compiler should forbid all explicit implementations of the trait `Freeze` outside of libcore. + +### Error Messages + +When a user attempts to explicitly implement `Freeze` like so: + +```rust +pub struct X { + a: Mutex<()>, +} + +unsafe impl Freeze for X {} +``` + +The suggested error message is: + +```rust +error[EXXXX]: `Freeze` cannot be explicitly implemented + --> src/lib.rs:8:6 + | +8 | unsafe impl Freeze for X {} + | ^^^^^^ the trait `Freeze` cannot be explicitly implemented for `X` + | + | hint: to make `X` satisfy `Freeze`, try adding a `Box` around `a`: + | +5 | a: Mutex<()>, + | ^^^^^^^^^ change this to `Box>` + | +``` + +The particular design of such an error message is left open to implementations. + # Drawbacks [drawbacks]: #drawbacks @@ -153,4 +180,5 @@ unsafe impl Sync for PhantomUnfrozen {} # Future possibilities [future-possibilities]: #future-possibilities -It's possible that the community might want a feature similar to D's "immutable references". Basically this would be `Freeze` but transitive across pointers; however, I am unsure what the use case would be. +- Lifting the restriction that `Freeze` cannot be explicitly implemented would be a natural evolution of this feature. The unsafe code guidelines would need to be updated with a thorough explanation of the circumstances that an explicit implementation is or is not UB. +- It's possible that the community might want a feature similar to D's "immutable references". Basically this would be `Freeze` but transitive across pointers; however, I am unsure what the use case would be. From 26adde6c21f5fc0922ecac59e9c89be97e6c34a5 Mon Sep 17 00:00:00 2001 From: tyler Date: Sat, 13 Jun 2020 13:50:09 -0700 Subject: [PATCH 5/7] fix spelling --- text/0000-freeze.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-freeze.md b/text/0000-freeze.md index 1d4ed534bc6..ff599b7d69f 100644 --- a/text/0000-freeze.md +++ b/text/0000-freeze.md @@ -145,7 +145,7 @@ The community desire for `Freeze` is also currently small. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -This design has been relied on by rustc for 3 years, and has not required any significant maintenence, nor does this author expect there to be much maintenence after making it `pub`. +This design has been relied on by rustc for 3 years, and has not required any significant maintenance, nor does this author expect there to be much maintenance after making it `pub`. Crate owners who incidentally have `Freeze` types in their API, and wish to add in interior mutability at a later date, can do so by simply `Box`ing up any parts of their type which may be modified through an immutable reference to avoid breaking changes. @@ -165,7 +165,7 @@ The D programming language has a similar feature known as [immutable references] [unresolved-questions]: #unresolved-questions ## Design questions -- Should this trait have a different name besides `Freeze`? `Freeze` was a [public API](https://github.com/rust-lang/rust/pull/13076) long ago, and its meaning has somewhat changed. This may be confusing for oldtimers and/or newcomers who are googling the trait. Additionally, `freeze` is the name of an LLVM instruction used for turning uninitialized data into a fixed-but-arbitrary data value. +- Should this trait have a different name besides `Freeze`? `Freeze` was a [public API](https://github.com/rust-lang/rust/pull/13076) long ago, and its meaning has somewhat changed. This may be confusing for old-timers and/or newcomers who are googling the trait. Additionally, `freeze` is the name of an LLVM instruction used for turning uninitialized data into a fixed-but-arbitrary data value. - Is `PhantomUnfrozen` desirable? Users can write their own `PhantomUnfrozen` like so: ```rust #[repr(transparent)] From c33243d41bc011b371ea2088a31a89ada35de444 Mon Sep 17 00:00:00 2001 From: tyler Date: Sat, 13 Jun 2020 14:40:45 -0700 Subject: [PATCH 6/7] Resolve the design question of whether `PhantomUnfrozen` is necessary. Users aren't able to get the same functionality as PhantomUnfrozen without sacrificing performance. --- text/0000-freeze.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/text/0000-freeze.md b/text/0000-freeze.md index ff599b7d69f..0de8d9ccaeb 100644 --- a/text/0000-freeze.md +++ b/text/0000-freeze.md @@ -147,9 +147,15 @@ The community desire for `Freeze` is also currently small. This design has been relied on by rustc for 3 years, and has not required any significant maintenance, nor does this author expect there to be much maintenance after making it `pub`. -Crate owners who incidentally have `Freeze` types in their API, and wish to add in interior mutability at a later date, can do so by simply `Box`ing up any parts of their type which may be modified through an immutable reference to avoid breaking changes. +The addition of `PhantomUnfrozen` is brand new, and while users could write a similar type to opt out of `Freeze`, it would [remove](https://rust.godbolt.org/z/zX_iuC) the `noalias readonly` llvm attributes which are used by the optimizer (note: all `PhantomData` types are `Freeze`). -No other designs have been considered. +```rust +#[repr(transparent)] +struct PhantomUnfrozen(UnsafeCell<()>); // slight pessimization +unsafe impl Sync for PhantomUnfrozen {} +``` + +Crate owners who incidentally have `Freeze` types in their API, and wish to add in interior mutability at a later date, can do so by simply `Box`ing up any parts of their type which may be modified through an immutable reference to avoid breaking changes. The impact of not doing this would be admittedly small. Users who want this feature would have to wait for `optin-builtin-traits`, use nightly rust, `Box` up data they intend to `Freeze`, or rely on `unsafe` code. This RFC author would elect to keep [`swym`](https://github.com/mtak-/swym) on nightly rust rather than pay the performance overhead of heap allocation. @@ -166,12 +172,6 @@ The D programming language has a similar feature known as [immutable references] ## Design questions - Should this trait have a different name besides `Freeze`? `Freeze` was a [public API](https://github.com/rust-lang/rust/pull/13076) long ago, and its meaning has somewhat changed. This may be confusing for old-timers and/or newcomers who are googling the trait. Additionally, `freeze` is the name of an LLVM instruction used for turning uninitialized data into a fixed-but-arbitrary data value. -- Is `PhantomUnfrozen` desirable? Users can write their own `PhantomUnfrozen` like so: -```rust -#[repr(transparent)] -struct PhantomUnfrozen(UnsafeCell<()>); -unsafe impl Sync for PhantomUnfrozen {} -``` - Should `UnsafeCell` implement `Freeze`? It's a situation that might possibly occur in the wild, and could be supported. ## Out of Scope From 54e2800b74a72c1f38b3cb38cc67a6a1490df05d Mon Sep 17 00:00:00 2001 From: tyler Date: Wed, 17 Jun 2020 11:43:00 -0700 Subject: [PATCH 7/7] RFC Freeze: remove PhantomUnfrozen, and give user workarounds. Clarify language implying `Freeze` has something to do with the heap - it doesn't. Discuss a bit more in rationale about how adding interior mutability is often already a breaking change. --- text/0000-freeze.md | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/text/0000-freeze.md b/text/0000-freeze.md index 0de8d9ccaeb..fe55d2d9b6d 100644 --- a/text/0000-freeze.md +++ b/text/0000-freeze.md @@ -6,11 +6,10 @@ # Summary [summary]: #summary -This RFC introduces new APIs to libcore/libstd to serve as safe abstractions for data which has no "shallow" interior mutability. +This RFC introduces a new API to libcore/libstd to serve as a safe abstraction for data which has no "shallow" interior mutability. ```rust pub unsafe auto trait Freeze {} -pub struct PhantomUnfrozen; ``` # Motivation @@ -52,7 +51,7 @@ Similarly to the above example, `crossbeam` would be able to expand `Atomic` to # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -`Freeze` is a new marker trait, similar to `Send` and `Sync`, that is intended to only be implemented for types which have no direct interior mutability, and are therefore safe to place in read only memory. +`Freeze` is a new marker trait, similar to `Send` and `Sync`, that is only implemented for types which have no direct interior mutability, and are therefore safe to place in read only memory. ## What types are `Freeze`? @@ -62,26 +61,32 @@ All other types are `Freeze`. This includes all primitives, `String`, `Vec`, `Op ## My type doesn't implement `Freeze`, but I need it to be `Freeze`. -To convert a type which is not `Freeze`, into a `Freeze` type, all that is required is to stick it on the heap. For example, `Box` is `Freeze` even if `T` is an `UnsafeCell`. +To convert a type which is not `Freeze`, into a `Freeze` type, add a pointer based indirection around the type. For example, `&T`, `&mut T` and `Box` are `Freeze` even if `T` is an `UnsafeCell`. Explicit implementations of `Freeze`, `unsafe impl Freeze for ..`, are forbidden by the compiler. ## How do I opt-out of `Freeze`? -This is only useful when you suspect your type might, at some point in the future, include a non-`Freeze` type. To protect your users from relying on the current implementation of your type, simply add `PhantomUnfrozen` as a member to your type. +This is only useful when you suspect your type might, at some point in the future, include a non-`Freeze` type. To protect your users from relying on the current implementation of your type, add an `UnsafeCell<()>` as a member to your type. ```rust struct MyType { - _dont_rely_on_freeze: PhantomUnfrozen, + _dont_rely_on_freeze: UnsafeCell<()>, } + +// only if appropriate +unsafe impl Sync for MyType {} +unsafe impl RefUnwindSafe for MyType {} ``` +Adding `UnsafeCell<()>` as a field to your type also forces your type to not be `Sync` and `RefUnwindSafe`. This may or may not be desirable. + # Reference-level explanation [reference-level-explanation]: #reference-level-explanation `Freeze` has been privately implemented in libcore for 3 years, and has not had major changes during that time. In that time it has been relied upon for deciding whether a `static` of a type is placed in read-only static memory or writable static memory. -`Freeze` needs to be made `pub` instead of `pub(crate)`. `PhantomUnfrozen` would be a new addition. The compiler requires some changes to forbid explicit implementations of `Freeze`. +`Freeze` needs to be made `pub` instead of `pub(crate)`. The compiler requires some changes to forbid explicit implementations of `Freeze`. ## libcore Implementation @@ -96,9 +101,6 @@ unsafe impl Freeze for *const T {} unsafe impl Freeze for *mut T {} unsafe impl Freeze for &T {} unsafe impl Freeze for &mut T {} - -pub struct PhantomUnfrozen; -impl !Freeze for PhantomUnfrozen {} ``` ## Compiler @@ -121,7 +123,7 @@ The suggested error message is: ```rust error[EXXXX]: `Freeze` cannot be explicitly implemented - --> src/lib.rs:8:6 + --> src/lib.rs:8:13 | 8 | unsafe impl Freeze for X {} | ^^^^^^ the trait `Freeze` cannot be explicitly implemented for `X` @@ -147,15 +149,9 @@ The community desire for `Freeze` is also currently small. This design has been relied on by rustc for 3 years, and has not required any significant maintenance, nor does this author expect there to be much maintenance after making it `pub`. -The addition of `PhantomUnfrozen` is brand new, and while users could write a similar type to opt out of `Freeze`, it would [remove](https://rust.godbolt.org/z/zX_iuC) the `noalias readonly` llvm attributes which are used by the optimizer (note: all `PhantomData` types are `Freeze`). - -```rust -#[repr(transparent)] -struct PhantomUnfrozen(UnsafeCell<()>); // slight pessimization -unsafe impl Sync for PhantomUnfrozen {} -``` +It is risky to give users the ability to explicitly `unsafe impl Freeze`, so the compiler forbids it. It is not expected for users to be writing their own `Freeze` abstractions (like `Sync`). Either a type has direct interior mutability or it doesn't. -Crate owners who incidentally have `Freeze` types in their API, and wish to add in interior mutability at a later date, can do so by simply `Box`ing up any parts of their type which may be modified through an immutable reference to avoid breaking changes. +Crate owners who incidentally have `Freeze` types in their API, and wish to add in interior mutability at a later date, can do so by simply adding any pointer based indirection (e.g. `Box`) to any parts of their type which may be modified through an immutable reference to avoid breaking changes. Moreover, adding interior mutability is often already a breaking change. `UnsafeCell` is not `Sync` nor `RefUnwindSafe`, and is invariant over `T`. The impact of not doing this would be admittedly small. Users who want this feature would have to wait for `optin-builtin-traits`, use nightly rust, `Box` up data they intend to `Freeze`, or rely on `unsafe` code. This RFC author would elect to keep [`swym`](https://github.com/mtak-/swym) on nightly rust rather than pay the performance overhead of heap allocation. @@ -172,7 +168,6 @@ The D programming language has a similar feature known as [immutable references] ## Design questions - Should this trait have a different name besides `Freeze`? `Freeze` was a [public API](https://github.com/rust-lang/rust/pull/13076) long ago, and its meaning has somewhat changed. This may be confusing for old-timers and/or newcomers who are googling the trait. Additionally, `freeze` is the name of an LLVM instruction used for turning uninitialized data into a fixed-but-arbitrary data value. -- Should `UnsafeCell` implement `Freeze`? It's a situation that might possibly occur in the wild, and could be supported. ## Out of Scope - Discussions of whether `UnsafeCell` should or could implement `Copy`.