Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make conversions to and from std::time::SystemTime non-panicking #1421

Merged
merged 1 commit into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 48 additions & 15 deletions src/datetime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ use crate::naive::{Days, IsoWeek, NaiveDate, NaiveDateTime, NaiveTime};
use crate::offset::Local;
use crate::offset::{FixedOffset, Offset, TimeZone, Utc};
use crate::try_opt;
#[cfg(any(feature = "clock", feature = "std"))]
use crate::OutOfRange;
use crate::{Datelike, Months, TimeDelta, Timelike, Weekday};

#[cfg(any(feature = "rkyv", feature = "rkyv-16", feature = "rkyv-32", feature = "rkyv-64"))]
Expand Down Expand Up @@ -1536,42 +1538,73 @@ impl str::FromStr for DateTime<Local> {
}

#[cfg(feature = "std")]
impl From<SystemTime> for DateTime<Utc> {
fn from(t: SystemTime) -> DateTime<Utc> {
impl TryFrom<SystemTime> for DateTime<Utc> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is nice to already have this feature on the 0.4 branch (main). Do you want to target that branch, and add these implementations instead of replacing the existing ones?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't. impl From<T> and impl TryFrom<T> are mutually exclusive. The standard library has a blanket impl TryFrom<T> here that covers every impl From<T>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's unfortunate 😞.

type Error = OutOfRange;

fn try_from(t: SystemTime) -> Result<DateTime<Utc>, OutOfRange> {
let (sec, nsec) = match t.duration_since(UNIX_EPOCH) {
pitdicker marked this conversation as resolved.
Show resolved Hide resolved
Ok(dur) => (dur.as_secs() as i64, dur.subsec_nanos()),
Ok(dur) => {
// `t` is at or after the Unix epoch.
let sec = i64::try_from(dur.as_secs()).map_err(|_| OutOfRange::new())?;
let nsec = dur.subsec_nanos();
(sec, nsec)
}
Err(e) => {
// unlikely but should be handled
// `t` is before the Unix epoch. `e.duration()` is how long before the epoch it
// is.
let dur = e.duration();
let (sec, nsec) = (dur.as_secs() as i64, dur.subsec_nanos());
let sec = i64::try_from(dur.as_secs()).map_err(|_| OutOfRange::new())?;
let nsec = dur.subsec_nanos();
if nsec == 0 {
// Overflow safety: `sec` was returned by `dur.as_secs()`, and is guaranteed to
// be non-negative. Negating a non-negative signed integer cannot overflow.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good comments on overflow cases 👍

(-sec, 0)
} else {
(-sec - 1, 1_000_000_000 - nsec)
// Overflow safety: In addition to the above, `-x - 1`, where `x` is a
// non-negative signed integer, also cannot overflow.
let sec = -sec - 1;
// Overflow safety: `nsec` was returned by `dur.subsec_nanos()`, and is
// guaranteed to be between 0 and 999_999_999 inclusive. Subtracting it from
// 1_000_000_000 is therefore guaranteed not to overflow.
let nsec = 1_000_000_000 - nsec;
(sec, nsec)
}
}
};
Utc.timestamp(sec, nsec).unwrap()
Utc.timestamp(sec, nsec).single().ok_or(OutOfRange::new())
}
}

#[cfg(feature = "clock")]
impl From<SystemTime> for DateTime<Local> {
fn from(t: SystemTime) -> DateTime<Local> {
DateTime::<Utc>::from(t).with_timezone(&Local)
impl TryFrom<SystemTime> for DateTime<Local> {
type Error = OutOfRange;

fn try_from(t: SystemTime) -> Result<DateTime<Local>, OutOfRange> {
DateTime::<Utc>::try_from(t).map(|t| t.with_timezone(&Local))
}
}

#[cfg(feature = "std")]
impl<Tz: TimeZone> From<DateTime<Tz>> for SystemTime {
fn from(dt: DateTime<Tz>) -> SystemTime {
impl<Tz: TimeZone> TryFrom<DateTime<Tz>> for SystemTime {
type Error = OutOfRange;

fn try_from(dt: DateTime<Tz>) -> Result<SystemTime, OutOfRange> {
let sec = dt.timestamp();
let sec_abs = sec.unsigned_abs();
let nsec = dt.timestamp_subsec_nanos();
if sec < 0 {
// unlikely but should be handled
UNIX_EPOCH - Duration::new(-sec as u64, 0) + Duration::new(0, nsec)
// `dt` is before the Unix epoch.
let mut t =
UNIX_EPOCH.checked_sub(Duration::new(sec_abs, 0)).ok_or_else(OutOfRange::new)?;

// Overflow safety: `t` is before the Unix epoch. Adding nanoseconds therefore cannot
// overflow.
t += Duration::new(0, nsec);

Ok(t)
} else {
UNIX_EPOCH + Duration::new(sec as u64, nsec)
// `dt` is after the Unix epoch.
UNIX_EPOCH.checked_add(Duration::new(sec_abs, nsec)).ok_or_else(OutOfRange::new)
}
}
}
Expand Down
34 changes: 23 additions & 11 deletions src/datetime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,9 +1150,9 @@ fn test_from_system_time() {
let epoch = Utc.with_ymd_and_hms(1970, 1, 1, 0, 0, 0).unwrap();

// SystemTime -> DateTime<Utc>
assert_eq!(DateTime::<Utc>::from(UNIX_EPOCH), epoch);
assert_eq!(DateTime::<Utc>::try_from(UNIX_EPOCH).unwrap(), epoch);
assert_eq!(
DateTime::<Utc>::from(UNIX_EPOCH + Duration::new(999_999_999, nanos)),
DateTime::<Utc>::try_from(UNIX_EPOCH + Duration::new(999_999_999, nanos)).unwrap(),
Utc.from_local_datetime(
&NaiveDate::from_ymd_opt(2001, 9, 9)
.unwrap()
Expand All @@ -1162,7 +1162,7 @@ fn test_from_system_time() {
.unwrap()
);
assert_eq!(
DateTime::<Utc>::from(UNIX_EPOCH - Duration::new(999_999_999, nanos)),
DateTime::<Utc>::try_from(UNIX_EPOCH - Duration::new(999_999_999, nanos)).unwrap(),
Utc.from_local_datetime(
&NaiveDate::from_ymd_opt(1938, 4, 24)
.unwrap()
Expand All @@ -1171,45 +1171,57 @@ fn test_from_system_time() {
)
.unwrap()
);
assert_eq!(
DateTime::<Utc>::try_from(UNIX_EPOCH - Duration::new(999_999_999, 0)).unwrap(),
Utc.from_local_datetime(
&NaiveDate::from_ymd_opt(1938, 4, 24).unwrap().and_hms_nano_opt(22, 13, 21, 0).unwrap()
)
.unwrap()
);

// DateTime<Utc> -> SystemTime
assert_eq!(SystemTime::from(epoch), UNIX_EPOCH);
assert_eq!(SystemTime::try_from(epoch).unwrap(), UNIX_EPOCH);
assert_eq!(
SystemTime::from(
SystemTime::try_from(
Utc.from_local_datetime(
&NaiveDate::from_ymd_opt(2001, 9, 9)
.unwrap()
.and_hms_nano_opt(1, 46, 39, nanos)
.unwrap()
)
.unwrap()
),
)
.unwrap(),
UNIX_EPOCH + Duration::new(999_999_999, nanos)
);
assert_eq!(
SystemTime::from(
SystemTime::try_from(
Utc.from_local_datetime(
&NaiveDate::from_ymd_opt(1938, 4, 24)
.unwrap()
.and_hms_nano_opt(22, 13, 20, 1_000)
.unwrap()
)
.unwrap()
),
)
.unwrap(),
UNIX_EPOCH - Duration::new(999_999_999, nanos)
);

// DateTime<any tz> -> SystemTime (via `with_timezone`)
#[cfg(feature = "clock")]
{
assert_eq!(SystemTime::from(epoch.with_timezone(&Local)), UNIX_EPOCH);
let local_at_epoch = epoch.with_timezone(&Local);
let local_systemtime_at_epoch = SystemTime::try_from(local_at_epoch).unwrap();
assert_eq!(local_systemtime_at_epoch, UNIX_EPOCH);
assert_eq!(DateTime::<Local>::try_from(local_systemtime_at_epoch).unwrap(), local_at_epoch);
}
assert_eq!(
SystemTime::from(epoch.with_timezone(&FixedOffset::east(32400).unwrap())),
SystemTime::try_from(epoch.with_timezone(&FixedOffset::east(32400).unwrap())).unwrap(),
UNIX_EPOCH
);
assert_eq!(
SystemTime::from(epoch.with_timezone(&FixedOffset::west(28800).unwrap())),
SystemTime::try_from(epoch.with_timezone(&FixedOffset::west(28800).unwrap())).unwrap(),
UNIX_EPOCH
);
}
Expand Down
18 changes: 12 additions & 6 deletions src/offset/utc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use core::fmt;
not(any(target_os = "emscripten", target_os = "wasi"))
))
))]
use std::time::{SystemTime, UNIX_EPOCH};
use std::time::SystemTime;

#[cfg(any(feature = "rkyv", feature = "rkyv-16", feature = "rkyv-32", feature = "rkyv-64"))]
use rkyv::{Archive, Deserialize, Serialize};
Expand All @@ -21,6 +21,8 @@ use super::{FixedOffset, LocalResult, Offset, TimeZone};
use crate::naive::NaiveDateTime;
#[cfg(feature = "now")]
use crate::DateTime;
#[cfg(all(feature = "now", doc))]
use crate::OutOfRange;

/// The UTC time zone. This is the most efficient time zone when you don't need the local time.
/// It is also used as an offset (which is also a dummy type).
Expand Down Expand Up @@ -74,18 +76,22 @@ impl Utc {
/// let offset = FixedOffset::east(5 * 60 * 60).unwrap();
/// let now_with_offset = Utc::now().with_timezone(&offset);
/// ```
///
/// # Panics
///
/// Panics if the system clock is set to a time in the extremely distant past or future, such
/// that it is out of the range representable by `DateTime<Utc>`. It is assumed that this
/// crate will no longer be in use by that time.
#[cfg(not(all(
target_arch = "wasm32",
feature = "wasmbind",
not(any(target_os = "emscripten", target_os = "wasi"))
)))]
#[must_use]
pub fn now() -> DateTime<Utc> {
let now =
SystemTime::now().duration_since(UNIX_EPOCH).expect("system time before Unix epoch");
let naive =
NaiveDateTime::from_timestamp(now.as_secs() as i64, now.subsec_nanos()).unwrap();
Utc.from_utc_datetime(&naive)
SystemTime::now().try_into().expect(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll want to make this fallible, but can leave this for a future PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue #1440.

"system clock is set to a time extremely far into the past or future; cannot convert",
)
}

/// Returns a `DateTime` which corresponds to the current date and time.
Expand Down
Loading