nyx-space / hifitime

A high fidelity time management library in Rust
Mozilla Public License 2.0
338 stars 19 forks source link

Conversions to/from `std::time::Duration` are lossy and easy to misuse #337

Open max-heller opened 1 week ago

max-heller commented 1 week ago

The implementations of From<std::time::Duration> for Duration and From<Duration> for std::time::Duration are lossy because

https://github.com/nyx-space/hifitime/blob/cdcbc9f34b5a243eafcd7bbdebc10523b743fb07/src/duration/std.rs#L17-L45

The docs for From suggest that implementations should be lossless and I've run into difficult-to-track down bugs because of the lossiness and ease of calling From/Into. I see a couple of options that would make more sense:

  1. Keep From implementations with saturating behavior, but remove the lossiness caused by the float conversion. This is still confusing, but much less so than running into float precision losses.
  2. Replace the From implementations with TryFrom implementations to account for possible overflow, and also use integer conversions instead of float conversions
ChristopherRabotin commented 1 week ago

Hi Max,

You raise a good point: a number of the initializers in hifitime are marked as error-less because the representation is manually minimized or maxed out.

I've just released version 4.0.0. Ideas for version 5 are already being worked on. Switching from the error-free From trait to the TryFrom trait should be one of the improvements for version 5.

In the meantime, version 4.1 could implement the first solution you propose by getting rid of the integer conversions.