--- 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.) ---