Do `fix_*_builtin_expr` hacks on the writeback results
During writeback, we do `fix_{scalar,index}_builtin_expr` so that during MIR build we generate built-in MIR instructions instead of method calls for certain built-in arithmetic operations. We do this by checking the types of these built-in operations are scalar types, and remove the method def-id to essentially mark the operation as built-in and not "overloaded".
For lazy norm and the new trait solver, this is a problem, because we don't actually normalize all the types we end up seeing in the typeck results until they're copied over writeback's copy of the typeck results. To fix this, delay these fixup calls until after this normalization has been done.
This doesn't affect the old trait solver, but does simplify the code a bit IMO, since we can remove a few sets of calls to `resolve_vars_if_possible` and some `borrow_mut`s.
r? `@lcnr`
Don't hold the active queries lock while calling `make_query`
This moves the call to `make_query` outside the parts that holds the active queries lock in `try_collect_active_jobs`. This should help removed the deadlock and borrow panic that has been observed when printing the query stack during an ICE.
cc `@SparrowLii`
r? `@cjgillot`
Rollup of 9 pull requests
Successful merges:
- #112034 (Migrate `item_opaque_ty` to Askama)
- #112179 (Avoid passing --cpu-features when empty)
- #112309 (bootstrap: remove dependency `is-terminal`)
- #112388 (Migrate GUI colors test to original CSS color format)
- #112389 (Add a test for #105709)
- #112392 (Fix ICE for while loop with assignment condition with LHS place expr)
- #112394 (Remove accidental comment)
- #112396 (Track more diagnostics in `rustc_expand`)
- #112401 (Don't `use compile_error as print`)
r? `@ghost`
`@rustbot` modify labels: rollup
Don't `use compile_error as print`
I've spent **1.5 hours** debugging this while trying to compile #112400, if we use `compile_error!`, we should not just forward user input to it, but issue a reasonable error message.
The better solution would be to use a lint like `clippy::print_stdout`, but since we don't have clippy in CI, let's at least make the macro error better.
Also note that some functions called here actually do use `println` (see for example `print_type_sizes` function).
Removed use of iteration through a HashMap/HashSet in rustc_incremental and replaced with IndexMap/IndexSet
This allows for the `#[allow(rustc::potential_query_instability)]` in rustc_incremental to be removed, moving towards fixing #84447 (although a LOT more modules have to be changed to fully resolve it). Only HashMaps/HashSets that are being iterated through have been modified (although many structs and traits outside of rustc_incremental had to be modified as well, as they had fields/methods that involved a HashMap/HashSet that would be iterated through)
I'm making a PR for just 1 module changed to test for performance regressions and such, for future changes I'll either edit this PR to reflect additional modules being converted, or batch multiple modules of changes together and make a PR for each group of modules.
Avoid unwind across `extern "C"` in `thread_local::fast_local`
This is a minimal fix for #112285, in case we want a simple patch that can be easily to backported if that's desirable.
*(Note: I have another broader cleanup which I've mostly omitted from here to avoid clutter, except for the `Cell` change, which isn't needed to fix UB, but simplifies safety comments).*
The only tier-1 target that this occurs on in a way that seems likely to cause problems in practice linux-gnu, although I believe some folks care about that platform somewhat 😉. I'm unsure how big of an issue this is. I've seen stuff like this behave quite badly, but there's a number of reasons to think this might actually be "fine in practice".
I've hedged my bets and assumed we'll backport this at least to beta but my feeling is that there's not enough evidence this is a problem worth backporting further than that.
### More details
This issue seems to have existed since `thread_local!`'s `const` init functionality was added. It occurs if you have a `const`-initialized thread local for a type that `needs_drop`, the drop panics, and you're on a target with support for static thread locals. In this case, we will end up defining an `extern "C"` function in the user crate rather than in libstd, and because the user crate will not have `#![feature(c_unwind)]` enabled, their panic will not be caught by an auto-inserted abort guard.
In practice, the actual situation where problems are likely[^ub] is somewhat narrower.
On most targets with static thread locals, we manage the TLS dtor list by hand (for reentrancy reasons among others). In these cases, while the users code may panic, we're calling it inside our own `extern "C"` (or `extern "system"`) function, which seems to (at least in practice) catch the panic and convert it to an abort.
However, on a few targets, most notably linux-gnu with recent glibc (but also fuchsia and redox), a tls dtor registration mechanism exists which we can actually use directly, [`__cxa_thread_atexit_impl`](https://github.com/rust-lang/rust/blob/master/library/std/src/sys/unix/thread_local_dtor.rs#L26-L36).
This is the case that seems most likely to be a cause for concern, as now we're passing a function to the system library and panicking out of it in a case where there are may not be Rust frames above it on the call stack (since it's running thread shutdown), and even if there were, it may not be prepared to handle such unwinding. If that's the case, it'd be bad.
Is it? Dunno. The fact that it's a `__cxa_*` function makes me think they probably have considered that the callback could throw but I have no evidence here and it doesn't seem to be written down anywhere, so it's just a guess. (I would not be surprised if someone comes into this thread to tell me how definitely-bad-news it is).
That said, as I said, all this is actually UB! If this isn't a "technically UB but fine in practice", but all bets are off if this is the kind of thing we are telling LLVM about.
[^ub]: This is UB so take that with a grain of salt -- I'm absolutely making assumptions about how the UB will behave "in practice" here, which is almost certainly a mistake.
Avoid one `rustc` rebuild in the optimized build pipeline
This PR changes the optimized build pipeline to avoid one `rustc` rebuild, inspired by [this comment](https://github.com/rust-lang/rust/issues/112011#issuecomment-1564991175). This speeds up the pipeline by 5-10 minutes. After this change, we **no longer gather LLVM PGO profiles from compiling stage 2 of `rustc`**.
Now we build `rustc` two times (1x PGO instrumented, 1x PGO optimized) and LLVM three times (1x normal, 1x PGO instrumented, 1x PGO optimized). It should be possible to cache the normal LLVM build, but I'll leave that for another PR.
Force all native libraries to be statically linked when linking a static binary
Previously, `#[link]` without an explicit `kind = "static"` would confuse the linker and end up producing a dynamically linked library because of the `-Bdynamic` flag. However this binary would not work correctly anyways since it was linked with startup code for a static binary.
This PR solves this by forcing all native libraries to be statically linked when the output is a static binary that cannot link to dynamic libraries anyways.
Fixes#108878Fixes#102993
Remember names of `cfg`-ed out items to mention them in diagnostics
# Examples
## `serde::Deserialize` without the `derive` feature (a classic beginner mistake)
I had to slightly modify serde so that it uses explicit re-exports instead of a glob re-export. (Update: a serde PR was merged that adds the manual re-exports)
```
error[E0433]: failed to resolve: could not find `Serialize` in `serde`
--> src/main.rs:1:17
|
1 | #[derive(serde::Serialize)]
| ^^^^^^^^^ could not find `Serialize` in `serde`
|
note: crate `serde` has an item named `Serialize` but it is inactive because its cfg predicate evaluated to false
--> /home/gh-Nilstrieb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.160/src/lib.rs:343:1
|
343 | #[cfg(feature = "serde_derive")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
344 | pub use serde_derive::{Deserialize, Serialize};
| ^^^^^^^^^
= note: the item is gated behind the `serde_derive` feature
= note: see https://doc.rust-lang.org/cargo/reference/features.html for how to activate a crate's feature
```
(the suggestion is not ideal but that's serde's fault)
I already tested the metadata size impact locally by compiling the `windows` crate without any features. `800k` -> `809k`
r? `@ghost`
Rollup of 6 pull requests
Successful merges:
- #112076 (Fall back to bidirectional normalizes-to if no subst-relate candidate in alias-relate goal)
- #112122 (Add `-Ztrait-solver=next-coherence`)
- #112251 (rustdoc: convert `if let Some()` that always matches to variable)
- #112345 (fix(expand): prevent infinity loop in macro containing only "///")
- #112359 (Respect `RUST_BACKTRACE` for delayed bugs)
- #112382 (download-rustc: Fix `x test core` on MacOS)
r? `@ghost`
`@rustbot` modify labels: rollup
download-rustc: Fix `x test core` on MacOS
before, this hardcoded `.so` as the extension for dynamically linked objects, which is incorrect everywhere except linux.
Respect `RUST_BACKTRACE` for delayed bugs
Sometimes, especially with MIR validation, the backtraces from delayed bugs are noise and make it harder to look at them. Respect the environment variable and don't print it when the user doesn't want it.
fix(expand): prevent infinity loop in macro containing only "///"
Fixes https://github.com/rust-lang/rust/issues/112342
Issue #112342 was caused by an infinity loop in `parse_tt_inner`, and the state of it is as follows:
- `matcher`: `[Sequence, Token(Doc), SequenceKleeneOpNoSep(op: ZeroOrMore), Eof]`
- loop:
| Iteration | Action |
| - | - |
| 0 | enter `Sequence`|
| 1 | enter `Token(Doc)` and `mp.idx += 1` had been executed |
| 2 | enter `SequenceKleeneOpNoSep` and reset `mp.idx` to `1` |
| 3 | enter `Token(Doc)` again|
To prevent the infinite loop, a check for whether it only contains `DocComment` in `check_lhs_no_empty_seq` had been added.
Add `-Ztrait-solver=next-coherence`
Flag that conditionally uses the trait solver *only* during coherence, for more testing and/or eventual partial-migration onto the trait solver (in the medium- to long-term).
* This still uses the selection context in some of the coherence methods I think, so it's not "complete". Putting this up for review and/or for further work in-tree.
* I probably need to spend a bit more time making sure that we don't sneakily create any other infcx's during coherence that also need the new solver enabled.
r? `@lcnr`
Fall back to bidirectional normalizes-to if no subst-relate candidate in alias-relate goal
Sometimes we get into the case where the choice of normalizes-to branch in alias-relate are both valid, but we cannot make a choice of which one to take because they are different -- either returning equivalent but permuted region constraints, or equivalent opaque type definitions but differing modulo normalization.
In this case, we can make progress by considering a fourth candidate where we compute both normalizes-to branches together and canonicalize that as a response. This is essentially the AND intersection of both normalizes-to branches. In an ideal world, we'd be returning something more like the OR intersection of both branches, but we have no way of representing that either for regions (maybe eventually) or opaques (don't see that happening ever).
This is incomplete, so like the subst-relate fallback it's only considered outside of coherence. But it doesn't seem like a dramatic strengthening of inference or anything, and is useful for helping opaque type inference succeed when the hidden type is a projection.
## Example
Consider the goal - `AliasRelate(Tait, <[i32; 32] as IntoIterator>::IntoIter)`.
We have three ways of currently solving this goal:
1. SubstRelate - fails because we can't directly equate the substs of different alias kinds.
2. NormalizesToRhs - `Tait normalizes-to <[i32; 32] as IntoIterator>::IntoIter`
* Ends up infering opaque definition - `Tait := <[i32; 32] as IntoIterator>::IntoIter`
3. NormalizesToLhs - `<[i32; 32] as IntoIterator>::IntoIter normalizes-to Tait`
* Find impl candidate, substitute the associated type - `std::array::IntoIter<i32, 32>`
* Equate `std::array::IntoIter<i32, 32>` and `Tait`
* Ends up infering opaque definition - `Tait := std::array::IntoIter<i32, 32>`
The problem here is that 2 and 3 are essentially both valid, since we have aliases that normalize on both sides, but due to lazy norm, they end up inferring different opaque type definitions that are only equal *after* normalizing them further.
---
r? `@lcnr`
