Rollup of 8 pull requests
Successful merges:
- #127418 (Wrap too long type name)
- #127594 (Fuchsia status code match arm)
- #127835 (Fix ICE in suggestion caused by `⩵` being recovered as `==`)
- #127858 (match lowering: Rename `MatchPair` to `MatchPairTree`)
- #127871 (Mention that type parameters are used recursively on bivariance error)
- #127913 (remove `debug-logging` default from tools profile)
- #127925 (Remove tag field from `Relation`s)
- #127929 (Use more accurate span for `addr_of!` suggestion)
r? `@ghost`
`@rustbot` modify labels: rollup
Use more accurate span for `addr_of!` suggestion
Use a multipart suggestion instead of a single whole-span replacement:
```
error[E0796]: creating a shared reference to a mutable static
--> $DIR/reference-to-mut-static-unsafe-fn.rs:10:18
|
LL | let _y = &X;
| ^^ shared reference to mutable static
|
= note: this shared reference has lifetime `'static`, but if the static ever gets mutated, or a mutable reference is created, then any further use of this shared reference is Undefined Behavior
help: use `addr_of!` instead to create a raw pointer
|
LL | let _y = addr_of!(X);
| ~~~~~~~~~ +
```
Remove tag field from `Relation`s
Can just use the relation name w/ `std::any::type_name`. Also changes some printing to use instrument. Also changes some instrument levels to `trace` since I expect relations are somewhat hot, so having them print on debug is probably noisy.
r? lcnr
remove `debug-logging` default from tools profile
`debug-logging` conflicts with `download-rustc` option, and doesn't really make sense to enable it for a profile that is used for tool development.
Mention that type parameters are used recursively on bivariance error
Right now when a type parameter is used recursively, even with indirection (so it has a finite size) we say that the type parameter is unused:
```
struct B<T>(Box<B<T>>);
```
This is confusing, because the type parameter is *used*, it just doesn't have its variance constrained. This PR tweaks that message to mention that it must be used *non-recursively*.
Not sure if we should actually mention "variance" here, but also I'd somewhat prefer we don't keep the power users in the dark w.r.t the real underlying issue, which is that the variance isn't constrained. That technical detail is reserved for a note, though.
cc `@fee1-dead`
Fixes#118976Fixes#26283Fixes#53191Fixes#105740Fixes#110466
match lowering: Rename `MatchPair` to `MatchPairTree`
In #120904, `MatchPair` became able to store other match pairs as children, forming a tree. That has made the old name confusing, so this patch renames the type to `MatchPairTree`.
This PR also includes a patch renaming the `test` method to `pick_test_for_match_pair`, since it would conflict with the main change.
r? `@Nadrieril`
Fix ICE in suggestion caused by `⩵` being recovered as `==`
The second suggestion shown here would previously incorrectly assume that the span corresponding to `⩵` was 2 bytes wide composed by 2 1 byte wide chars, so a span pointing at `==` could point only at one of the `=` to remove it. Instead, we now replace the whole thing (as we should have the whole time):
```
error: unknown start of token: \u{2a75}
--> $DIR/unicode-double-equals-recovery.rs:1:16
|
LL | const A: usize ⩵ 2;
| ^
|
help: Unicode character '⩵' (Two Consecutive Equals Signs) looks like '==' (Double Equals Sign), but it is not
|
LL | const A: usize == 2;
| ~~
error: unexpected `==`
--> $DIR/unicode-double-equals-recovery.rs:1:16
|
LL | const A: usize ⩵ 2;
| ^
|
help: try using `=` instead
|
LL | const A: usize = 2;
| ~
```
Fix#127823.
Fuchsia status code match arm
Adds a match arm for the Fuchsia status code upon a process abort. An additional change moves the Windows status code down into the match arm itself instead of being defined as a constant elsewhere.
r? tmandry
Use a multipart suggestion instead of a single whole-span replacement:
```
error[E0796]: creating a shared reference to a mutable static
--> $DIR/reference-to-mut-static-unsafe-fn.rs:10:18
|
LL | let _y = &X;
| ^^ shared reference to mutable static
|
= note: this shared reference has lifetime `'static`, but if the static ever gets mutated, or a mutable reference is created, then any further use of this shared reference is Undefined Behavior
help: use `addr_of!` instead to create a raw pointer
|
LL | let _y = addr_of!(X);
| ~~~~~~~~~ +
```
Rollup of 6 pull requests
Successful merges:
- #124881 (Use ThreadId instead of TLS-address in `ReentrantLock`)
- #127656 (make pub_use_of_private_extern_crate show up in cargo's future breakage reports)
- #127748 (Use Option's discriminant as its size hint)
- #127854 (Add internal lint for detecting non-glob imports of `rustc_type_ir::inherent`)
- #127908 (Update extern linking documentation)
- #127919 (Allow a git command for getting the current branch in bootstrap to fail)
r? `@ghost`
`@rustbot` modify labels: rollup
The second suggestion shown here would previously incorrectly assume that the span corresponding to `⩵` was 2 bytes wide composed by 2 1 byte wide chars, so a span pointing at `==` could point only at one of the `=` to remove it. Instead, we now replace the whole thing (as we should have the whole time):
```
error: unknown start of token: \u{2a75}
--> $DIR/unicode-double-equals-recovery.rs:1:16
|
LL | const A: usize ⩵ 2;
| ^
|
help: Unicode character '⩵' (Two Consecutive Equals Signs) looks like '==' (Double Equals Sign), but it is not
|
LL | const A: usize == 2;
| ~~
error: unexpected `==`
--> $DIR/unicode-double-equals-recovery.rs:1:16
|
LL | const A: usize ⩵ 2;
| ^
|
help: try using `=` instead
|
LL | const A: usize = 2;
| ~
```
Update extern linking documentation
In particular, remove the note saying cdylibs can't link against dylibs — that hasn't been true for over four years.
* 2019-11-07: note is written: b54e8ecc2e
* 2020-01-23: restriction is lifted (without updating docs): 72aaa3a414
Use Option's discriminant as its size hint
I was looking at this in MIR after a question on discord, and noticed that it ends up with a switch in MIR (<https://rust.godbolt.org/z/3q4cYnnb3>), which it doesn't need because (as `Option::as_slice` uses) the discriminant is already the length.
make pub_use_of_private_extern_crate show up in cargo's future breakage reports
This has been a lint for many years.
However, turns out that outright removing it right now would lead to [tons of crater regressions](https://github.com/rust-lang/rust/pull/127656#issuecomment-2233288534) due to crates depending on an ancient version of `bitflags`. So for now this PR just makes this future-compat lint show up in cargo's reports, so people are warned when they use a dependency that is affected by this.
r? `@petrochenkov`
Use ThreadId instead of TLS-address in `ReentrantLock`
Fixes#123458
`ReentrantLock` currently uses the address of a thread local variable as an ID that's unique across all currently running threads. This can lead to uninituitive behavior as in #123458 if TLS blocks get reused. This PR changes `ReentrantLock` to instead use the `ThreadId` provided by `std` as the unique ID. `ThreadId` guarantees uniqueness across the lifetime of the whole process, so we don't need to worry about reusing IDs of terminated threads. The main appeal of this PR is thus the possibility of changing the `ReentrantLock` API to guarantee that if a thread leaks a lock guard, no other thread may ever acquire that lock again.
This does entail some complications:
- previously, the only way to retrieve the current thread ID would've been using `thread::current().id()` which creates a temporary `Arc` and which isn't available in TLS destructors. As part of this PR, the thread ID instead gets cached in its own thread local, as suggested [here](https://github.com/rust-lang/rust/issues/123458#issuecomment-2038207704).
- `ThreadId` is always 64-bit whereas the current implementation uses a usize-sized ID. Since this ID needs to be updated atomically, we can't simply use a single atomic variable on 32 bit platforms. Instead, we fall back to using a (sound) seqlock on 32-bit platforms, which works because only one thread at a time can write to the ID. This seqlock is technically susceptible to the ABA problem, but the attack vector to create actual unsoundness has to be very specific:
- You would need to be able to lock+unlock the lock exactly 2^31 times (or a multiple thereof) while a thread trying to lock it sleeps
- The sleeping thread would have to suspend after reading one half of the thread id but before reading the other half
- The teared result from combining the halves of the thread ID would have to exactly line up with the sleeping thread's ID
The risk of this occurring seems slim enough to be acceptable to me, but correct me if I'm wrong. This also means that the size of the lock increases by 8 bytes on 32-bit platforms, but this also shouldn't be an issue.
Performance wise, I did some crude testing of the only case where this could lead to real slowdowns, which is the case of locking a `ReentrantLock` that's already locked by the current thread. On both aarch64 and x86-64, there is (expectedly) pretty much no performance hit. I didn't have any 32-bit platforms to test the seqlock performance on, so I did the next best thing and just forced the 64-bit platforms to use the seqlock implementation. There, the performance degraded by ~1-2ns/(lock+unlock) on x86-64 and ~6-8ns/(lock+unlock) on aarch64, which is measurable but seems acceptable to me seeing as 32-bit platforms should be a small minority anyways.
cc `@joboet` `@RalfJung` `@CAD97`
This changes `ReentrantLock` to use `ThreadId` for the thread ownership check instead of the address of a thread local. Unlike TLS blocks, `ThreadId` is guaranteed to be unique across the lifetime of the process, so if any thread ever terminates while holding a `ReentrantLockGuard`, no other thread may ever acquire that lock again.
On platforms with 64-bit atomics, this is a very simple change. On other platforms, the approach used is slightly more involved, as explained in the module comment.
This also adds a `CURRENT_ID` thread local in addition to the already existing `CURRENT`. This allows us to access the current `ThreadId` without the relatively heavy machinery used by `thread::current().id()`.
Fix ambiguous cases of multiple & in elided self lifetimes
This change proposes simpler rules to identify the lifetime on `self` parameters which may be used to elide a return type lifetime.
## The old rules
(copied from [this comment](https://github.com/rust-lang/rust/pull/117967#discussion_r1420554242))
Most of the code can be found in [late.rs](https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_resolve/late.rs.html) and acts on AST types. The function [resolve_fn_params](https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_resolve/late.rs.html#2006), in the success case, returns a single lifetime which can be used to elide the lifetime of return types.
Here's how:
* If the first parameter is called self then we search that parameter using "`self` search rules", below
* If no unique applicable lifetime was found, search all other parameters using "regular parameter search rules", below
(In practice the code does extra work to assemble good diagnostic information, so it's not quite laid out like the above.)
### `self` search rules
This is primarily handled in [find_lifetime_for_self](https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_resolve/late.rs.html#2118) , and is described slightly [here](https://github.com/rust-lang/rust/issues/117715#issuecomment-1813115477) already. The code:
1. Recursively walks the type of the `self` parameter (there's some complexity about resolving various special cases, but it's essentially just walking the type as far as I can see)
2. Each time we find a reference anywhere in the type, if the **direct** referent is `Self` (either spelled `Self` or by some alias resolution which I don't fully understand), then we'll add that to a set of candidate lifetimes
3. If there's exactly one such unique lifetime candidate found, we return this lifetime.
### Regular parameter search rules
1. Find all the lifetimes in each parameter, including implicit, explicit etc.
2. If there's exactly one parameter containing lifetimes, and if that parameter contains exactly one (unique) lifetime, *and if we didn't find a `self` lifetime parameter already*, we'll return this lifetime.
## The new rules
There are no changes to the "regular parameter search rules" or to the overall flow, only to the `self` search rules which are now:
1. Recursively walks the type of the `self` parameter, searching for lifetimes of reference types whose referent **contains** `Self`.[^1]
2. Keep a record of:
* Whether 0, 1 or n unique lifetimes are found on references encountered during the walk
4. If no lifetime was found, we don't return a lifetime. (This means other parameters' lifetimes may be used for return type lifetime elision).
5. If there's one lifetime found, we return the lifetime.
6. If multiple lifetimes were found, we abort elision entirely (other parameters' lifetimes won't be used).
[^1]: this prevents us from considering lifetimes from inside of the self-type
## Examples that were accepted before and will now be rejected
```rust
fn a(self: &Box<&Self>) -> &u32
fn b(self: &Pin<&mut Self>) -> &String
fn c(self: &mut &Self) -> Option<&Self>
fn d(self: &mut &Box<Self>, arg: &usize) -> &usize // previously used the lt from arg
```
### Examples that change the elided lifetime
```rust
fn e(self: &mut Box<Self>, arg: &usize) -> &usize
// ^ new ^ previous
```
## Examples that were rejected before and will now be accepted
```rust
fn f(self: &Box<Self>) -> &u32
```
---
*edit: old PR description:*
```rust
struct Concrete(u32);
impl Concrete {
fn m(self: &Box<Self>) -> &u32 {
&self.0
}
}
```
resulted in a confusing error.
```rust
impl Concrete {
fn n(self: &Box<&Self>) -> &u32 {
&self.0
}
}
```
resulted in no error or warning, despite apparent ambiguity over the elided lifetime.
Fixes https://github.com/rust-lang/rust/issues/117715
`debug-logging` conflicts with `download-rustc` option, and doesn't really
make sense to enable it for a profile that is used for tool development.
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Rollup of 7 pull requests
Successful merges:
- #127491 (Migrate 8 very similar FFI `run-make` tests to rmake)
- #127687 (Const-to-pattern-to-MIR cleanup)
- #127822 (Migrate `issue-85401-static-mir`, `missing-crate-dependency` and `unstable-flag-required` `run-make` tests to rmake)
- #127842 (Remove `TrailingToken`.)
- #127864 (cleanup: remove support for 3DNow! cpu features)
- #127899 (Mark myself as on leave)
- #127901 (Add missing GHA group for building `llvm-bitcode-linker`)
r? `@ghost`
`@rustbot` modify labels: rollup
In particular, remove the note saying cdylibs can't link against dylibs — that hasn't been true for over four years.
* 2019-11-07: note is written: b54e8ecc2e
* 2020-01-23: restriction is lifted (without updating docs): 72aaa3a414
cleanup: remove support for 3DNow! cpu features
In llvm/llvm-project@f0eb5587ce all support for 3DNow! intrinsics and instructions were removed. Per the commit message there, only AMD chips between 1998 and 2011 or so actually supported these instructions, and they were effectively replaced by SSE which was available on many more chips. I'd be very surprised if anyone had ever used these from Rust.
`@rustbot` label: +llvm-main
Remove `TrailingToken`.
It's used in `Parser::collect_tokens_trailing_token` to decide whether to capture a trailing token. But the callers actually know whether to capture a trailing token, so it's simpler for them to just pass in a bool.
Also, the `TrailingToken::Gt` case was weird, because it didn't result in a trailing token being captured. It could have been subsumed by the `TrailingToken::MaybeComma` case, and it effectively is in the new code.
r? `@petrochenkov`
Migrate `issue-85401-static-mir`, `missing-crate-dependency` and `unstable-flag-required` `run-make` tests to rmake
Part of #121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).
try-job: armhf-gnu
try-job: test-various
try-job: x86_64-msvc
try-job: aarch64-apple
try-job: dist-x86_64-linux
Const-to-pattern-to-MIR cleanup
Now that all uses of constants without structural equality are hard errors, there's a bunch of cleanup we can do in the code that handles patterns: we can always funnel patterns through valtrees first (rather than having a fallback path for when valtree construction fails), and we can make sure that if we emit a `PartialEq` call it is not calling anything user-defined.
To keep the error messages the same, I made valtree construction failures return the information of *which* type it is that cannot be valtree'd. `search_for_structural_match_violation` is now not needed any more at all, so I removed it.
r? `@oli-obk`
Migrate 8 very similar FFI `run-make` tests to rmake
Part of #121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).
There are some more of these, but while the code is almost always the same, I want to keep the number reasonable so my doc comments can be inspected for potential inaccuracies. Tell me if 8 is too much, I can cut this down.
For the tracking issue:
- issue-25581
- extern-fn-with-extern-types
- extern-fn-struct-passing-abi
- longjmp-across-rust
- static-extern-type
- extern-fn-explicit-align
- extern-fn-with-packed-struct
- extern-fn-mangle
It's used in `Parser::collect_tokens_trailing_token` to decide whether
to capture a trailing token. But the callers actually know whether to
capture a trailing token, so it's simpler for them to just pass in a
bool.
Also, the `TrailingToken::Gt` case was weird, because it didn't result
in a trailing token being captured. It could have been subsumed by the
`TrailingToken::MaybeComma` case, and it effectively is in the new code.
Rollup of 8 pull requests
Successful merges:
- #127077 (Make language around `ToOwned` for `BorrowedFd` more precise)
- #127783 (Put the dots back in RTN pretty printing)
- #127787 (Migrate `atomic-lock-free` to `rmake`)
- #127810 (Rename `tcx` to `cx` in `rustc_type_ir`)
- #127878 (Fix associated item removal suggestion)
- #127886 (Accurate `use` rename suggestion span)
- #127888 (More accurate span for type parameter suggestion)
- #127889 (More accurate span for anonymous argument suggestion)
r? `@ghost`
`@rustbot` modify labels: rollup
More accurate span for anonymous argument suggestion
Use smaller span for suggesting adding `_:` ahead of a type:
```
error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
--> $DIR/anon-params-denied-2018.rs:12:47
|
LL | fn foo_with_qualified_path(<Bar as T>::Baz);
| ^ expected one of 8 possible tokens
|
= note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
help: explicitly ignore the parameter name
|
LL | fn foo_with_qualified_path(_: <Bar as T>::Baz);
| ++
```
More accurate span for type parameter suggestion
After:
```
error[E0229]: associated item constraints are not allowed here
--> $DIR/impl-block-params-declared-in-wrong-spot-issue-113073.rs:3:10
|
LL | impl Foo<T: Default> for String {}
| ^^^^^^^^^^ associated item constraint not allowed here
|
help: declare the type parameter right after the `impl` keyword
|
LL - impl Foo<T: Default> for String {}
LL + impl<T: Default> Foo<T> for String {}
|
```
Before:
```
error[E0229]: associated item constraints are not allowed here
--> $DIR/impl-block-params-declared-in-wrong-spot-issue-113073.rs:3:10
|
LL | impl Foo<T: Default> for String {}
| ^^^^^^^^^^ associated item constraint not allowed here
|
help: declare the type parameter right after the `impl` keyword
|
LL | impl<T: Default> Foo<T> for String {}
| ++++++++++++ ~
```
