Re: [PATCH v1 1/2] rust: time: Rename Delta's methods as_micros_ceil and as_millis

[Alice Ryhl]

On Tue, Jun 24, 2025 at 3:14 PM FUJITA Tomonori
<fujita.tomonori@xxxxxxxxx> wrote:
>
> On Tue, 24 Jun 2025 14:54:24 +0100
> Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
>
> >> >> 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@xxxxxxxxx/
> >> >
> >> > 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
> >>
> >> I think that both Delta and Instant structs implement Copy.
> >>
> >> #[repr(transparent)]
> >> #[derive(PartialEq, PartialOrd, Eq, Ord)]
> >> pub struct Instant<C: ClockSource> {
> >>     inner: bindings::ktime_t,
> >>     _c: PhantomData<C>,
> >> }
> >>
> >> impl<C: ClockSource> Clone for Instant<C> {
> >>     fn clone(&self) -> Self {
> >>         *self
> >>     }
> >> }
> >>
> >> impl<C: ClockSource> Copy for Instant<C> {}
> >>
> >> #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
> >> pub struct Delta {
> >>     nanos: i64,
> >> }
> >>
> >> The above warning is about the trait method.
> >
> > Wait, it's a trait method!?
>
> Yes. Clippy warns the following implementation:
>
> pub trait HrTimerExpires {
>     fn as_nanos(self) -> i64;
> }
>
> Clippy doesn't warn when the methods on Delta and Instant are written
> similarly. So Clippy is happy about the followings:
>
> pub trait HrTimerExpires {
>     fn as_nanos(&self) -> i64;
> }
>
> impl HrTimerExpires for Delta {
>     fn as_nanos(&self) -> i64 {
>         Delta::as_nanos(*self)
>     }
> }

If it's a trait method, then it should take &self because you don't
know whether it's Copy.

Alice