Skip to content

Conversation

@joboet
Copy link
Member

@joboet joboet commented Sep 26, 2025

The docs for Instant and SystemTime mention that

An Instant is a wrapper around system-specific types and it may behave differently depending on the underlying operating system.

and

[checked_add 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.

In my understanding, this implies that, should calling checked_add or checked_sub on an Instant/SystemTime succeed, it should always be possible to pass the resulting time value back to the operating system.

This expectation is currently violated on Windows and a number of other non-UNIX platforms (SGX, VEX, WASIp1, WASIp2, Xous) that immediately convert time values retrieved from the OS into Durations and use those Durations to perform arithmetic. Thus it becomes possible to create Instants/SystemTimes that are not representable by the OS, which becomes problematic when considering functions such as File::set_times or sleep_until.

This PR thus changes the Instant and SystemTime structures on the aforementioned platforms to store the data structure returned by their OS and changes the arithmetic functions to act on those values. In particular, this results

  • in a loss of precision on VEX and Xous, as their clock only has micro-/millisecond precision (hence .checked_add(Duration::from_nanos(1)) might return the same time value)
  • in a reduction of the representable range on all platforms (though the representable range is still larger than 100 years even on Windows, which is what is guaranteed by our documentation)
  • in slightly changed performance characteristics on Windows, as the ticks of the performance counter will be converted only when performing arithmetic instead of when creating an Instant (this might actually be beneficial as Instant::elapsed will now only do one conversion instead of two)

I've left UEFI's Instant untouched in this PR, even though it is suffering from the same issue, as the fix is quite non-trivial there.

After this PR, the only platform where File::set_times can fail because of an invalid SystemTime is on 32-bit UNIXes that lack 64-bit time support. Changing them would conflict with the guarantee that 100 year additions are portable, however.

@rustbot rustbot added O-SGX Target: SGX O-wasi Operating system: Wasi, Webassembly System Interface O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 26, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@joboet joboet added the I-libs-nominated Nominated for discussion during a libs team meeting. label Sep 26, 2025
@joboet
Copy link
Member Author

joboet commented Oct 2, 2025

This was discussed in the last T-libs meeting, the consensus being that T-libs would like to keep the nanosecond-precision arithmetic of the current version. Hence I'm closing this in favour of a PR that improves the documentation – in particular, it should not create the false expectation that Instant or SystemTime can be losslessly converted back to the platform time representation.

@joboet joboet closed this Oct 2, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I-libs-nominated Nominated for discussion during a libs team meeting. O-SGX Target: SGX O-wasi Operating system: Wasi, Webassembly System Interface O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants