Re: [PATCH v3 2/2] rust: types: `Opaque` doc: Add some destructor description

[Boqun Feng <boqun.feng@gmail.com>]

On Wed, Mar 05, 2025 at 10:07:33PM +0100, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@gmail.com> writes:
> 
[...]
> >> >> >  rust/kernel/types.rs | 26 +++++++++++++++++++++-----
> >> >> >  1 file changed, 21 insertions(+), 5 deletions(-)
> >> >> >
> >> >> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> >> >> > index af30e9c0ebccb..f370cdb48a648 100644
> >> >> > --- a/rust/kernel/types.rs
> >> >> > +++ b/rust/kernel/types.rs
> >> >> > @@ -271,17 +271,33 @@ fn drop(&mut self) {
> >> >> >
> >> >> >  /// Stores an opaque value.
> >> >> >  ///
> >> >> > -/// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
> >> >> > +/// [`Opaque<T>`] opts out of the following Rust language invariants for the contained `T`
> >> >> > +/// by using [`UnsafeCell`]:
> >> >> >  ///
> >> >> > -/// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
> >> >> > -/// It gets rid of all the usual assumptions that Rust has for a value:
> >> >> > +/// * Initialization invariant - the contained value is allowed to be uninitialized and
> >> >> > +///   contain invalid bit patterns.
> >> >> > +/// * Immutability invariant - [`Opaque<T>`] allows interior mutability.
> >> >> > +/// * Uniqueness invariant - [`Opaque<T>`] allows aliasing of shared references.
> >> >>
> >> >> This last one is wrong (I know it's probably my fault, sorry). It should be:
> >> >>
> >> >>   /// * Uniqueness invariant - [`Opaque<T>`] allows aliasing of **exclusive** references.
> >> >>
> >> >
> >> > Hmm... are we trying to say "`&mut Opaque<T>` still cannot mean
> >> > noalias" here? If so I feel the wording might be confusing. I would
> >> > suggest we don't use "uniqueness" here. Maybe something like:
> >> >
> >> > * Always aliased invariant - `&mut` [`Opaque<T>`] is not necessarily a
> >> >   unique pointer, and thus the compiler cannot just make aliasing
> >> >   assumptions.
> >> >
> >> > (I use the wording from UnsafePinned[1], maybe there could be better
> >> > wording)
> >> >
> >> > [1]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#summary
> >>
> >> I like my wording better. It says the same with fewer words, and we have
> >> a more wordy section below anyway. We could combine:
> >>
> >>  * Uniqueness invariant - [`Opaque<T>`] allows aliasing of **exclusive**
> >>    references. That is, `&mut` [`Opaque<T>`] is not necessarily a unique
> >>    pointer.
> >>
> >>
> >> How is that?
> >>
> >
> > I think my biggest problem is the word "allows", the phrase "allows
> > aliasing" sounds like Opaque<T> provide something *optional*, however,
> > as I understand it, Opaque<T> here just disallows certain optimization
> > from compiler, in other words, programmers want to use `Opaque<T>` to
> > forbid something, hence "allows" may not be the good word to use here?
> >
> > How about:
> >
> >   * Uniqueness invariant - [`Opaque<T>`] disallow alias assumptions
> >     made by compiler on an *exclusive* references.
> 
> I don't object, but I feel like we should be able to define this within
> the context of the Rust language, without going into details about
> compiler internals.
> 
> Without `Opaque` it is illegal to have aliased `&mut T`. With `Opaque`,
> it is legal to have aliased `&mut Opaque<T>`. Or at least that is my
> understanding.
> 

Oh, I see. That's why you used "allows", however, allow me to explain
why it looks confusing to me. For `&mut T`, we can still have an
aliasing pointer (a pointer that points to the same variable as the
mutable reference):

	let mut x = ...;

	let mut_ref = &mut x;
	let aliased_ptr = mut_ref as *mut _;

it's just that if we dereference the pointer while the `&mut T` exists,
it's UB, e.g.

	unsafe { *aliased_ptr = ... };

and `Opaque<T>` allows above to be not a UB (there still are rules like
we cannot have data races, violating these rules would still be UB).

So to me "aliasing" can mean "having an aliasing pointer", and that as
shown above can be provided by both non-Opaque and Opaque cases. I
wonder whether "allow aliased/aliasing accesses" is better than "allow
aliasing" here.

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>