owned this note
owned this note
Published
Linked with GitHub
---
title: "Design meeting 2024-07-24: `Freeze` in bounds"
tags: ["T-lang", "design-meeting", "minutes"]
date: 2024-07-24
discussion: https://rust-lang.zulipchat.com/#narrow/stream/410673-t-lang.2Fmeetings/topic/Design.20meeting.202024-07-24
url: https://hackmd.io/bHpacrVlRk2fcnp_0gVM_w
---
- 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.
- 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
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:
```rust
pub trait VTable<'a>: Copy {
const VT: &'a Self;
}
pub struct VtAccumulator<Tail, Head> {
tail: Tail,
head: Head,
}
impl<Tail: VTable<'a>, Head: VTable<'a>> VTable<'a> for VtAccumulator<Tail, Head> {
const VT: &'a Self = &Self {tail: *Tail::VT, head: *Head::VT}; // Doesn't compile since 1.78
}
```
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 `[u8; core::mem::size_of::<T>()]`) 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.
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
`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 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".
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
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.
```
From a cursory review, the following documentation improvements may be considered:
```markdown
[`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,
provided that it is behind an indirection (such as `Box<UnsafeCell<U>>`).
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.
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`.
# 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<T>` is currently `!Freeze` regardless of `T`, allowing `UnsafeCell<T>: 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.
```
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<T>` where `core::mem::size_of::<T>() == 0`
- `[T; 0]` where `T: !Freeze`.
Note that `core::marker::PhantomData<T>` is `Freeze` regardless of `T`'s `Freeze`ness.
```
# 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 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<i32>);
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
- 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.
- 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 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).
- 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
- [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".
# 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.
- The current status-quo is that it cannot be implemented manually (experimentally verified with 2024-05-12's nightly).
- 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.
- Providing such a trait could be left to the ecosystem unless we'd want it to be an auto-trait also (unlikely).
---
# Discussion
## Attendance
- People: TC, tmandry, pnkfelix, Josh, eholk, Pierre Avital, Urgau, RalfJ
## Meeting roles
- Minutes, driver: TC
## Double negatives
Josh: `core::marker::PhantomNotShallowImmutable` -> `core::marker::PhantomShallowMutable`?
pnkfelix: Does the shallowness matter to the people who want this marker type? It seems like `core::marker::PhantomMutable` (or `core::marker::PhantomInteriorMutable`) would also be fine.
Josh: :+1:, seems fine and simpler.
Pierre: :+1:, with a preference for the `InteriorMutable` variant.
## "Shallow"?
TC: I'm not sure about the word "shallow" here. I mean, I get it, that's it's shallow in a memory layout sense and in the sense of lack of indirection. But thinking of the ADTs in a more abstract sense, we may still be "projecting" through fields, and that doesn't feel shallow, in a sense. So there's some ambiguity.
tmandry: Agreed, same thought here, I think there are a couple of possible interpretations of "Shallow":
* The struct definition does not contain `UnsafeCell`.
* The struct does not transitively contain `UnsafeCell`, except through pointer indirections.
Ralf: We used "shallow" in discussions around structural match as well. There it meant "do not recurse into any fields". I dont think the term ended up in anything stable, but clearly "shallow" can have that meaning.
## Are we onboard with a rename?
pnkfelix: We should ensure whether or not we're in favor of renaming before going down this path.
tmandry: No general objection to the idea of renaming, but I had the same concern as TC with the word "shallow" and how that might be ambiguous.
tmandry: We could of course not use `freeze` for the LLVM thing. We could rename that.
TC: +1. I'm open to renaming, but I also want to see a good, hopefully concise, alternative.
pnkfelix: I've always thought that "freeze" was a bad name.
Josh: Agreed that "freeze" is a non-obvious name. "shallow immutable" is self-describing and would be much more obvious.
Pierre: I'm personally _okay_ with either, I've received feedback from colleagues that they'd like the trait to be renamed. They insisted on hinting at the "un-indirected" nature.
Pierre: I also like "inline" from a suggestion that pnkfelix made in chat here.
pnkfelix: Just to capture for posterity, here are all the suggestions that were made in chat, as alternatives to "Shallow" in "ShallowImmutable": {Interior, Direct, Inline, ByVal, Value, Local}
## Stability guidelines
Josh: (Assuming this is correct) Can we add something to the stability guidelines stating that changing something from non-`ShallowImmutable` to `ShallowImmutable` is a compatible change, while the reverse is a breaking change?
We should also notify the maintainers of cargo-semver-checks and similar tools.
Discussion raised: if you rely on someone else's type being `!ShallowImmutable` and they become `ShallowImmutable` you could be surprised, but we should probably note that you shouldn't rely on other types and should declare it yourself.
## Rollout and stability hazards
tmandry: Should we roll out the `Phantom` type first, so ecosystem types have the chance to opt out before other libraries depend on them?
Pierre: Other libraries technically can already depend on the trait, through static promotion.
Ralf: This...
```rust
struct S(UnsafeCell<()>);
```
...is a backward compatible way to opt-out.
## Opt-in derive
tmandry: Would your use case be satisfied with just an opt-in (non-transitive)? Generic code would use a trait bound like `OptInFreeze` which has to be derived on the types.
Pierre: Yes, everything uses a proc macro.
Ralf: But we would still have to rely on `Freeze` for static promotion. This would retain the semver hazards.
## PhantomData
Josh:
> Note that `core::marker::PhantomData<T>` is `Freeze` regardless of `T`'s `Freeze`ness.
I'm not clear on the reason for this. Is there a reason we wouldn't make `PhantomData<T>` be `!Freeze` if `T` is `!Freeze`? That seems like it would produce desirable results. Would that have unwanted side effects?
Pierre:
This mostly was a reminder of the status quo (at least at time of writing the RFC). I somewhat agree with you, though I feel the most important part of this would be to "pick a side".
pnkfelix: My intuition is that *either* side of the choice will have its adherents. And as far as I can tell, no matter which way we decide, there is no way to *express* the other direction (i.e. if `PhantomData<T>` hides the freeze-ness of `T`, there's no way to propagate it, and if it exposes the freeze-ness of `T`, there's no way to hide it.) That seems bad, but also maybe something we could address in the future?
Ralf: An attempt to make PhantomData propagate !Freeze was made [here](https://github.com/rust-lang/rust/pull/114559), but I gave up after seeing 240 regressions on crater.
Pierre: There could be value in having `PhantomData` carry the `Freeze`. We could have something like a `PhantomFreezeIf<T>`.
Ralf: If we had propagated `Freeze` in Rust 1.0, probably nobody would have complained. But someone made the argument for it, and it makes sense from a certain angle, where it was a marker for optimizations (e.g. noalias) due to zero-sizedness.
pnkfelix: It is almost like we need a separate internal marker trait here.
tmandry: It'd be nice to sketch out what the path would be to clawing this back, in terms of what could or could not happen over an edition.
## Future possibility: `impl !ShallowImmutable for Type`
Josh: We've talked about providing other ways to declare something `!Send` or `!Sync`, such as `impl !Send for Type`, which AIUI we may stabilize in the future. If we do so, is there any reason we *shouldn't* support `impl !ShallowImmutable for Type` at the same time?
Piere: I feel like such a feature would generally be amazing to have for any given trait as a way to promise "X will ~~_never_~~ (not without a major bump) be implemented for Y", helping alleviate some of the orphan rule's pains.
Josh: Hmmm. We'd need to distinguish "will never" from "does not currently". I don't know which syntax we plan to use to ergonomically express the latter. Whatever we do for `Send` and `Sync` we should also do for `ShallowImmutable`.
## Next steps - `PhantomData` communicating `Freeze`
TC: What next steps should we consider here?
Pierre: Maybe the `PhantomFreezeIf` could help here and be a step. We should think about the transition/migration point that was brought up.
tmandry: I really want to know whether there's a way we can do this that's not exposing compiler details, etc., and in a way that allows `PhantomData` to work consistently.
Ralf: The `PhantomData` type also always implements `StructuralEq`, but that one seems justified.
Pierre: I see the argument that `PhantomData` should not propagate `Freeze`. It'd be easy to accidentally propagate it through indirection.
TC: Is this question a blocker here, or could we handle this later and do `PhantomNotFreeze` for now.
tmandry: Maybe we could handle this later. Not sure. This is my biggest concern right now.
Pierre: There is not and should not be a way to manually implement `Freeze`. So the trouble here with having `PhantomData` propagate it is that there's no way for the developer to say, "no, really, this type *doesn't* have interior mutability".
Ralf: The existing data we have is that the SemVer problem here is probably not serious.
Ralf: I looked into the crater regression for the main crate generating the regression here. Stabilizing const ref to `Cell` might take care of a large part of the regression.
Ralf: Once we have const mut refs, I'm not sure why we wouldn't have const refs to `Cell`.
tmandry: So you're saying we might have a path to having `PhantomData` communicate freezeness if we want.
tmandry: Turning over Pierre's point in my head, I agree that there should be a way to get it back.
TC: +1. If there's a way to get it back, then I feel good about making `PhantomData` communicate this if we can, in terms of breakage. Otherwise, I start wondering if we can punt this question and go with `PhantomNotFreeze` for now.
Ralf: The natural way would be to allow an `unsafe impl Freeze for ..`. There are a number of interesting opsem questions here.
## Next steps
TC: Let's write out the next steps of what we need to decide to move this RFC forward and what the known options are.
- What should we call this?
- Option 1: `Freeze`
- Option 2: `ShallowImmutable`
- There is some ambiguity to the word "shallow".
- Option 3: `LocalImmutable`
- Ralf: Arguably this is just as ambiguous. "Local" to what?
- Josh: :+1:, "local" seems confusing to me, and not self-explanatory.
- Option 4: `InlineImmutable`
- Option 5: `ValueImmutable`
- Option 6: `DirectImmutable`
- pnkfelix: (as the clear antonym of "indirect", since that word seems to come up a lot when describing the mutability that is ignored here.)
- Option 7: `InteriorImmutable` (injected by nikomatsakis)
- Argument: embedding a "cell" is called "interior mutability", this is the opposite
- counterpoint: `Box<Cell<u32>>` is still *logically* interior mutability, but is `Freeze`
- (pnkfelix is willing to contest this; in that the type *has* interior mutability, but its behind a level of indirection. Which is exactly what we're discussing...)
- nikomatsakis: (Sorry, I wrote "but not Freeze", but that was a typo, corrected.) My point is that `Box<Cell<u32>>` *is* "InteriorImmutable" (if we use that name) and *yet* also has interior mutability. In other words, pnkfelix, I think I agree with you it's not a perfect name.
- What do we do about `PhantomData` communicating `Freeze`?
- Option 1: Ship `PhantomNotFreeze` / `PhantomMutable` / `PhantomInteriorMutable`.
- Option 2: Ship `PhantomFreezeIf<T>`.
- This would work like `PhantomData` but propagate `Freeze`.
- Option 3: Make `PhantomData` communicate `Freeze` and add `unsafe impl Freeze for ..`.
- This makes `PhantomData` work consistently and removes the wart, and it allows "getting `Freeze` back" when needed.
- This sounds like it'd have substantial breakage, based on crater runs of past attempts.
- But maybe not as much as in the past if we stabilize const references to `Cell`.
- Seems possible to do it if we want it enough.
- `unsafe impl Freeze` is tricky; we might want to have a compiler check for unsoundness.
- Are we concerned about expanding semver hazards?
- Option 1: We can stabilize both `Freeze` and `PhantomNotFreeze` at the same time.
- Option 2: We need to stabilize `PhantomNotFreeze` ahead of `Freeze`.
- Option 3: This entire endeavor is doomed because of existing expectations in the ecosystem.
- Does `PhantomNotFreeze` have any opsem consequences? As in, is the exception of "mutation allowed behind shared references" tied to `UnsafeCell` or tied to the newly public trait? (If it is tied to `UnsafeCell`, we need to have the guarantee that any type containing `UnsafeCell` can never implement the trait -- currently that is ensured by not stabilizing `unsafe impl Freeze`.)
- Ralf: my current feeling is that it should be tied to `UnsafeCell`.
TC: Proposal: we leave four open questions corresponding to the bullet points above. We have the RFC body presumptively go with `PhantomNotFreeze` (but note the open question). We otherwise accept the RFC.
tmandry: Probably the RFC can narrow the SemVer question a bit further, and that would be good.
TC: +1.
pnkfelix: Sounds fine.
Josh: No objections.
Pierre: Sounds good.
nikomatsakis (remote): +1
(The meeting ended here.)
---