[PATCH v1] rust: time: Add examples with doctest for Delta

FUJITA Tomonori posted 1 patch 3 months, 1 week ago
rust/kernel/time.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
[PATCH v1] rust: time: Add examples with doctest for Delta
Posted by FUJITA Tomonori 3 months, 1 week ago
Add examples with doctest for Delta methods and associated
functions. These examples explicitly test overflow and saturation
behavior.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 64c8dcf548d6..c6322275115a 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -228,11 +228,34 @@ impl Delta {
     /// A span of time equal to zero.
     pub const ZERO: Self = Self { nanos: 0 };
 
+    /// Create a new [`Delta`] from a number of nanoseconds.
+    ///
+    /// The `nanos` can range from [`i64::MIN`] to [`i64::MAX`].
+    #[inline]
+    pub const fn from_nanos(nanos: i64) -> Self {
+        Self { nanos }
+    }
+
     /// Create a new [`Delta`] from a number of microseconds.
     ///
     /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
     /// If `micros` is outside this range, `i64::MIN` is used for negative values,
     /// and `i64::MAX` is used for positive values due to saturation.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let delta = kernel::time::Delta::from_micros(5);
+    /// assert_eq!(delta.as_nanos(), 5_000);
+    /// let delta = kernel::time::Delta::from_micros(9_223_372_036_854_775);
+    /// assert_eq!(delta.as_nanos(), 9_223_372_036_854_775_000);
+    /// let delta = kernel::time::Delta::from_micros(9_223_372_036_854_776);
+    /// assert_eq!(delta.as_nanos(), i64::MAX);
+    /// let delta = kernel::time::Delta::from_micros(-9_223_372_036_854_775);
+    /// assert_eq!(delta.as_nanos(), -9_223_372_036_854_775_000);
+    /// let delta = kernel::time::Delta::from_micros(-9_223_372_036_854_776);
+    /// assert_eq!(delta.as_nanos(), i64::MIN);
+    /// ```
     #[inline]
     pub const fn from_micros(micros: i64) -> Self {
         Self {
@@ -245,6 +268,21 @@ pub const fn from_micros(micros: i64) -> Self {
     /// The `millis` can range from -9_223_372_036_854 to 9_223_372_036_854.
     /// If `millis` is outside this range, `i64::MIN` is used for negative values,
     /// and `i64::MAX` is used for positive values due to saturation.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let delta = kernel::time::Delta::from_millis(5);
+    /// assert_eq!(delta.as_nanos(), 5_000_000);
+    /// let delta = kernel::time::Delta::from_millis(9_223_372_036_854);
+    /// assert_eq!(delta.as_nanos(), 9_223_372_036_854_000_000);
+    /// let delta = kernel::time::Delta::from_millis(9_223_372_036_855);
+    /// assert_eq!(delta.as_nanos(), i64::MAX);
+    /// let delta = kernel::time::Delta::from_millis(-9_223_372_036_854);
+    /// assert_eq!(delta.as_nanos(), -9_223_372_036_854_000_000);
+    /// let delta = kernel::time::Delta::from_millis(-9_223_372_036_855);
+    /// assert_eq!(delta.as_nanos(), i64::MIN);
+    /// ```
     #[inline]
     pub const fn from_millis(millis: i64) -> Self {
         Self {
@@ -257,6 +295,21 @@ pub const fn from_millis(millis: i64) -> Self {
     /// The `secs` can range from -9_223_372_036 to 9_223_372_036.
     /// If `secs` is outside this range, `i64::MIN` is used for negative values,
     /// and `i64::MAX` is used for positive values due to saturation.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let delta = kernel::time::Delta::from_secs(5);
+    /// assert_eq!(delta.as_nanos(), 5_000_000_000);
+    /// let delta = kernel::time::Delta::from_secs(9_223_372_036);
+    /// assert_eq!(delta.as_nanos(), 9_223_372_036_000_000_000);
+    /// let delta = kernel::time::Delta::from_secs(9_223_372_037);
+    /// assert_eq!(delta.as_nanos(), i64::MAX);
+    /// let delta = kernel::time::Delta::from_secs(-9_223_372_036);
+    /// assert_eq!(delta.as_nanos(), -9_223_372_036_000_000_000);
+    /// let delta = kernel::time::Delta::from_secs(-9_223_372_037);
+    /// assert_eq!(delta.as_nanos(), i64::MIN);
+    /// ```
     #[inline]
     pub const fn from_secs(secs: i64) -> Self {
         Self {
@@ -284,6 +337,13 @@ pub const fn as_nanos(self) -> i64 {
 
     /// Return the smallest number of microseconds greater than or equal
     /// to the value in the [`Delta`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let delta = kernel::time::Delta::from_nanos(123_456_789);
+    /// assert_eq!(delta.as_micros_ceil(), 123_457);
+    /// ```
     #[inline]
     pub fn as_micros_ceil(self) -> i64 {
         #[cfg(CONFIG_64BIT)]
@@ -299,6 +359,13 @@ pub fn as_micros_ceil(self) -> i64 {
     }
 
     /// Return the number of milliseconds in the [`Delta`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let delta = kernel::time::Delta::from_nanos(123_456_789);
+    /// assert_eq!(delta.as_millis(), 123);
+    /// ```
     #[inline]
     pub fn as_millis(self) -> i64 {
         #[cfg(CONFIG_64BIT)]

base-commit: d4b29ddf82a458935f1bd4909b8a7a13df9d3bdc
-- 
2.43.0
Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by Andreas Hindborg 3 months, 1 week ago
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> Add examples with doctest for Delta methods and associated
> functions. These examples explicitly test overflow and saturation
> behavior.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/time.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 64c8dcf548d6..c6322275115a 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -228,11 +228,34 @@ impl Delta {
>      /// A span of time equal to zero.
>      pub const ZERO: Self = Self { nanos: 0 };
>
> +    /// Create a new [`Delta`] from a number of nanoseconds.
> +    ///
> +    /// The `nanos` can range from [`i64::MIN`] to [`i64::MAX`].
> +    #[inline]
> +    pub const fn from_nanos(nanos: i64) -> Self {
> +        Self { nanos }
> +    }
> +
>      /// Create a new [`Delta`] from a number of microseconds.
>      ///
>      /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
>      /// If `micros` is outside this range, `i64::MIN` is used for negative values,
>      /// and `i64::MAX` is used for positive values due to saturation.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// let delta = kernel::time::Delta::from_micros(5);
> +    /// assert_eq!(delta.as_nanos(), 5_000);
> +    /// let delta = kernel::time::Delta::from_micros(9_223_372_036_854_775);
> +    /// assert_eq!(delta.as_nanos(), 9_223_372_036_854_775_000);
> +    /// let delta = kernel::time::Delta::from_micros(9_223_372_036_854_776);
> +    /// assert_eq!(delta.as_nanos(), i64::MAX);
> +    /// let delta = kernel::time::Delta::from_micros(-9_223_372_036_854_775);
> +    /// assert_eq!(delta.as_nanos(), -9_223_372_036_854_775_000);
> +    /// let delta = kernel::time::Delta::from_micros(-9_223_372_036_854_776);
> +    /// assert_eq!(delta.as_nanos(), i64::MIN);
> +    /// ```


I think this kind of test would be better suited for the new `#[test]`
macro. Would you agree?


Best regards,
Andreas Hindborg
Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by Miguel Ojeda 3 months, 1 week ago
On Wed, Jul 2, 2025 at 10:36 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> I think this kind of test would be better suited for the new `#[test]`
> macro. Would you agree?

I don't mind seeing the edge cases, since the behavior is mentioned in
the docs, just like sometimes we show e.g. the `Err`/`None` cases in
other functions etc., and it may help to further highlight that this
can actually return possibly unexpected values.

It is also what the standard library does, at least in some similar cases, e.g.

    https://doc.rust-lang.org/std/primitive.i64.html#method.saturating_add

Maybe instead we can help making it a bit more readable, e.g. avoiding
the intermediate variable to have a single line plus using a `# use
Delta` to further reduce the line length?

Also adding a newline between the "normal" case and the saturation
cases would probably help too.

Cheers,
Miguel
Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by FUJITA Tomonori 3 months ago
On Wed, 2 Jul 2025 14:22:48 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Wed, Jul 2, 2025 at 10:36 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> I think this kind of test would be better suited for the new `#[test]`
>> macro. Would you agree?
> 
> I don't mind seeing the edge cases, since the behavior is mentioned in
> the docs, just like sometimes we show e.g. the `Err`/`None` cases in
> other functions etc., and it may help to further highlight that this
> can actually return possibly unexpected values.
> 
> It is also what the standard library does, at least in some similar cases, e.g.
> 
>     https://doc.rust-lang.org/std/primitive.i64.html#method.saturating_add
> 
> Maybe instead we can help making it a bit more readable, e.g. avoiding
> the intermediate variable to have a single line plus using a `# use
> Delta` to further reduce the line length?
> 
> Also adding a newline between the "normal" case and the saturation
> cases would probably help too.

I've updated from_micros() based on the above suggestion - looks
better to me. What do you think?

/// # Examples
///
/// ```
/// use kernel::time::Delta;
///
/// assert_eq!(Delta::from_micros(5).as_nanos(), 5_000);
/// assert_eq!(Delta::from_micros(9_223_372_036_854_775).as_nanos(), 9_223_372_036_854_775_000);
/// assert_eq!(Delta::from_micros(-9_223_372_036_854_775).as_nanos(), -9_223_372_036_854_775_000);
///
/// assert_eq!(Delta::from_micros(9_223_372_036_854_776).as_nanos(), i64::MAX);
/// assert_eq!(Delta::from_micros(-9_223_372_036_854_776).as_nanos(), i64::MIN);
/// ```
#[inline]
pub const fn from_micros(micros: i64) -> Self {
    Self {
        nanos: micros.saturating_mul(NSEC_PER_USEC),
    }
}


Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by Andreas Hindborg 3 months ago
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Wed, 2 Jul 2025 14:22:48 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>
>> On Wed, Jul 2, 2025 at 10:36 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>> I think this kind of test would be better suited for the new `#[test]`
>>> macro. Would you agree?
>>
>> I don't mind seeing the edge cases, since the behavior is mentioned in
>> the docs, just like sometimes we show e.g. the `Err`/`None` cases in
>> other functions etc., and it may help to further highlight that this
>> can actually return possibly unexpected values.
>>
>> It is also what the standard library does, at least in some similar cases, e.g.
>>
>>     https://doc.rust-lang.org/std/primitive.i64.html#method.saturating_add
>>
>> Maybe instead we can help making it a bit more readable, e.g. avoiding
>> the intermediate variable to have a single line plus using a `# use
>> Delta` to further reduce the line length?
>>
>> Also adding a newline between the "normal" case and the saturation
>> cases would probably help too.
>
> I've updated from_micros() based on the above suggestion - looks
> better to me. What do you think?
>
> /// # Examples
> ///
> /// ```
> /// use kernel::time::Delta;
> ///
> /// assert_eq!(Delta::from_micros(5).as_nanos(), 5_000);
> /// assert_eq!(Delta::from_micros(9_223_372_036_854_775).as_nanos(), 9_223_372_036_854_775_000);
> /// assert_eq!(Delta::from_micros(-9_223_372_036_854_775).as_nanos(), -9_223_372_036_854_775_000);
> ///
> /// assert_eq!(Delta::from_micros(9_223_372_036_854_776).as_nanos(), i64::MAX);
> /// assert_eq!(Delta::from_micros(-9_223_372_036_854_776).as_nanos(), i64::MIN);
> /// ```
> #[inline]
> pub const fn from_micros(micros: i64) -> Self {
>     Self {
>         nanos: micros.saturating_mul(NSEC_PER_USEC),
>     }
> }

From my point of view, I would like to see

  assert_eq!(Delta::from_micros(9_223_372_036_854_775).as_nanos(), 9_223_372_036_854_775_000);
  assert_eq!(Delta::from_micros(-9_223_372_036_854_775).as_nanos(), -9_223_372_036_854_775_000);

  assert_eq!(Delta::from_micros(9_223_372_036_854_776).as_nanos(), i64::MAX);
  assert_eq!(Delta::from_micros(-9_223_372_036_854_776).as_nanos(), i64::MIN);

moved to a `#[test]` block. They do not provide value for me when
reading the docs. I don't know what the very large constants are and
what I am supposed to learn from those asserts. Maybe if the constants
had a name, or were expressed relative to another constant?

I think this one:

  /// assert_eq!(Delta::from_micros(5).as_nanos(), 5_000);

is fine in the documentation block.


Best regards,
Andreas Hindborg
Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by Miguel Ojeda 3 months ago
On Fri, Jul 4, 2025 at 9:28 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> I don't know what the very large constants are

They come from `i64::MAX / 1000`.

> Maybe if the constants had a name, or were expressed relative to another constant?

Yes, we should do that.

Cheers,
Miguel
Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by FUJITA Tomonori 3 months, 1 week ago
On Wed, 02 Jul 2025 10:36:19 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> 
>> Add examples with doctest for Delta methods and associated
>> functions. These examples explicitly test overflow and saturation
>> behavior.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>>  rust/kernel/time.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
>> index 64c8dcf548d6..c6322275115a 100644
>> --- a/rust/kernel/time.rs
>> +++ b/rust/kernel/time.rs
>> @@ -228,11 +228,34 @@ impl Delta {
>>      /// A span of time equal to zero.
>>      pub const ZERO: Self = Self { nanos: 0 };
>>
>> +    /// Create a new [`Delta`] from a number of nanoseconds.
>> +    ///
>> +    /// The `nanos` can range from [`i64::MIN`] to [`i64::MAX`].
>> +    #[inline]
>> +    pub const fn from_nanos(nanos: i64) -> Self {
>> +        Self { nanos }
>> +    }
>> +
>>      /// Create a new [`Delta`] from a number of microseconds.
>>      ///
>>      /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
>>      /// If `micros` is outside this range, `i64::MIN` is used for negative values,
>>      /// and `i64::MAX` is used for positive values due to saturation.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// let delta = kernel::time::Delta::from_micros(5);
>> +    /// assert_eq!(delta.as_nanos(), 5_000);
>> +    /// let delta = kernel::time::Delta::from_micros(9_223_372_036_854_775);
>> +    /// assert_eq!(delta.as_nanos(), 9_223_372_036_854_775_000);
>> +    /// let delta = kernel::time::Delta::from_micros(9_223_372_036_854_776);
>> +    /// assert_eq!(delta.as_nanos(), i64::MAX);
>> +    /// let delta = kernel::time::Delta::from_micros(-9_223_372_036_854_775);
>> +    /// assert_eq!(delta.as_nanos(), -9_223_372_036_854_775_000);
>> +    /// let delta = kernel::time::Delta::from_micros(-9_223_372_036_854_776);
>> +    /// assert_eq!(delta.as_nanos(), i64::MIN);
>> +    /// ```
> 
> 
> I think this kind of test would be better suited for the new `#[test]`
> macro. Would you agree?

I think that Miguel suggested adding examples but either is fine by me:

https://lore.kernel.org/lkml/CANiq72kiTwpcH6S0XaTEBnLxqyJ6EDVLoZPi9X+MWkanK5wq=w@mail.gmail.com/
Re: [PATCH v1] rust: time: Add examples with doctest for Delta
Posted by Miguel Ojeda 3 months, 1 week ago
On Wed, Jul 2, 2025 at 11:09 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> I think that Miguel suggested adding examples but either is fine by me:
>
> https://lore.kernel.org/lkml/CANiq72kiTwpcH6S0XaTEBnLxqyJ6EDVLoZPi9X+MWkanK5wq=w@mail.gmail.com/

Ah, if Andreas was talking about the examples in general (not just the
edge cases within each example, which is what I understood in my
previous reply), then we definitely want to have examples in our
documentation. Unit tests serve a different purpose.

It is a balance -- to me, examples should try to be minimal and yet
show all "cases" a user may need to care about, but if other tests
would be useful as tests (e.g. passing `i64::MAX` as input), then
those would be unit tests.

Cheers,
Miguel