* [PATCH v1] rust: time: Add examples with doctest for Delta @ 2025-07-01 0:18 ` FUJITA Tomonori 2025-07-02 8:36 ` Andreas Hindborg 0 siblings, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2025-07-01 0:18 UTC (permalink / raw) To: a.hindborg, alex.gaynor, ojeda Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-01 0:18 ` [PATCH v1] rust: time: Add examples with doctest for Delta FUJITA Tomonori @ 2025-07-02 8:36 ` Andreas Hindborg 2025-07-02 9:09 ` FUJITA Tomonori 2025-07-02 12:22 ` Miguel Ojeda 0 siblings, 2 replies; 8+ messages in thread From: Andreas Hindborg @ 2025-07-02 8:36 UTC (permalink / raw) To: FUJITA Tomonori Cc: alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross "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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-02 8:36 ` Andreas Hindborg @ 2025-07-02 9:09 ` FUJITA Tomonori 2025-07-02 12:34 ` Miguel Ojeda 2025-07-02 12:22 ` Miguel Ojeda 1 sibling, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2025-07-02 9:09 UTC (permalink / raw) To: a.hindborg, ojeda Cc: fujita.tomonori, alex.gaynor, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross 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/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-02 9:09 ` FUJITA Tomonori @ 2025-07-02 12:34 ` Miguel Ojeda 0 siblings, 0 replies; 8+ messages in thread From: Miguel Ojeda @ 2025-07-02 12:34 UTC (permalink / raw) To: FUJITA Tomonori Cc: a.hindborg, ojeda, alex.gaynor, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-02 8:36 ` Andreas Hindborg 2025-07-02 9:09 ` FUJITA Tomonori @ 2025-07-02 12:22 ` Miguel Ojeda 2025-07-04 0:36 ` FUJITA Tomonori 1 sibling, 1 reply; 8+ messages in thread From: Miguel Ojeda @ 2025-07-02 12:22 UTC (permalink / raw) To: Andreas Hindborg Cc: FUJITA Tomonori, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-02 12:22 ` Miguel Ojeda @ 2025-07-04 0:36 ` FUJITA Tomonori 2025-07-04 7:28 ` Andreas Hindborg 0 siblings, 1 reply; 8+ messages in thread From: FUJITA Tomonori @ 2025-07-04 0:36 UTC (permalink / raw) To: miguel.ojeda.sandonis, a.hindborg Cc: fujita.tomonori, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross 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), } } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-04 0:36 ` FUJITA Tomonori @ 2025-07-04 7:28 ` Andreas Hindborg 2025-07-04 7:38 ` Miguel Ojeda 0 siblings, 1 reply; 8+ messages in thread From: Andreas Hindborg @ 2025-07-04 7:28 UTC (permalink / raw) To: FUJITA Tomonori Cc: miguel.ojeda.sandonis, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross "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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] rust: time: Add examples with doctest for Delta 2025-07-04 7:28 ` Andreas Hindborg @ 2025-07-04 7:38 ` Miguel Ojeda 0 siblings, 0 replies; 8+ messages in thread From: Miguel Ojeda @ 2025-07-04 7:38 UTC (permalink / raw) To: Andreas Hindborg Cc: FUJITA Tomonori, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-04 7:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <kyrCsGDYN5QdXPkUe4OxMxU2UzHfJ-vH5z643gA-_KFJIaew0duyMXU0yfTrGN5ZCNaecP8Yu2kIXKXMoWS2lA==@protonmail.internalid>
2025-07-01 0:18 ` [PATCH v1] rust: time: Add examples with doctest for Delta FUJITA Tomonori
2025-07-02 8:36 ` Andreas Hindborg
2025-07-02 9:09 ` FUJITA Tomonori
2025-07-02 12:34 ` Miguel Ojeda
2025-07-02 12:22 ` Miguel Ojeda
2025-07-04 0:36 ` FUJITA Tomonori
2025-07-04 7:28 ` Andreas Hindborg
2025-07-04 7:38 ` Miguel Ojeda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).