Skip to content

Commit

Permalink
Rollup merge of rust-lang#59374 - faern:simplify-checked-duration-sin…
Browse files Browse the repository at this point in the history
…ce, r=shepmaster

Simplify checked_duration_since

This follows the same design as we updated to in rust-lang#56490. Internally, all the system specific time implementations are checked, no panics. Then the panicking publicly exported API can just call the checked version of itself and make do with a single panic (`expect`) at the top.

Since the internal sys implementations are now checked, this gets rid of the extra `if self >= &earlier` check in `checked_duration_since`. Except likely making the generated machine code simpler, it also reduces the algorithm from "Check panic condition -> call possibly panicking method" to just "call non panicking method".

Added two test cases:
* Edge case: Make sure `checked_duration_since` on two equal `Instant`s produce a zero duration, not a `None`.
* Most common/intended usage: Make sure `later.checked_duration_since(earlier)`, returns an expected value.
  • Loading branch information
Centril authored Mar 26, 2019
2 parents cb15da7 + 1ccad16 commit 8aa161b
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 37 deletions.
8 changes: 3 additions & 5 deletions src/libstd/sys/cloudabi/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,9 @@ impl Instant {
Instant { t: 0 }
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
let diff = self.t
.checked_sub(other.t)
.expect("second instant is later than self");
Duration::new(diff / NSEC_PER_SEC, (diff % NSEC_PER_SEC) as u32)
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
let diff = self.t.checked_sub(other.t)?;
Some(Duration::new(diff / NSEC_PER_SEC, (diff % NSEC_PER_SEC) as u32))
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down
6 changes: 2 additions & 4 deletions src/libstd/sys/redox/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,8 @@ impl Instant {
false
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
self.t.sub_timespec(&other.t).unwrap_or_else(|_| {
panic!("specified instant was later than self")
})
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.t.sub_timespec(&other.t).ok()
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/sgx/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ impl Instant {
Instant(usercalls::insecure_time())
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
self.0 - other.0
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.0.checked_sub(other.0)
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down
13 changes: 5 additions & 8 deletions src/libstd/sys/unix/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,11 @@ mod inner {
true
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
let diff = self.t.checked_sub(other.t)?;
let info = info();
let diff = self.t.checked_sub(other.t)
.expect("second instant is later than self");
let nanos = mul_div_u64(diff, info.numer as u64, info.denom as u64);
Duration::new(nanos / NSEC_PER_SEC, (nanos % NSEC_PER_SEC) as u32)
Some(Duration::new(nanos / NSEC_PER_SEC, (nanos % NSEC_PER_SEC) as u32))
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down Expand Up @@ -285,10 +284,8 @@ mod inner {
false // last clause, used so `||` is always trailing above
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
self.t.sub_timespec(&other.t).unwrap_or_else(|_| {
panic!("specified instant was later than self")
})
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.t.sub_timespec(&other.t).ok()
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/wasm/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ impl Instant {
false
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
self.0 - other.0
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
self.0.checked_sub(other.0)
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down
8 changes: 4 additions & 4 deletions src/libstd/sys/windows/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,17 @@ impl Instant {
Instant { t: Duration::from_secs(0) }
}

pub fn sub_instant(&self, other: &Instant) -> Duration {
pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {
// On windows there's a threshold below which we consider two timestamps
// equivalent due to measurement error. For more details + doc link,
// check the docs on epsilon.
let epsilon =
perf_counter::PerformanceCounterInstant::epsilon();
if other.t > self.t && other.t - self.t <= epsilon {
return Duration::new(0, 0)
Some(Duration::new(0, 0))
} else {
self.t.checked_sub(other.t)
}
self.t.checked_sub(other.t)
.expect("specified instant was later than self")
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Instant> {
Expand Down
23 changes: 11 additions & 12 deletions src/libstd/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ impl Instant {
/// ```
#[stable(feature = "time2", since = "1.8.0")]
pub fn duration_since(&self, earlier: Instant) -> Duration {
self.0.sub_instant(&earlier.0)
self.0.checked_sub_instant(&earlier.0).expect("supplied instant is later than self")
}

/// Returns the amount of time elapsed from another instant to this one,
Expand All @@ -233,11 +233,7 @@ impl Instant {
/// ```
#[unstable(feature = "checked_duration_since", issue = "58402")]
pub fn checked_duration_since(&self, earlier: Instant) -> Option<Duration> {
if self >= &earlier {
Some(self.0.sub_instant(&earlier.0))
} else {
None
}
self.0.checked_sub_instant(&earlier.0)
}

/// Returns the amount of time elapsed from another instant to this one,
Expand Down Expand Up @@ -664,20 +660,23 @@ mod tests {

#[test]
#[should_panic]
fn instant_duration_panic() {
fn instant_duration_since_panic() {
let a = Instant::now();
(a - Duration::new(1, 0)).duration_since(a);
}

#[test]
fn checked_instant_duration_nopanic() {
let a = Instant::now();
let ret = (a - Duration::new(1, 0)).checked_duration_since(a);
assert_eq!(ret, None);
fn instant_checked_duration_since_nopanic() {
let now = Instant::now();
let earlier = now - Duration::new(1, 0);
let later = now + Duration::new(1, 0);
assert_eq!(earlier.checked_duration_since(now), None);
assert_eq!(later.checked_duration_since(now), Some(Duration::new(1, 0)));
assert_eq!(now.checked_duration_since(now), Some(Duration::new(0, 0)));
}

#[test]
fn saturating_instant_duration_nopanic() {
fn instant_saturating_duration_since_nopanic() {
let a = Instant::now();
let ret = (a - Duration::new(1, 0)).saturating_duration_since(a);
assert_eq!(ret, Duration::new(0,0));
Expand Down

0 comments on commit 8aa161b

Please sign in to comment.