[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ-ks9kcNvGqGrU1nKjYs_4XPbdxo2cW8Tj9JOGJesGO4StdAw@mail.gmail.com>
Date: Tue, 18 Mar 2025 10:12:28 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Benno Lossin <benno.lossin@...ton.me>, Danilo Krummrich <dakr@...nel.org>,
Andrew Ballance <andrewjballance@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] rust: alloc: add `Vec::dec_len`
On Tue, Mar 18, 2025 at 5:30 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> On Mon, Mar 17, 2025 at 09:53:04AM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 8:59 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 11:47:50AM +0000, Alice Ryhl wrote:
> > > > On Mon, Mar 17, 2025 at 07:34:44AM -0400, Tamir Duberstein wrote:
> > > > > On Mon, Mar 17, 2025 at 6:04 AM Benno Lossin <benno.lossin@...ton.me> wrote:
> > > > > >
> > > > > > On Sun Mar 16, 2025 at 11:32 PM CET, Tamir Duberstein wrote:
> > > > > > > Add `Vec::dec_len` that reduces the length of the receiver. This method
> > > > > > > is intended to be used from methods that remove elements from `Vec` such
> > > > > > > as `truncate`, `pop`, `remove`, and others. This method is intentionally
> > > > > > > not `pub`.
> > > > > >
> > > > > > I think it should be `pub`. Otherwise we're loosing functionality
> > > > > > compared to now. If one decides to give the raw pointer to some C API
> > > > > > that takes ownership of the pointer, then I want them to be able to call
> > > > > > `dec_len` manually.
> > > > >
> > > > > This is premature. It is trivial to make this function pub when the need arises.
> > > >
> > > > Normally I'd agree with Benno, but in this case I think having it
> > > > private is preferable. The function is safe, so it's too easy for
> > > > end-users to confuse it with truncate.
> > >
> > > Thinking more about this ... I think we should have `set_len` and
> > > `inc_len` instead. That way, both methods are unsafe so people will not
> > > accidentally use `set_len` when they meant to use `truncate`.
> > >
> > > Alice
> >
> > Isn't it fine to keep this function unsafe given that it can break
> > invariants in its current form? As expressed earlier, I would prefer
> > not to make it safe by using saturating_sub.
>
> I guess that's okay. But I would think that with the exception of
> `Vec::pop`, the interface you want for Vec methods that decrease the
> length is set_len, not dec_len. That is the case for clear, truncate,
> and drain.
>
> Even for the Vec methods that actually use
>
> set_len(original_len - removed_cnt)
>
> they make this call after having previously called set_len(0).
The methods you're describing are all on Vec, right? In other words,
their usage calls for a private `dec_len` or `set_len`. As I've said
repeatedly in the course of this discussion: I would prefer not to
introduce `dec_len` at all here. It (or `set_len`) can be introduced
in the series that adds truncate or your patch that adds clear, where
its signature can be properly scrutinized in the context of an actual
caller.
Tamir
Powered by blists - more mailing lists