[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH5fLgjJRbjPnN58RuaBtNrY1A-tif3ohETLxdbaE4b46Hjbqg@mail.gmail.com>
Date: Tue, 24 Jun 2025 13:15:32 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: miguel.ojeda.sandonis@...il.com, a.hindborg@...nel.org,
alex.gaynor@...il.com, ojeda@...nel.org, anna-maria@...utronix.de,
bjorn3_gh@...tonmail.com, boqun.feng@...il.com, dakr@...nel.org,
frederic@...nel.org, gary@...yguo.net, jstultz@...gle.com,
linux-kernel@...r.kernel.org, lossin@...nel.org, lyude@...hat.com,
rust-for-linux@...r.kernel.org, sboyd@...nel.org, tglx@...utronix.de,
tmgross@...ch.edu
Subject: Re: [PATCH v1 1/2] rust: time: Rename Delta's methods as_micros_ceil
and as_millis
On Thu, Jun 19, 2025 at 8:08 AM FUJITA Tomonori
<fujita.tomonori@...il.com> wrote:
>
> On Wed, 18 Jun 2025 17:47:27 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:
>
> > On Wed, Jun 18, 2025 at 3:17 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
> >>
> >> There are also methods such as Duration::as_millis(). Yes, those take
> >> &self but &self is equivalent to self for Copy types, so there is no
> >> difference. And even if we did treat them differently,
> >> Duration::as_millis() is actually borrowed->owned as the return type
> >> is not a reference, so ...
> >
> > In most cases it may not matter, but even if taking either was exactly
> > the same, the point of the discussion(s) was what is more idiomatic,
> > i.e. how to spell those signatures.
> >
> > I understand you are saying that `Duration::as_millis()` is already a
> > stable example from the standard library of something that is not
> > borrowed -> borrowed, and thus the guidelines should be understood as
> > implying it is fine either way. It is still confusing, as shown by
> > these repeated discussions, and on the parameter's side of things,
> > they still seem to prefer `&self`, including in the equivalent methods
> > of this patch.
> >
> > Personally, I would use `self`, and clarify the guidelines.
>
> So would the function be defined like this?
>
> fn as_nanos(self) -> i64;
>
> If that's the case, then we've come full circle back to the original
> problem; Clippy warns against using as_* names for trait methods that
> take self as follows:
>
> warning: methods called `as_*` usually take `self` by reference or `self` by mutable reference
> --> /home/fujita/git/linux-rust/rust/kernel/time/hrtimer.rs:430:17
> |
> 430 | fn as_nanos(self) -> i64;
> | ^^^^
> |
> = help: consider choosing a less ambiguous name
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
> = note: `-W clippy::wrong-self-convention` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::wrong_self_convention)]`
>
> https://lore.kernel.org/rust-for-linux/20250610132823.3457263-2-fujita.tomonori@gmail.com/
Are we missing a derive(Copy) for this type? Clippy does not emit that
lint if the type is Copy:
https://github.com/rust-lang/rust-clippy/issues/273
Alice
Powered by blists - more mailing lists