RFC: add the Freeze trait to libcore/libstd

[mtak-]

Rendered

There was some pre RFC discussion here.

[Lokathor]

Isn't LLVM's name for turning uninit data into a fixed-but-arbitrary data value also called "freeze"?

If so, perhaps we should use some other name.

This is a bikeshed, but seems like a fair one.

[mtak-]

@Lokathor Totally fair. Freeze also use to be a public API long ago, so googling "rust freeze trait" may yield some inaccurate information. I'll add that to the unresolved questions.

[RalfJung]

Yeah those are two totally different uses of the same word. :/

[mtak-]

To get bikeshedding rolling, how about Final, Immut, NoAlias, or ReadOnly? The last two match the llvm attributes that &T would have.

Additionally, I think libcore must provide a PhantomUnfrozen as the user workaround is a pessimization that would remove the noalias readonly attributes.

[Aaron1011]

Additionally, I think libcore must provide a PhantomUnfrozen as the user workaround is a pessimization that would remove the noalias readonly attributes.

That would seem to imply that there are two kinds of !Freeze types:

• 'really !Freeze (contains an UnsafeCell) which affects both Rust mutable/immutable memory placement and LLVM attributes
• 'psuedo-!Freeze (contains PhantomUnfrozen but not an UnsafeCell) which does not affect LLVM attributes but causes rustc to do 'something'.

What are the use cases for PhantomUnfrozen?

[mtak-]

The only use case for PhantomUnfrozen is to allow crate authors to reserve some extra flexibility in their implementations without breaking downstream crates. In other words, to prevent folks from depending on one or more of a crate author's types being Freeze.

[Lokathor]

So, what if people just opted into Freeze instead using a normal Derive, like how it works with Copy

[mtak-]

I think most of the arguments for having Unpin, Sync, and Send not work the same as Copy would also apply to Freeze.

For example:

#[derive(Freeze)]
pub struct Bar<T>(*const T);

Would not be Freeze if T is not Freeze. That is incorrect.

[Lokathor]

That's just because the derivation system is overly simplistic and needs to be improved.

The fundamental idea that trait impls are a public part of the API of a type, thus also being a stability hazard that designers should have to opt in to, stands.

[Ixrec]

To get bikeshedding rolling, how about Final, Immut, NoAlias, or ReadOnly? The last two match the llvm attributes that &T would have.

I also think the "shallow"/"direct" aspect in the RFC should be part of the name, or else a lot of people will misinterpret this trait name as being about deep immutability the first time they see it. And unless I'm missing a huge part of the motivation, nobody should be writing this trait name very often, so I'm inclined toward clear/descriptive names rather than short names. My current favorite would be ShallowImmutable and (if we need it) PhantomShallowMutable.

