Checked arithmetic is wrong for Durations smaller than 100ns on Windows

[cvengler]

I tried this code:

let system_time: SystemTime = ...;

// Add 1ns.
system_time.checked_add(Duration::new(0, 1)).unwrap();

I expected to see this happen: The unwrap to fail because SystemTime on Windows only has an accuracy of 100ns.

Instead, this happened: The addition "works" by returning some, although the value is unchanged from the original one, meaning an addition of a Duration < 100ns is equivalent of adding Duration::ZERO.

[cvengler]

cc @ChrisDenton as discussed in #148825

[ds84182]

What part of the checked_add documentation implies it should return an error in this case? Arithmetic overflow did not occur.

[cvengler]

What part of the checked_add documentation implies it should return an error in this case? Arithmetic overflow did not occur.

The doc says:

Returns Some(t) where t is the time self + duration if t can be represented as SystemTime (which means it’s inside the bounds of the underlying data structure), None otherwise.

Clearly, self + duration cannot be represented if the precision of the operating system is greater than the duration itself.

[workingjubilee]

There is an argument to be made that the operating system does represent these times, it just happens to represent every 100ns the same way, as opposed to ones that are out-of-bounds.

However, I am somewhat skeptical that we also have an implementation that is consistent with that perspective.

The ambiguity should be clarified.

[QuineDot]

Given the API (and other methods of the same name on e.g primitives), if a checked addition of 1ns failed, I would expect a checked addition of any greater duration to also fail.

If I add 140ns and get back something with a delta of 100ns, is that also bad?

Perhaps there could be a new method with a more refined return type.

(Rounding up was a possibility mentioned in the linked PR. Given that adding 0 is already not an error, I don't know that rounding up is any better than rounding to the nearest fencepost or rounding down, though.)

[ds84182]

Maybe the precision of SystemTime should be exposed as a constant, along with documentation improvements for platform dependent precision.

[workingjubilee]

I feel dubious about going to significant effort to make working with SystemTime extremely portable: there is not even a guarantee that the precision of SystemTime is a constant. Instead, we should explicitly define the model we're using for interactions like this, and only then consider further API expansions. Preferably not ones that can be easily undermined by an architecture shipping a different clock.