korken89 / fugit

`fugit` provides a comprehensive library of `Duration` and `Instant` for the handling of time in embedded systems, doing all it can at compile time.
Apache License 2.0
56 stars 16 forks source link

`Instant::checked_add_duration` claims to check for overflow, doesn't #38

Open Dridus opened 2 years ago

Dridus commented 2 years ago

use of wrapping_add where the bases are equal, here: https://github.com/korken89/fugit/blob/master/src/instant.rs#L171 and similarly if bases are different, here: https://github.com/korken89/fugit/blob/master/src/instant.rs#L181

claim that it checks for overflow, here: https://github.com/korken89/fugit/blob/master/src/instant.rs#L156

evcxr transcript showing that it wraps:

>> :dep fugit = "0.3.6"
>> fugit::Instant::<u32, 1, 1000>::from_ticks(0xffff_ffff) + fugit::Duration::<u32, 1, 1000>::from_ticks(10)
Instant { ticks: 9 }
korken89 commented 2 years ago

Hi, this is intended behavior. fugit's Instant represents a wrap-able time of MAX/2 -1 span (think iXX semantics). The overflow check that it does is checking whether the change of base in checked_add_duration would cause overflows.

Dridus commented 2 years ago

it was pretty surprising to me that the implementation of Add wrapped, as usually I have to reach for wrapping_add or similar to get that effect rather than panics, but Add doesn't make that behavior explicit in its docs, it's just the default it seems for most numeric types.

that said, if the current behavior is intended, may I suggest expanding the doc comment to make it clear what kind of overflow is being checked for?