Saturating add/sub for durations, to avoid overflow when summing up many small duration in a accumulator

[PTaylor-us]

saturating add/sub can be also useful for durations, to avoid overflow when summing up many small duration in a accumulator...

Originally posted by @tib888 in https://github.com/PTaylor-FluenTech/embedded-time/issues/5#issuecomment-629613452

[PTaylor-us]

What failure mode is optimal? A panic or an inaccurate duration? My current thought is a panic is better. Otherwise, you may have some timing that is off, but you're not alerted to it.

[tib888]

Just implement the Saturating trait on Duration, so one can use it if it suits his goals. https://docs.rs/num-traits/0.2.2/num_traits/ops/saturating/trait.Saturating.html

It is implemented also on the basic integer types, so you can use them during the implementation instead of '+': https://doc.rust-lang.org/std/primitive.u32.html#method.saturating_add

[tib888]

You can also implement the CheckedXXX operations, which give chance for explicit error handling: https://docs.rs/num-traits/0.2.2/num_traits/ops/checked/trait.CheckedAdd.html

But wrapping behavior for example makes no sense on Duration, but it does on Instant as you already know. https://docs.rs/num-traits/0.2.2/num_traits/ops/wrapping/trait.WrappingAdd.html

[PTaylor-us]

Just implement the Saturating trait on Duration, so one can use it if it suits his goals.

That makes a lot of sense.

You can also implement the CheckedXXX operations, which give chance for explicit error handling.

I actually went through all the arithmetic operations (other than the wrapping ops) and replaced them with checked ops propagating the Option along with any other conversion/casting errors as Options.

One issue that's been difficult for me to get used to is coming from C++ where there is implicit integer promotion in arithmetic operations.

[devjgm]

I think saturating arithmetic should be the default behavior for duration and instant types. Panicking in release builds for "Duration::MAX + 1" is harsh, especially when they both effectively mean "infinite future". Wrapping is almost never what one wants. Checked arithmetic might be what one wants, but in those cases, explicitly calling a checked_ method seems like the right behavior.

Have you considered making saturating arithmetic the default? FWIW, some precedent for this behavior is: https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h