I dislike Final because of its established use in other languages for inheritance stuff, and NoAlias because (unless I've horribly misunderstood something) this has nothing at all to do with whether aliases can exist, only with whether mutation can be done through those aliases. I have no complaints with ReadOnly.

[mtak-]

Freeze is unsafe. Freeze has no methods. Freeze is about the structure of a type. In the above example, Bar is always correct to be Freeze. Whether or not it should be PartialEq, Hash, Copy, etc. cannot be known without more information. IMO, it shares much more in common with Send/Sync than Copy. One thing it does share with Copy is that lambdas would need to automatically impl Freeze.

The fundamental idea that trait impls are a public part of the API of a type, thus also being a stability hazard that designers should have to opt in to, stands.

I do not feel that this is a substantial enough problem to block adoption, and I don't think #[derive(..)] is a good option. It is not a feature that will see widespread usage. I would be interested in more opinions though.

That would seem to imply that there are two kinds of !Freeze types

We could reduce the number of kinds of !Freeze types to 1 by placing a PhantomUnfrozen in
UnsafeCell. That might somewhat harm teachability tho.

My current favorite would be ShallowImmutable

I like that too.

[RalfJung]

I think libcore must provide a PhantomUnfrozen as the user workaround is a

PhantomData<UnsafeCell<()>> should do it.

EDIT:

remove the noalias readonly attributes

Eh, that is the entire point, yes. PhantomUnfrozen would do the same. Those attributes are controlled by the Freeze trait; indeed those attributes are the only reason the Freeze trait exists. Are you saying now that we should sometimes add those attributes even for !Freeze types? IMO that makes no sense -- it rather sounds like you actually want a different trait. Freeze is supposed to exactly reflect whether the type can be mutated while shared, and as such should coincide exactly with whether we add these attributes.

EDIT2: Ah you are worried about future compatibility. Hm. I have to say a non-attribute-affecting Freeze still sounds like a rather big hack though. We'd basically need a duplicate of that trait internally in the compiler to determine whether to add attributes.

[mtak-]

PhantomData<UnsafeCell<()>> should do it.

This wouldn't work due to this impl (necessary since Box/Vec/etc have a PhantomData<T> in their memory layout):

unsafe impl<T: ?Sized> Freeze for PhantomData<T> {}

[RalfJung]

unsafe impl<T: ?Sized> Freeze for PhantomData<T> {}

Interesting, I did not know this existed.

[Ixrec]

Is it possible to use specialization to make PhantomData<UnsafeCell<()>> impl !Freeze? (I'm way behind on what subsets of specializations are sound/unstable/maybe-stabilizable/exposable-in-std)

[clarfonthey]

Genuine question: is there a reason why Freeze can't exist while also adopting a convention of its implementation not being semver-compatible?

Like, it technically already exists as a feature of the language. And we can technically already observe it in code through the restriction on const borrowing. For a relatively obscure feature, I don't see why its existence would require everyone to add PhantomCell like fields to their types. Just say that it isn't considered a semver-compatible restriction and add some linting around it to discourage its use unless you really, really need it.

[Avi-D-coder]

I agree 💯.

We shouldn't let semver constrain language features. Compatibility is important, but nothing prevents me as a author from making a breaking change in a minor version. Ultimately semver is a useful convention followed by authors, not a guarantee upheld by the compiler.

[clarfonthey]

Semver is a bit more than a convention considering how cargo is designed to reduce the number of versions used based upon it, and there's a very specific definition of breaking changes that could theoretically be applied algorithmically to Rust code. But, I agree in this case that we could make an exception.

[RalfJung]

There is some desire to use Freeze for some copy-on-write data structures. However, as I noted on Zulip, I think Freeze alone is not sufficient for this.

I somehow feel like rust-lang/rust#49206 is related, because this is about observing the address at which some shared data lies, but I cannot put my finger onto it or make this precise.

[dtolnay]

Approving on behalf of withoutboats, as per rust-lang/team#526.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC is now closed.

[kupiakos]

We should reconsider stabilizing some sort of built-in Freeze. As of today, it's impossible to declare a generic value parameter in a const fn and to take its address, since it may have interior mutability. We shouldn't hamstring generics in const for a possibility we could opt-out of if given the chance. Adding interior mutability is already a breaking change because of this, so I really don't buy the semver argument.

[RalfJung]

The const fn situation is a temporary one, so it is IMO not a good argument for permanently stabilizing Freeze.

Think of it this way: we could have just disallowed &T in const fn entirely. But to allow at least some more code, we decided to allow &impl Freeze and only disallow interior mutable and generic references. That's way better than disallowing all references. I can understand it is frustrating though.

[Voultapher]

I recently discovered? another use-case for Freeze that has nothing to do with shared data https://internals.rust-lang.org/t/interior-mutability-reflection/18198

I'm currently in the process of researching sort implementations. And have ideas I want to and am already upstreaming into the standard library. One important concept is something I call observable_is_less. The user can provide a custom comparison function either by implementing Ord or by calling sort_by. For types that allow interior mutability as marked by containing an UnsafeCell, this user provided comparison function could mutate the stack value. No matter what path the execution takes, or even if Ord is implemented incorrectly. One invariant that needs to be upheld by the sort implementations is that every possible write to the stack value of the type, has to be 'observed' in the user provided slice when the sort function returns, either normally or via panic. This gets tricky when working with temporary memory, eg. for merging. Certain optimisations would be invalid for such types, for example they may compare two values, but might skip writing one of them into temporary memory because it has already been written before. And then later write that temporary memory over parts of the input slice. In that case one or more 'writes' as per calls to is_less would have been missed. This leads to unexpected behaviour and even possibly UB.

Right now, I'm using Copy via specialisation to determine what types these optimisations are safe to do for, but it would be great and benchmarks suggest beneficial even for types such as String to allow such optimisations. However I'm not aware of any way to write such a function fn has_interior_mutability<T>() -> bool. The compiler already has this information, it would be great to somehow get access to that information. IIRC there is a way to 'check' if a type has interior mutability by seeing if a function compiles that would be invariant over a shared reference to such type. But such check can only cause a compiler error, not provide runtime/compile-time branching.

[Voultapher]

Crate owners have to now commit to an interior mutability story, or risk breaking changes in the future."

They already do, there is more cases than the const fn one, that break when such a type is added eg:

pub struct SomeType<'a> {
    val: &'a i32,
    // x: std::cell::Cell<&'a i32>, // Enable this for error.
}


pub fn assert_covariance<'x: 'y, 'y>(x: SomeType<'x>) -> SomeType<'y> {
    //  This function only compiles for covariant types.
    x
}

