Re: [PATCH v3 0/5] rust: time: Convert hrtimer to use Instant and Delta

[FUJITA Tomonori]

On Tue, 24 Jun 2025 21:03:24 +0200
Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:

> Andreas Hindborg <a.hindborg@xxxxxxxxxx> writes:
> 
>> "FUJITA Tomonori" <fujita.tomonori@xxxxxxxxx> writes:
>>
>>> On Tue, 24 Jun 2025 15:11:31 +0200
>>> Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
>>>
>>>>> and already introduces pain for
>>>>> others (and likely even more pain when we need to rename it back next
>>>>> cycle), it doesn't look like a good idea to keep it.
>>>>
>>>> Ok, I'll drop it.
>>>
>>> Do you want me to send the updated hrtimer conversion patchset
>>> (using as_* names)?
>>
>> No, I am just about finished fixing up the rest. You can check if it is
>> OK when I push.
> 
> I pushed it, please check.

Thanks!

The commit d9fc00dc7354 ("rust: time: Add HrTimerExpires trait") adds
to Instant structure:

+    #[inline]
+    pub(crate) fn as_nanos(&self) -> i64 {
+        self.inner
+    }

Would it be better to take self instead of &self?

pub(crate) fn as_nanos(self) -> i64 {

Because the as_nanos method on the Delta struct takes self, wouldnʼt it
be better to keep it consistent? I think that my original patch adds
into_nanos() that takes self. 

This commit also adds HrTimerExpire strait, which as_nanos() method
takes &self:

+/// Time representations that can be used as expiration values in [`HrTimer`].
+pub trait HrTimerExpires {
+    /// Converts the expiration time into a nanosecond representation.
+    ///
+    /// This value corresponds to a raw ktime_t value, suitable for passing to kernel
+    /// timer functions. The interpretation (absolute vs relative) depends on the
+    /// associated [HrTimerMode] in use.
+    fn as_nanos(&self) -> i64;
+}

That's because as I reported, Clippy warns if as_* take self.

As Alice pointed out, Clippy doesn't warn if a type implements
Copy. So we can add Copy to HrTimerExpires trait, then Clippy doesn't
warn about as_nanos method that takes self:

+/// Time representations that can be used as expiration values in [`HrTimer`].
+pub trait HrTimerExpires: Copy {
+    /// Converts the expiration time into a nanosecond representation.
+    ///
+    /// This value corresponds to a raw ktime_t value, suitable for passing to kernel
+    /// timer functions. The interpretation (absolute vs relative) depends on the
+    /// associated [HrTimerMode] in use.
+    fn as_nanos(self) -> i64;
+}

I'm fine with either (taking &self or Adding Copy).