Skip to content

Commit

Permalink
Added Duration::try_milliseconds()
Browse files Browse the repository at this point in the history
  - Added a new Duration::try_milliseconds() function, to attempt to
    create a new milliseconds-based Duration, and return None if it
    fails, based on checking against MAX and MIN. Currently, failure can
    only happen for exactly one value, which is i64::MIN.

  - Updated Duration::milliseconds() to call try_milliseconds() and
    panic if None is returned. Although panicking in production code and
    especially library code is bad, this is in keeping with current
    Chrono behaviour. Note that this function is now no longer const.

  - Updated the Duration::milliseconds() documentation to make it clear
    that it now panics.

  - Added documentation to Duration::microseconds() and nanoseconds() to
    make it clear that they are infallible.

  - All tests now pass, including the one previously ignored.
  • Loading branch information
danwilliams committed Jan 27, 2024
1 parent 0c82d37 commit c2d2cdd
Showing 1 changed file with 38 additions and 10 deletions.
48 changes: 38 additions & 10 deletions src/duration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,44 @@ impl Duration {
}

/// Makes a new `Duration` with the given number of milliseconds.
///
/// # Panics
///
/// Panics when the duration is out of bounds, i.e. when the duration is
/// more than `i64::MAX` milliseconds or less than `-i64::MAX` milliseconds.
/// Notably, this is not the same as `i64::MIN`.
#[inline]
pub const fn milliseconds(milliseconds: i64) -> Duration {
pub fn milliseconds(milliseconds: i64) -> Duration {
Duration::try_milliseconds(milliseconds).expect("Duration::milliseconds out of bounds")
}

/// Makes a new `Duration` with the given number of milliseconds.
///
/// # Errors
///
/// Returns `None` when the duration is more than `i64::MAX` milliseconds or
/// less than `-i64::MAX` milliseconds. Notably, this is not the same as
/// `i64::MIN`.
#[inline]
pub fn try_milliseconds(milliseconds: i64) -> Option<Duration> {
let (secs, millis) = div_mod_floor_64(milliseconds, MILLIS_PER_SEC);
let nanos = millis as i32 * NANOS_PER_MILLI;
Duration { secs, nanos }
let d = Duration { secs, nanos: millis as i32 * NANOS_PER_MILLI };
// Technically we don't need to compare against MAX, as this function
// accepts an i64, and MAX is aligned to i64::MAX milliseconds. However,
// it is good practice to check it here in order to catch any future
// changes that might be made.
if d < MIN || d > MAX {
return None;
}
Some(d)
}

/// Makes a new `Duration` with the given number of microseconds.
///
/// The number of microseconds acceptable by this constructor is far less
/// than the total number that can actually be stored in a `Duration`, so it
/// is not possible to specify a value that would be out of bounds. This
/// function is therefore infallible.
#[inline]
pub const fn microseconds(microseconds: i64) -> Duration {
let (secs, micros) = div_mod_floor_64(microseconds, MICROS_PER_SEC);
Expand All @@ -227,6 +257,11 @@ impl Duration {
}

/// Makes a new `Duration` with the given number of nanoseconds.
///
/// The number of nanoseconds acceptable by this constructor is far less
/// than the total number that can actually be stored in a `Duration`, so it
/// is not possible to specify a value that would be out of bounds. This
/// function is therefore infallible.
#[inline]
pub const fn nanoseconds(nanos: i64) -> Duration {
let (secs, nanos) = div_mod_floor_64(nanos, NANOS_PER_SEC as i64);
Expand Down Expand Up @@ -709,19 +744,12 @@ mod tests {
.is_none());
}
#[test]
#[ignore]
#[should_panic(expected = "Duration::milliseconds out of bounds")]
fn test_duration_milliseconds_min_underflow_panic() {
// Here we ensure that trying to create a value one millisecond below the
// minimum storable value will fail. This test is necessary because the
// storable range is -i64::MAX, but the constructor type of i64 will allow
// i64::MIN, which is one value below.
// WARNING:
// This test currently fails, because Duration::milliseconds() does not
// check against MIN, and hence allows an unsupported value to be created.
// Therefore it is currently marked as "ignore", until the fix is applied.
// This is because the current implementation of milliseconds() cannot
// actually panic.
let _ = Duration::milliseconds(i64::MIN); // Same as -i64::MAX - 1
}

Expand Down

0 comments on commit c2d2cdd

Please sign in to comment.