-
Notifications
You must be signed in to change notification settings - Fork 1.6k
RFC: add the Freeze trait to libcore/libstd #2944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b9fa347
rfc to add the Freeze trait to libcore/libstd
mtak- e6a1065
RFC freeze trait - add bikeshed question
mtak- 8612bee
fix poor usage of paranthesis. edit ROM section to be less confusing.…
mtak- 053cb6c
forbid explicit implementations of `Freeze`
mtak- 26adde6
fix spelling
mtak- c33243d
Resolve the design question of whether `PhantomUnfrozen` is necessary…
mtak- 54e2800
RFC Freeze: remove PhantomUnfrozen, and give user workarounds.
mtak- File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,179 @@ | ||
- 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 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 {} | ||
``` | ||
|
||
# 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` 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<AtomicU8>` _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 | ||
|
||
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<T>`), 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't work, because a Box<Mutex<T>> will be Freeze, but it will still cause a double free. |
||
|
||
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 only implemented for types which have no direct interior mutability, and are therefore safe to place in read only memory. | ||
|
||
## What types are `Freeze`? | ||
|
||
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<String>`, `Box`, `Arc`, `Rc`, etc. | ||
|
||
## 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, add a pointer based indirection around the type. For example, `&T`, `&mut T` and `Box<T>` are `Freeze` even if `T` is an `UnsafeCell`. | ||
|
||
Explicit implementations of `Freeze`, `unsafe impl Freeze for ..`, are forbidden by the compiler. | ||
mtak- marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## 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, add an `UnsafeCell<()>` as a member to your type. | ||
|
||
```rust | ||
struct MyType { | ||
_dont_rely_on_freeze: UnsafeCell<()>, | ||
} | ||
|
||
// only if appropriate | ||
unsafe impl Sync for MyType {} | ||
unsafe impl RefUnwindSafe for MyType {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
``` | ||
|
||
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)`. The compiler requires some changes to forbid explicit implementations of `Freeze`. | ||
|
||
## libcore Implementation | ||
|
||
`libcore/marker.rs`: | ||
```rust | ||
#[lang = "freeze"] | ||
pub unsafe auto trait Freeze {} | ||
|
||
impl<T: ?Sized> !Freeze for UnsafeCell<T> {} | ||
unsafe impl<T: ?Sized> Freeze for PhantomData<T> {} | ||
unsafe impl<T: ?Sized> Freeze for *const T {} | ||
unsafe impl<T: ?Sized> Freeze for *mut T {} | ||
unsafe impl<T: ?Sized> Freeze for &T {} | ||
unsafe impl<T: ?Sized> Freeze for &mut T {} | ||
``` | ||
|
||
## 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:13 | ||
| | ||
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<Mutex<()>>` | ||
| | ||
``` | ||
|
||
The particular design of such an error message is left open to implementations. | ||
|
||
# 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 maintenance, nor does this author expect there to be much maintenance after making it `pub`. | ||
|
||
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 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<T>` 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. | ||
|
||
# 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 | ||
- 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. | ||
|
||
## Out of Scope | ||
- Discussions of whether `UnsafeCell` should or could implement `Copy`. | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
- 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. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.