Avoid SeqCst or static mut in mach_timebase_info and QueryPerformanceFrequency caches
This patch went through a couple iterations but the end result is replacing a pattern where an `AtomicUsize` (updated with many SeqCst ops) guards a `static mut` with a single `AtomicU64` that is known to use 0 as a value indicating that it is not initialized.
The code in both places exists to cache values used in the conversion of Instants to Durations on macOS, iOS, and Windows.
I have no numbers to prove that this improves performance (It seems a little futile to benchmark something like this), but it's much simpler, safer, and in practice we'd expect it to be faster everywhere where Relaxed operations on AtomicU64 are cheaper than SeqCst operations on AtomicUsize, which is a lot of places.
Anyway, it also removes a bunch of unsafe code and greatly simplifies the logic, so IMO that alone would be worth it unless it was a regression.
If you want to take a look at the assembly output though, see https://godbolt.org/z/rbr6vn for x86_64, https://godbolt.org/z/cqcbqv for aarch64 (Note that this just the output of the mac side, but i'd expect the windows part to be the same and don't feel like doing another godbolt for it). There are several versions of this function in the godbolt:
- `info_new`: version in the current patch
- `info_less_new`: version in initial PR
- `info_original`: version currently in the tree
- `info_orig_but_better_orderings`: a version that just tries to change the original code's orderings from SeqCst to the (probably) minimal orderings required for soundness/correctness.
The biggest concern I have here is if we can use AtomicU64, or if there are targets that dont have it that this code supports. AFAICT: no. (If that changes in the future, it's easy enough to do something different for them)
r? `@Amanieu` because he caught a couple issues last time I tried to do a patch reducing orderings 😅
---
<details>
<summary>I rewrote this whole message so the original is inside here</summary>
I happened to notice the code we use for caching the result of mach_timebase_info uses SeqCst exclusively.
However, thinking a little more, it's actually pretty easy to avoid the static mut by packing the timebase info into an AtomicU64.
This entirely avoids needing to do the compare_exchange. The AtomicU64 can be read/written using Relaxed ops, which on current macos/ios platforms (x86_64/aarch64) have no overhead compared to direct loads/stores. This simplifies the code and makes it a lot safer too.
I have no numbers to prove that this improves performance (It seems a little futile to benchmark something like this), although it should do that on both targets it applies to.
That said, it also removes a bunch of unsafe code and simplifies the logic (arguably at least — there are only two states now, initialized or not), so I think it's a net win even without concrete numbers.
If you want to take a look at the assembly output though, see below. It has the new version, the original, and a version of the original with lower Orderings (which is still worse than the version in this PR)
- godbolt.org/z/obfqf9 x86_64-apple-darwin
- godbolt.org/z/Wz5cWc aarch64-unknown-linux-gnu (godbolt can't do aarch64-apple-ios but that doesn't matter here)
A different (and more efficient) option than this would be to just use the AtomicU64 and use the knowledge that after initialization the denominator should be nonzero... That felt like it's relying on too many things I'm not confident in, so I didn't want to do that.
</details>
Allow generic parameters in intra-doc links
Fixes#62834.
---
The contents of the generics will be mostly ignored (except for warning
if fully-qualified syntax is used, which is currently unsupported in
intra-doc links - see issue #74563).
* Allow links like `Vec<T>`, `Result<T, E>`, and `Option<Box<T>>`
* Allow links like `Vec::<T>::new()`
* Warn on
* Unbalanced angle brackets (e.g. `Vec<T` or `Vec<T>>`)
* Missing type to apply generics to (`<T>` or `<Box<T>>`)
* Use of fully-qualified syntax (`<Vec as IntoIterator>::into_iter`)
* Invalid path separator (`Vec:<T>:new`)
* Too many angle brackets (`Vec<<T>>`)
* Empty angle brackets (`Vec<>`)
Note that this implementation *does* allow some constructs that aren't
valid in the actual Rust syntax, for example `Box::<T>new()`. That may
not be supported in rustdoc in the future; it is an implementation
detail.
doc: disambiguate stat in MetadataExt::as_raw_stat
A few architectures in `os::linux::raw` import `libc::stat`, rather than
defining that type directly. However, that also imports the _function_
called `stat`, which makes this doc link ambiguous:
error: `crate::os::linux::raw::stat` is both a struct and a function
--> library/std/src/os/linux/fs.rs:21:19
|
21 | /// [`stat`]: crate::os::linux::raw::stat
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ambiguous link
|
= note: `-D broken-intra-doc-links` implied by `-D warnings`
help: to link to the struct, prefix with the item type
|
21 | /// [`stat`]: struct@crate::os::linux::raw::stat
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to link to the function, add parentheses
|
21 | /// [`stat`]: crate::os::linux::raw::stat()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We want the `struct`, so it's now prefixed accordingly.
fix __rust_alloc_error_handler comment
`__rust_alloc_error_handler` was added in the same `extern` block as the allocator functions, but the comment there was not actually correct for `__rust_alloc_error_handler`. So move it down to the rest of the default allocator handling with a fixed comment. At least the comment reflects my understanding of what happens, please check carefully. :)
r? @Amanieu Cc @haraldh
Link to documentation-specific guidelines.
Changed contribution information URL because it's not obvious how to get from the current URL to the documentation-specific content.
The current URL points to this "Getting Started" page, which contains nothing specific about documentation[*] and instead launches into how to *build* `rustc` which is not a strict prerequisite for contributing documentation fixes:
* https://rustc-dev-guide.rust-lang.org/getting-started.html
[*] The most specific content is a "Writing documentation" bullet point which is not itself a link to anything (I guess a patch for that might be helpful too).
### Why?
Making this change will make it easier for people who wish to make small "drive by" documentation fixes (and read contribution guidelines ;) ) which I find are often how I start contributing to a project. (Exhibit A: https://github.com/rust-lang/rust/pull/77050 :) )
### Background
My impression is the change of content linked is an unintentional change due to a couple of other changes:
* Originally, the link pointed to `contributing.md` which started with a "table of contents" linking to each section. But the content in `contributing.md` was removed and replaced with a link to the "Getting Started" section here:
* 3f6928f1f6 (diff-6a3371457528722a734f3c51d9238c13L1)
But the changed link doesn't actually point to the equivalent content, which is now located here:
* https://rustc-dev-guide.rust-lang.org/contributing.html
(If the "Guide to Rustc Development" is now considered the canonical location of "How to Contribute" content it might be a good idea to merge some of the "Contributing" Introduction section into the "Getting Started" section.)
* This was then compounded by changing the link from `contributing.md` to `contributing.html` here:
* https://github.com/rust-lang/rust/pull/74037/files#diff-242481015141f373dcb178e93cffa850L88
In order to even find the new location of the previous `contributing.md` content I ended up needing to do a GitHub search of the `rust-lang` org for the phrase "Documentation improvements are very welcome". :D
Fix error checking in posix_spawn implementation of Command
* Check for errors returned from posix_spawn*_init functions
* Check for non-zero return value from posix_spawn functions
A few architectures in `os::linux::raw` import `libc::stat`, rather than
defining that type directly. However, that also imports the _function_
called `stat`, which makes this doc link ambiguous:
error: `crate::os::linux::raw::stat` is both a struct and a function
--> library/std/src/os/linux/fs.rs:21:19
|
21 | /// [`stat`]: crate::os::linux::raw::stat
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ambiguous link
|
= note: `-D broken-intra-doc-links` implied by `-D warnings`
help: to link to the struct, prefix with the item type
|
21 | /// [`stat`]: struct@crate::os::linux::raw::stat
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to link to the function, add parentheses
|
21 | /// [`stat`]: crate::os::linux::raw::stat()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We want the `struct`, so it's now prefixed accordingly.
`DirEntry` contains a `ReadDir` handle, which used to just be a wrapper
on `Arc<InnerReadDir>`. Commit af75314ecd added `end_of_stream: bool`
which is not needed by `DirEntry`, but adds 8 bytes after padding. We
can let `DirEntry` have an `Arc<InnerReadDir>` directly to avoid that.
The posix_spawnattr_init & posix_spawn_file_actions_init might fail,
but their return code is not checked.
Check for non-zero return code and destroy only succesfully initialized
objects.
The cvt function compares the argument with -1 and when equal returns a new
io::Error constructed from errno. It is used together posix_spawn_* functions.
This is incorrect. Those functions do not set errno. Instead they return
non-zero error code directly.
Check for non-zero return code and use it to construct a new io::Error.
(docs): make mutex error comment consistent with codebase
Although exceptionally minor, I found this stands out from other error reporting language used in doc comments. With the existence of the `failure` crate, I suppose this could be slightly ambiguous. In any case, this change brings the particular comment into a consistent state with other mentions of returning errors.
BTreeMap: comment why drain_filter's size_hint is somewhat pessimistic
The `size_hint` of the `DrainFilter` iterator doesn't adjust as you iterate. This hardly seems important to me, but there has been a comparable PR #64383 in the past. I guess a scenario is that you first iterate half the map manually and keep most of the key/value pairs in the map, and then tell the predicate to drain most of the key/value pairs and `.collect` the iterator over the remaining half of the map.
I am totally ambivalent whether this is better or not.
r? @Mark-Simulacrum
Give `impl Trait` in a `const fn` its own feature gate
...previously it was gated under `#![feature(const_fn)]`.
I think we actually want to do this in all const-contexts? If so, this should be `#![feature(const_impl_trait)]` instead. I don't think there's any way to make use of `impl Trait` within a `const` initializer.
cc #77463
r? `@oli-obk`
Eliminate bounds checking in slice::Windows
This is how `<core::slice::Windows as Iterator>::next` looks right now:
```rust
fn next(&mut self) -> Option<&'a [T]> {
if self.size > self.v.len() {
None
} else {
let ret = Some(&self.v[..self.size]);
self.v = &self.v[1..];
ret
}
}
```
The line with `self.v = &self.v[1..];` relies on assumption that `self.v` is definitely not empty at this point. Else branch is taken when `self.size <= self.v.len()`, so `self.v` can be empty if `self.size` is zero. In practice, since `Windows` is never created directly but rather trough `[T]::windows` which panics when `size` is zero, `self.size` is never zero. However, the compiler doesn't know about this check, so it keeps the code which checks bounds and panics.
Using `NonZeroUsize` lets the compiler know about this invariant and reliably eliminate bounds checking without `unsafe` on `-O2`. Here is assembly of `Windows<'a, u32>::next` before and after this change ([goldbolt](https://godbolt.org/z/xrefzx)):
<details>
<summary>Before</summary>
```
example::next:
push rax
mov rcx, qword ptr [rdi + 8]
mov rdx, qword ptr [rdi + 16]
cmp rdx, rcx
jbe .LBB0_2
xor eax, eax
pop rcx
ret
.LBB0_2:
test rcx, rcx
je .LBB0_5
mov rax, qword ptr [rdi]
mov rsi, rax
add rsi, 4
add rcx, -1
mov qword ptr [rdi], rsi
mov qword ptr [rdi + 8], rcx
pop rcx
ret
.LBB0_5:
lea rdx, [rip + .L__unnamed_1]
mov edi, 1
xor esi, esi
call qword ptr [rip + core::slice::slice_index_order_fail@GOTPCREL]
ud2
.L__unnamed_2:
.ascii "./example.rs"
.L__unnamed_1:
.quad .L__unnamed_2
.asciz "\f\000\000\000\000\000\000\000\016\000\000\000\027\000\000"
```
</details>
<details>
<summary>After</summary>
```
example::next:
mov rcx, qword ptr [rdi + 8]
mov rdx, qword ptr [rdi + 16]
cmp rdx, rcx
jbe .LBB0_2
xor eax, eax
ret
.LBB0_2:
mov rax, qword ptr [rdi]
lea rsi, [rax + 4]
add rcx, -1
mov qword ptr [rdi], rsi
mov qword ptr [rdi + 8], rcx
ret
```
</details>
Note the lack of call to `core::slice::slice_index_order_fail` in second snippet.
#### Possible reasons _not_ to merge this PR:
* this changes the error message on panic in `[T]::windows`. However, AFAIK this messages are not covered by backwards compatibility policy.
Add PartialEq impls for Vec <-> slice
This is a follow-up to #71660 and rust-lang/rfcs#2917 to add two more missing vec/slice PartialEq impls:
```
impl<A, B> PartialEq<[B]> for Vec<A> where A: PartialEq<B> { .. }
impl<A, B> PartialEq<Vec<B>> for [A] where A: PartialEq<B> { .. }
```
Since this is insta-stable, it should go through the `@rust-lang/libs` FCP process. Note that I used version 1.47.0 for the `stable` attribute because I assume this will not merge before the 1.46.0 branch is cut next week.
Rollup of 11 pull requests
Successful merges:
- #76784 (Add some docs to rustdoc::clean::inline and def_id functions)
- #76911 (fix VecDeque::iter_mut aliasing issues)
- #77400 (Fix suggestions for x.py setup)
- #77515 (Update to chalk 0.31)
- #77568 (inliner: use caller param_env)
- #77571 (Use matches! for core::char methods)
- #77582 (Move `EarlyOtherwiseBranch` to mir-opt-level 2)
- #77590 (Update RLS and Rustfmt)
- #77605 (Fix rustc_def_path to show the full path and not the trimmed one)
- #77614 (Let backends access span information)
- #77624 (Add c as a shorthand check alternative for new options #77603)
Failed merges:
r? `@ghost`
