search

time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.13k stars 281 forks source link

`subsec_{milli,micro,nano}seconds` may have over-inclusive documented ranges #669

Closed samcowger closed 8 months ago

samcowger commented 8 months ago

Documentation of these functions promises that they will produce values in the ranges -1_000..1_000, -1_000_000..1_000_000, and -1_000_000_000..1_000_000_000, respectively, but I don't see how they could produce the lowest value in their stated bounds.

In terms of code, I'll use subsec_nanoseconds as an example:

https://github.com/time-rs/time/blob/7a6a800843e999807ddc4a317e95705f4ed82da5/time/src/duration.rs#L912-L923

self.nanoseconds is a Nanoseconds, which is a RangedI32 - in particular, a RangedI32<{ -(Nanosecond::per(Second) as i32 - 1) }, { Nanosecond::per(Second) as i32 - 1 }>. per seems to be defined appropriately, so this gives us RangedI32<-999_999_999, 999_999_999>. Given that a RangedI32<MIN, MAX> promises to contain "[a]n i32 that is known to be in the range MIN..=MAX", it's impossible for -1_000_000_000 to inhabit Nanoseconds, and so to be produced by subsec_nanoseconds.

I believe the same logic can be extended to subsec_microseconds and subsec_milliseconds, since they defer to subsec_nanoseconds.

jhpratt commented 8 months ago

Yeah, this is a mistake. It should be changed to -999_999_999..=999_999_999, using the same number for both bounds for symmetry. Likewise for other similar values.