Skip to content

Conversation

@joboet
Copy link
Member

@joboet joboet commented Jan 15, 2026

This is probably the most complex step of #117276 so far. Unfortunately, quite some of the internal time logic defined in the PAL is also used in other places like the filesystem code, so this isn't just a series of simple moves. I've left all that logic inside the PAL and only moved the actual SystemTime/Instant implementations.

While there are no functional changes, this PR also contains some slight code cleanups on Windows and Hermit, these are explained in the relevant commits.

For additional details see the individual commits, I've tried to make the messages as helpful as possible about what's going on.

@rustbot rustbot added the O-hermit Operating System: Hermit label Jan 15, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 15, 2026

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

@rustbot rustbot added O-itron Operating System: ITRON O-SGX Target: SGX O-solid Operating System: SOLID O-unix Operating system: Unix-like O-wasm Target: WASM (WebAssembly), http://webassembly.org/ 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 Jan 15, 2026
@rust-log-analyzer

This comment has been minimized.

@joboet
Copy link
Member Author

joboet commented Jan 15, 2026

I've changed my mind, I'll just move these in their entirety.

@joboet joboet closed this Jan 15, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 15, 2026
@joboet
Copy link
Member Author

joboet commented Jan 16, 2026

Ok, no, let's actually do it this way first...

@joboet joboet reopened this Jan 16, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 16, 2026
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2026

The Miri subtree was changed

cc @rust-lang/miri

let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH");
let SECONDS_TO_UNIX_EPOCH = INTERVALS_TO_UNIX_EPOCH / INTERVALS_PER_SEC;
// The amount of seconds between 1601/1/1 and 1970/1/1.
const SECONDS_TO_UNIX_EPOCH: u64 = 11_644_473_600;
Copy link
Member

@RalfJung RalfJung Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't use values from std here any more, the comment should have links that show why this is the right value.

(Same for the other case further down the file.)

@rust-bors

This comment has been minimized.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks pretty reasonable so r=me after the change Ralf requested and a rebase. The detailed commit messages helped quite a bit, thanks for that.

FYI the commit message bodies aren't wrapped, may be worth fixing before merging.

View changes since this review

@tgross35
Copy link
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 24, 2026
Let's start with the easy ones:
* Motor just reexports its platform library
* The SGX code is just a trivial move
* Trusty, WASM and ZKVM are unsupported, this is very trivial. And we can get
  rid of some `#[path = ...]`s, yay!
Now that the `unsupported` module exists, we can use it for VEX. VEX actually
supports `Instant` though, so the implementation-select needs to combine that
with the `unsupported` module.
On SOLID, the conversion functions are also used to implement helpers for
timeout conversion, so these stay in the PAL. The `Instant` (µITRON) and
`SystemTime` (SOLID-specific) implementations are merged into one. While it was
nice to have the µITRON parts in a separate module, there really isn't a need
for this currently, as there is no other µITRON target. Let's not worry about
this until such a target gets added...

Note that I've extracted the `get_tim` call from `Instant` into a wrapper
function in the PAL to avoid the need to make the inner `Instant` field public
for use in the PAL.
Next up: UEFI. Unfortunately the time conversion internals are also required by
the filesystem code, so I've left them in the PAL. The `Instant` internals
however are only used for the `Instant` implementation, so I've moved them to
`sys` (for now).
Windows has a similar problem as UEFI: some internals are also used for
implementing other things. Thus I've left everything except for the actual
`Instant` and `SystemTime` implementations inside the PAL.

I've also taken the liberty of improving the code clarity a bit: there were a
number of manual `SystemTime` conversions (including one that masked an `i64`
to 32-bits before casting to `u32`, yikes) that now just call `from_intervals`.
Also, defining a `PerformanceCounterInstant` just to convert it to a regular
`Instant` immediately doesn't really make sense to me. I've thus changed the
`perf_counter` module to contain only free functions that wrap system
functionality. The actual conversion now happens in `Instant::now`.
@rustbot
Copy link
Collaborator

rustbot commented Jan 27, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer

This comment has been minimized.

Now for UNIX: `Timespec` is also used for things like futex or `Condvar`
timeouts, so it stays in the PAL along with some related definitions.
Everything else is `SystemTime`- and `Instant`-specific, and is thus moved to
`sys::time`.
WASI and TEEOS share the UNIX implementation. Since `Timespec` lives in the
PAL, this means that the `#[path]` imports of `unix/time.rs` still remain for
these two, unfortunately. Maybe we'll solve that in the future by always
including the UNIX PAL for these remotely UNIXy platforms. But that's a story
for another time...
Last on the list: Hermit. Since I anticipate that Hermit will share the UNIX
implementation in the future, I've left `Timespec` in the PAL to maintain a
similar structure. Also, I noticed that some public `Instant` methods were
redefined on the internal `Instant`. But the networking code can just use the
public `Instant`, so I've removed them.
The `std` paths are subject to change, but the values themselves will never
change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-hermit Operating System: Hermit O-itron Operating System: ITRON O-SGX Target: SGX O-solid Operating System: SOLID O-unix Operating system: Unix-like O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

5 participants