linux-kernel - Re: [PATCH v1 1/2] rust: time: Rename Delta's methods as_micros_ceil and as

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250625.013914.1516176234783852509.fujita.tomonori@gmail.com>
Date: Wed, 25 Jun 2025 01:39:14 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: aliceryhl@...gle.com
Cc: fujita.tomonori@...il.com, 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 Tue, 24 Jun 2025 15:45:45 +0100
Alice Ryhl <aliceryhl@...gle.com> wrote:

> On Tue, Jun 24, 2025 at 3:14 PM FUJITA Tomonori
> <fujita.tomonori@...il.com> wrote:
>>
>> On Tue, 24 Jun 2025 14:54:24 +0100
>> Alice Ryhl <aliceryhl@...gle.com> 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@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
>> >>
>> >> 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.

A trait method as_* should take &self. The same name of a method in a
structure which implements that trait can take self (if the structure
implements Copy and the cost is free) instead of &self, although it
doesn't match the same name of the trait method.

Also, the conversions table says that the ownership of as_* is
borrowed -> borrowed so neither of them matches the table.

Right?