Re: [PATCH 1/2] rust: alloc: extend safety requirements of Vec::set_len()

[Danilo Krummrich <dakr@kernel.org>]

On Sun, Mar 16, 2025 at 09:38:23AM +0000, Benno Lossin wrote:
> On Sun Mar 16, 2025 at 1:33 AM CET, Tamir Duberstein wrote:
> > On Sat, Mar 15, 2025 at 2:36 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Sat, Mar 15, 2025 at 05:44:29PM +0000, Benno Lossin wrote:
> >> >
> >> > What is meant by "copied"? I agree with Tamir, this can probably just be
> >> > "taken ownership of".
> >>
> >> If not dropped in place, the only way to take ownership of the object is to copy
> >> the corresponding memory, i.e. the caller can not take ownership of the object
> >> at its current memory location, since this memory is owned by the vector.
> >
> > I think this is the point I was trying to make: the language semantics
> > are such that "taking ownership of" is equivalent to "become
> > responsible for the dropping of", so it doesn't make sense to write
> > that here, as though this requirement is specific to the semantics of
> > this function.
> >
> >> AFAIU, when we speak of ownership, we mean ownership over a value or object, but
> >> not ownership over the underlying memory.
> >>
> >> Hence, I think it's important to mention that it needs to be either dropped in
> >> place or copied in order to take ownership.
> >>
> >> Even if we agree that "drop in place" is considered as "take ownership", we need
> >> to differentiate between the special case of taking ownership where its dropped
> >> in place and the requirement to copy the value and then take ownership.
> 
> I get where you're coming from, but I wouldn't call that copying a
> value. Copying for me means that there would be two instances of the
> same object. But since the one in `[new_len, len)` will get invalidated,
> I don't think that copy makes sense.

What I meant is "copy the memory where the value is stored".

> 
> Also why even is this a safety requirement? There is `mem::forget`, so
> doing `vec.set_len(0)` can also be done by safe code:
> 
>     while let Some(val) = vec.pop() {
>         forget(val);
>     }
> 
> I don't think this needs to be a safety requirement.

Given that mem::forget() is considered safe, you clearly have a point here.

Let's go with just "must be taken ownership of" then. Unless there's subsequent
feedback, I won't send a new version for this, since you both already gave your
(conditional) RB for this.

(I tend to forget that mem::forget() is formally safe, since it can cause all
kinds of bugs, e.g. dead lock the whole machine if called for a lock in the
kernel, etc.)