Re: [PATCH v11 1/4] rust: types: Add Ownable/Owned types

[Oliver Mangold <oliver.mangold@pm.me>]

On 250819 1026, Benno Lossin wrote:
> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
> > On 250819 0027, Benno Lossin wrote:
> >> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
> >> > On 250818 1446, Andreas Hindborg wrote:
> >> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
> >> >> > +impl<T: OwnableMut> DerefMut for Owned<T> {
> >> >> > +    fn deref_mut(&mut self) -> &mut Self::Target {
> >> >> > +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
> >> >> > +        // return a mutable reference to it.
> >> >> > +        unsafe { self.ptr.as_mut() }
> >> >> > +    }
> >> >> > +}
> >> >>
> >> >> I think someone mentioned this before, but handing out mutable
> >> >> references can be a problem if `T: !Unpin`. For instance, we don't want
> >> >> to hand out `&mut Page` in case of `Owned<Page>`.
> >> >>
> >> >
> >> > That was the reason, why `OwnableMut` was introduced in the first place.
> >> > It's clear, I guess, that as-is it cannot be implemented on many classes.
> >>
> >> Yeah the safety requirements ensure that you can't implement it on
> >> `!Unpin` types.
> >>
> >> But I'm not sure it's useful then? As you said there aren't many types
> >> that will implement the type then, so how about we change the meaning
> >> and make it give out a pinned mutable reference instead?
> >
> > Making `deref_mut()` give out a pinned type won't work. The return types of
> > deref() are required to match.
> 
> I meant the changes that Andreas suggested.
> 
> >> > Good question, I have been thinking about it, too. But it might
> >> > be, that it isn't needed at all. As I understand, usually Rust wrappers
> >> > are around non-movable C structs. Do we actually have a useful application
> >> > for OwnableMut?
> >>
> >> Also, do we even need two different traits? Which types would only
> >> implement `Ownable` but not `OwnableMut`?
> >
> > I'm not 100% sure, but on a quick glance it looks indeed be safe to
> > substitute `OwnableMut` by `Unpin`.
> 
> We just have to change the safety requirements of `OwnableMut`.

You mean of `Ownable`, when `OwnableMut` is removed? Yes. A good question
in that context is, what it actually means to have an `&mut Opaque<T>`
where `T` is `Unpin`. If that implies being allowed to obtain an `&mut T`,
it would we easy, I guess.

> > If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
> > it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
> 
> Well the `DerefMut` impl still is convenient in the `Unpin` case.

I agree. What I meant is, it could not introduce an extra safety
requirement having it, if that indirect method can be used anyway.

But what I am wondering is, if we actually want to start using `Pin`
at all. Isn't `Opaque` currently used about everywhere pinning is needed?

Best,

Oliver