Conversions between Timestamp and std::time::SystemTime

[dcormier]

Closes #834.

[KodrAus]

I think we shouldn't need to cfg out these impls on wasm32-unknown-unknown. The value of UNIX_EPOCH on wasm32-unknown-unknown is meaningless, but you also can't construct a SystemTime anyways, so I think leaving these impls is ok, and will be less likely to result in impossible build failures for anyone targeting multiple platforms.

[dcormier]

I've updated the PR with those adjustments.

[KodrAus]

I think the js feature is likely also unnecessary here. Since it's a no-op outside of wasm32-unknown-unknown, its presence shouldn't have any impact on std-code.

[dcormier]

Done.

[KodrAus]

Ah, mismatched_lifetime_syntaxes strikes again. I'll submit a PR to fix all that up

[KodrAus]

The failure appears to be here:

 failures:
  
  ---- timestamp::test_conversion::from_system_time stdout ----
  
  thread 'timestamp::test_conversion::from_system_time' panicked at src\timestamp.rs:1251:9:
  assertion `left == right` failed
    left: Timestamp { seconds: 1501520400, subsec_nanos: 1, counter: 0, usable_counter_bits: 0 }
   right: Timestamp { seconds: 1501520400, subsec_nanos: 0, counter: 0, usable_counter_bits: 0 }
  
  ---- timestamp::test_conversion::from_system_time_before_epoch stdout ----
  
  thread 'timestamp::test_conversion::from_system_time_before_epoch' panicked at src\timestamp.rs:1260:43:
  called `Result::unwrap_err()` on an `Ok` value: Timestamp { seconds: 0, subsec_nanos: 0, counter: 0, usable_counter_bits: 0 }

[KodrAus]

This could be Window's 100ns system time granularity? We could try updating the test case to use 100 nanos instead of just 1.

[KodrAus]

The WASI failure appears to be in the from_system_time_before_epoch test case:

          12:  0x8cded - uuid-6d0d58a3e0a20e3c.wasm!core::option::unwrap_failed::hf7f62ca5847d7f2d
          13:  0x26bd3 - uuid-6d0d58a3e0a20e3c.wasm!uuid::timestamp::test_conversion::from_system_time_before_epoch::h038bbadfcb94bfb7
          14:  0x2a43f - uuid-6d0d58a3e0a20e3c.wasm!uuid::timestamp::test_conversion::from_system_time_before_epoch::{{closure}}::h42d9040da4fb3cc8

[dcormier]

This could be Window's 100ns system time granularity? We could try updating the test case to use 100 nanos instead of just 1.

Ah, that may be. I've updated the test cases to use 1µs resolution.

[dcormier]

The WASI failure is curious. I'd like to see if that test output changes with that latest commit (which should at least resolve the Windows issue). I'll also see about running that test locally so I can iterate on it without it having to go through CI, if needed.

I wonder if UNIX_EPOCH has a different value when compiled for WASI?

---

Edit: I'm able to reproduce it locally. This returns None on WASI:

SystemTime::UNIX_EPOCH.checked_sub(Duration::from_nanos(1_000))

I'll poke around some more because I'm not familiar with this environment, but perhaps this test should be excluded on that platform?

---

Edit 2: A SystemTime on WASI simply holds a Duration, which stores unsigned values. So, a WASI SystemTime currently cannot be before UNIX_EPOCH.

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
pub struct SystemTime(Duration);

pub const UNIX_EPOCH: SystemTime = SystemTime(Duration::from_secs(0));

#[stable(feature = "duration", since = "1.3.0")]
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
#[rustc_diagnostic_item = "Duration"]
pub struct Duration {
    secs: u64,
    nanos: Nanoseconds, // Always 0 <= nanos < NANOS_PER_SEC
}

define_valid_range_type! {
    pub struct Nanoseconds(u32 as u32 in 0..=999_999_999);
}

My proposed solution (and the one I've updated this PR with) is to have that test gracefully exit when the platform running it does not support values < UNIX_EPOCH. Thoughts?

[KodrAus]

Thanks for working through this @dcormier!

This looks good to me 🙇

[dcormier]

Thank you! Much appreciated.