Re: [PATCH v2 2/2] rust: time: Implement basic arithmetic operations for Delta

[Benno Lossin]

On Fri Aug 8, 2025 at 10:56 AM CEST, Alice Ryhl wrote:
> On Fri, Aug 08, 2025 at 08:42:30AM +0200, Benno Lossin wrote:
>> On Thu Aug 7, 2025 at 9:06 PM CEST, Lyude Paul wrote:
>> > While rvkms is only going to be using a few of these, since Deltas are
>> > basically the same as i64 it's easy enough to just implement all of the
>> > basic arithmetic operations for Delta types.
>> >
>> > Keep in mind there's one quirk here - the kernel has no support for
>> > i64 % i64 on 32 bit platforms, the closest we have is i64 % i32 through
>> > div_s64_rem(). So, instead of implementing ops::Rem or ops::RemAssign we
>> > simply provide Delta::rem_nanos().
>> 
>> We could still provide the trait implementations on CONFIG_64BIT? WDYT?
>> 
>> > +impl ops::Div for Delta {
>> > +    type Output = Self;
>> > +
>> > +    #[inline]
>> > +    fn div(self, rhs: Self) -> Self::Output {
>> > +        #[cfg(CONFIG_64BIT)]
>> > +        {
>> 
>> This pattern seems to be rather common in this patchset & in general I
>> think I've also seen it elsewhere. We should think about adding a
>> `if_cfg!` macro:
>> 
>>     Self {
>>         nanos: if_cfg! {
>>             if CONFIG_64BIT {
>>                 self.nanos / rhs.nanos
>>             } else {
>>                 unsafe { ... }
>>             }
>>         },
>>     }
>> 
>> But we can do that later. I'll file a good-first-issue.
>
> This kind of macro breaks rustfmt, so I wouldn't recommend it.

Ah then just use `if_cfg!(...)` that should work.

> I would suggest just using the native Rust pattern rather than
> introduce even more macros.

Trevor showed me https://github.com/rust-lang/rust/issues/115585, so we
can use that in the future. I still think the macro is useful though.

---
Cheers,
Benno