libtime: Use Duration in Timespec arithmetic. #16650
Conversation
|
This looks great, thanks! It may take #16626 more time to land than this, so perhaps this could be rebased on master and this could land first? |
ruuda
force-pushed
the
timespec-arithmetic
branch
from
7df21fa to
913f0ff
Compare
|
This works with the current |
| // more than one second left, which fits in i64 in i32. | ||
| // FIXME: Construct duration from i64 to avoid overflow. | ||
| let d_nsec = (other - Duration::seconds(d_sec as i32)) | ||
| .num_nanoseconds().unwrap() as i32; |
There was a problem hiding this comment.
Could you align this .num_nanoseconds() to the right to not be left-aligned?
There was a problem hiding this comment.
I wondered about that, what the best style is (I could not find anything in the style guide). Do you mean aligning it with the opening parenthesis?
ruuda
force-pushed
the
timespec-arithmetic
branch
from
913f0ff to
cb8389d
Compare
|
Now that #16626 has been approve, this should probably wait to get merged, other than that this looks good to me! |
|
Now that #16626 has merged, should this be rebased to remove the FIXMEs? |
|
Yes, I’m running |
ruuda
force-pushed
the
timespec-arithmetic
branch
from
cb8389d to
d20de6d
Compare
This changes the `Add` and `Sub` implementations for `Timespec` introduced in #16573 to use `Duration` as the time span type instead of `Timespec` itself, as [suggested](#16573 (comment)) by @sfackler. This depends on #16626, because is uses `Duration::seconds(i64)`, whereas currently `Duration::seconds` takes an `i32`.
Fix: Fix metrics CI failing
This changes the
AddandSubimplementations forTimespecintroduced in #16573 to useDurationas the time span type instead ofTimespecitself, as suggested by @sfackler.This depends on #16626, because is uses
Duration::seconds(i64), whereas currentlyDuration::secondstakes ani32.