If the authors choose to add the uncommented line, the code stops compiling. I see a clear benefit here, and a dubious reason not to add something like this.

[bjorn3]

That is because the variance between &'a i32 and Cell<&'a i32> is different. That has nothing to do with interior mutability.

[Voultapher]

Well the variance here, changes as direct result of transitively containing UnsafeCell or not. Arguably that's the same effect for users.

[Voultapher]

If the main concern is code breaking. I suggest this alternative design:

(const?) fn has_direct_interior_mutability<T>() -> bool;

If we add such a function to the standard library it would serve the purpose of allowing such optimisations, without building things that will stop compiling if users add a new member. With constant propagation and folding the optimiser ought to be able to generate efficient code depending on the types property. To motivate it, here is what a simple type wrapping a u64 not marked copy would loose out on potential performance, rust_new_stable_no is with the relevant optimisations disabled for u64:

Of course a simple u64 wrapper should be marked Copy, but types such as String can't be and they also benefit from speedups, less pronounced, but still 10+%.

[RalfJung]

Well the variance here, changes as direct result of transitively containing UnsafeCell or not. Arguably that's the same effect for users.

Invariance can arise without interior mutability. So it's really unrelated to this discussion.

Right now, I'm using Copy via specialisation

It makes me very sad that Copy is used as a marker for absence of interior mutability. These two concepts are entirely orthogonal and there is no fundamental reason that Rust can't have Copy !Freeze types. The fact that people make assumptions like this will make future evolution of some parts of Rust a lot harder. :( (see rust-lang/rust#25053)

[Voultapher]

Invariance can arise without interior mutability. So it's really unrelated to this discussion.

It seems, I poorly explained my point. My understanding is, that the main reason this proposal was declined, was a worry that it would make it easier for library Authors to inadvertently break SemVer. I gave the example to demonstrate, that something very similar, already happens today on stable.

It makes me very sad that Copy is used as a marker for absence of interior mutability.

It's a tricky situation, here are the options I'm seeing:

• A) Ignore the possible optimisations.
• B) Adjust the code so it is safe for types with direct interior mutability.
• C) Add an unsafe marker trait.

A, is IMO a bad fit for Rust, a language that that markets itself as an efficiently way to exploit hardware resources. I tried B, and have not had good success, even the failed attempts I had, made the code significantly more complex. That's not a property that should be ignored, especially because unsafe code is being used. C, is really problematic too. It creates this two class 'society', where builtin u64 ... sort(), and struct X{u32: a, u32: b} ... sort_by_key(|x| x.a) would have vastly different performance.

Is using Copy a hack, absolutely! Are the alternatives better, I would argue, that no, the alternatives are worse. However there exists a solution, the compiler already knows the required information, it only has to surface it to the user. Then all these problems go away, including:

These two concepts are entirely orthogonal and there is no fundamental reason that Rust can't have Copy !Freeze types.

And if I understood correctly, Orson Peters the author of pdqsort who is currently working on glidesort came across the same problem and came up with the same 'solution' of limiting certain optimisations to Copy types. So I argue, if we don't provide a good solution, people will use the bad options, creating further problems down the line.

[ahorn]

Invariance can arise without interior mutability. So it's really unrelated to this discussion.

Workarounds, such as assert_covariance, seem to rely on the assumption that all covariant type constructors prevent interior mutability. Is it therefore fair to rephrase @RalfJung's criticism that covariance is merely a sufficient (but not a necessary condition) for the absence of interior mutability? Or is the criticism that covariance cannot rule out interior mutability, and checks such as assert_covariance are therefore doomed to begin with?

[RalfJung]

I meant to say that we are not making a guarantee that covariance has any relation with UnsafeCell either way.

In fact I don't even know what you mean by this question. Covariance is a property of type constructors, not types. The following type has interior mutability but is covariant in its only agument:

struct MyType<T>(T, Cell<i32>);

[ahorn]

Got it, thank you very much for taking the time to clarify and your example.

[joshlf]

It would be great to have this for zerocopy; we need to reason about this for things like google/zerocopy#8 and google/zerocopy#251.

[RalfJung]

This is a closed PR, please stop posting here, it's really not useful. rust-lang/rust#60715 is a more appropriate place to comment.