rust/kernel/time.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+)
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
"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
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
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), } }
"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
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
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/
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
© 2016 - 2025 Red Hat, Inc.