Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

I have a good amount of familiarity with these targets (and can test on a fairly wide range of them), so I'd like to take this. I'm a little hesitant about this change, but the current situation is admittedly fairly undesirable. I'll probably get to the review this weekend.

r? thomcc

This doesn't compile on aarch64-apple-darwin.

error[E0308]: mismatched types
   --> library/std/src/sys/unix/time.rs:293:39
    |
293 |             const clock_id: clock_t = libc::CLOCK_UPTIME_RAW;
    |                                       ^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `u32`

error[E0308]: mismatched types
   --> library/std/src/sys/unix/time.rs:323:43
    |
323 |             SystemTime { t: Timespec::now(libc::CLOCK_REALTIME) }
    |                             ------------- ^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `u32`
    |                             |
    |                             arguments to this function are incorrect
    |
note: associated function defined here
   --> library/std/src/sys/unix/time.rs:333:16
    |
333 |         pub fn now(clock: clock_t) -> Timespec {
    |                ^^^ --------------
help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit
    |
323 |             SystemTime { t: Timespec::now(libc::CLOCK_REALTIME.try_into().unwrap()) }
    |                                                               ++++++++++++++++++++

error[E0308]: mismatched types
    --> library/std/src/sys/unix/time.rs:362:46
     |
362  |             cvt(unsafe { libc::clock_gettime(clock, t.as_mut_ptr()) }).unwrap();
     |                          ------------------- ^^^^^ expected `u32`, found `i32`
     |                          |
     |                          arguments to this function are incorrect
     |
note: function defined here
    --> /Users/thom/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.135/src/unix/bsd/apple/mod.rs:5092:12
     |
5092 |     pub fn clock_gettime(clk_id: ::clockid_t, tp: *mut ::timespec) -> ::c_int;
     |            ^^^^^^^^^^^^^
help: you can convert an `i32` to a `u32` and panic if the converted value doesn't fit
     |
362  |             cvt(unsafe { libc::clock_gettime(clock.try_into().unwrap(), t.as_mut_ptr()) }).unwrap();
     |                                                   ++++++++++++++++++++

@rustbot author

I think the problem is the type alias used here.

Is there any reason to even have these? libc seems to have a clockid_t alias for every platform already.

If it's in libc on every platform there's no reason to have it in std::sys

Hmm, it seems that miri doesn't emulate clock_gettime on macOS. @RalfJung, any opinions as to how you'd like us to handle this?

This looks good to me. Seems to work on M1 too. libc::CLOCK_UPTIME_RAW is the same as mach_absolute_time effectively, which we already use, so I don't see a need to deliberate over this like we have on some other platforms. Thanks.

@bors r+

(Also, I like your username and avatar)

📌 Commit 015ab65 has been approved by thomcc

It is now in the queue for this repository.

Ah, wait, this is failing. I forgot that we need to resolve the situation with miri.

@bors r-

Probably the best solution is to continue using the x86_64 code on aarch64 when under miri). Pretty soon we should be able to globally improve the handling of this (assuming the min macOS version bump goes through).

Is the plan to use clock_gettime on all macos targets?

Plausibly. I'd probably prefer using clock_gettime_nsec_np given that what we actually want is nanoseconds, and apple's docs recommend that you prefer it if possible. But clock_gettime is an easier migration path.

The 'np' makes me thing that will require a dedicated implementation. ;) But it's fine, these clock APIs are usually not a lot of work. (Unlike synchronization APIs...)

The 'np' makes me thing that will require a dedicated implementation

Yes, most likely. But yeah, I don't expect it to be very complex.

LGTM

@bors r+

📌 Commit f4f5159 has been approved by thomcc

It is now in the queue for this repository.