This makes `-Zprint-type-sizes`'s output easier to read, because the
name of an `async fn` is more immediately recognizable than its span.
I also deleted the comment "FIXME(eddyb) should use `def_span`." because
it appears to have already been fixed by commit 67727aa7c3.
This saves some debug and scope metadata in every single function that calls it.
Normally wouldn't be worth it, but with the derives there's *so* many of these.
Fix validation on substituted callee bodies in MIR inliner
When inlining a coroutine, we will substitute the MIR body with the args of the call. There is code in the MIR validator that attempts to prevent query cycles, and will use the coroutine body directly when it detects that's the body that's being validated. That means that when inlining a coroutine body that has been substituted, it may no longer be parameterized over the original args of the coroutine, which will lead to substitution ICEs.
Fixes#119064
refactor check_{lang,library}_ub: use a single intrinsic
This enacts the plan I laid out [here](https://github.com/rust-lang/rust/pull/122282#issuecomment-1996917998): use a single intrinsic, called `ub_checks` (in aniticpation of https://github.com/rust-lang/compiler-team/issues/725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library.
This makes it easier to do something like https://github.com/rust-lang/rust/pull/122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time.
The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics.
r? `@saethlin`
Some of the marker statements used by coverage are added during MIR building
for use by the InstrumentCoverage pass (during analysis), and are not needed
afterwards.
misc cleanups from debugging something
rename `instantiate_canonical_with_fresh_inference_vars` to `instantiate_canonical` the substs for the canonical are not solely infer vars as that would be wildly wrong and it is rather confusing to see this method called and think that the entire canonicalization setup is completely broken when it is not 👍
also update region debug printing to be more like the custom impls for Ty/Const, right now regions in debug output are horribly verbose and make it incredibly hard to read but with this atleast boundvars and placeholders when debugging the new solver do not take up excessive amounts of space.
r? `@lcnr`
Remove some only- clauses from mir-opt tests
Derived from https://github.com/rust-lang/rust/pull/122295
Many of these tests were originally codegen tests, and MIR is more trivially portable than LLVM IR. We simply don't need to restrict the platform in most cases.
r? Nadrieril
match lowering: don't collect test alternatives ahead of time
I'm very happy with this one. Before this, when sorting candidates into the possible test branches, we manually computed `usize` indices to determine in which branch each candidate goes. To make this work we had a first pass that collected the possible alternatives we'd have to deal with, and a second pass that actually sorts the candidates.
In this PR, I replace `usize` indices with a dedicated enum. This makes `sort_candidates` easier to follow, and we don't need the first pass anymore.
r? ``@matthewjasper``
Add FileCheck annotations to MIR-opt unnamed-fields tests
Part of #116971
Adds filecheck annotations to unnamed-fields mir-opt tests in `tests/mir-opt/unnamed-fields`
match lowering: Lower bindings in a predictable order
After the recent refactorings, we can now lower bindings in a truly predictable order. The order in https://github.com/rust-lang/rust/pull/120214 was an improvement but not very clear. With this PR, we lower bindings from left to right, with the special case that `x @ pat` is traversed as `pat @ x` (i.e. `x` is lowered after any bindings in `pat`).
This description only applies in the absence of or-patterns. Or-patterns make everything complicated, because the binding place depends on the subpattern. Until I have a better idea I leave them to be handled in whatever weird order arises from today's code.
r? `@matthewjasper`
match lowering: Separate the `bool` case from other integers in `TestKind`
`TestKind::SwitchInt` had a special case for `bool` essentially everywhere it's used, so I made `TestKind::If` to handle the bool case on its own.
r? `@matthewjasper`
Make the success arms of `if lhs || rhs` meet up in a separate block
Extracted from #118305, where this is necessary to avoid introducing a bug when injecting marker statements into the then/else arms.
---
In the previous code (#111752), the success block of `lhs` would jump directly to the success block of `rhs`. However, `rhs_success_block` could already contain statements that are specific to the RHS, and the direct goto causes them to be executed in the LHS success path as well.
This patch therefore creates a fresh block that the LHS and RHS success blocks can both jump to.
---
I think the reason we currently get away with this is that `rhs_success_block` usually doesn't contain anything other than StorageDead statements for locals used by the RHS, and those statements don't seem to cause problems in the LHS success path (which never makes those locals live).
But if we start adding meaningful statements for branch coverage (or MC/DC coverage), it's important to keep the LHS and RHS blocks separate.
In the previous code, the success block of `lhs` would jump directly to the
success block of `rhs`. However, `rhs_success_block` could already contain
statements that are specific to the RHS, and the direct goto causes them to be
executed in the LHS success path as well.
This patch therefore creates a fresh block that the LHS and RHS success blocks
can both jump to.
Add `#[rustc_no_mir_inline]` for standard library UB checks
should help with #121110 and also with #120848
Because the MIR inliner cannot know whether the checks are enabled or not, so inlining is an unnecessary compile time pessimization when debug assertions are disabled. LLVM knows whether they are enabled or not, so it can optimize accordingly without wasting time.
r? `@saethlin`
match lowering: simplify empty candidate selection
In match lowering, `match_simplified_candidates` is tasked with removing candidates that are fully matched and linking them up properly. The code that does that was needlessly complicated; this PR simplifies it.
The overall change isn't big but I split it up into tiny commits to convince myself that I was correctly preserving behavior. The test changes are all due to the first commit. Let me know if you'd prefer me to split up the PR to make reviewing easier.
r? `@matthewjasper`
match lowering: eagerly simplify match pairs
This removes one important complication from match lowering. Before this, match pair simplification (which includes collecting bindings and type ascriptions) was intertwined with the whole match lowering algorithm.
I'm avoiding this by storing in each `MatchPair` the sub-`MatchPair`s that correspond to its subfields. This makes it possible to simplify everything (except or-patterns) in `Candidate::new()`.
This should open up further simplifications. It will also give us proper control over the order of bindings.
r? `@matthewjasper`
Use intrinsics::debug_assertions in debug_assert_nounwind
This is the first item in https://github.com/rust-lang/rust/issues/120848.
Based on the benchmarking in this PR, it looks like, for the programs in our benchmark suite, enabling all these additional checks does not introduce significant compile-time overhead, with the single exception of `Alignment::new_unchecked`. Therefore, I've added `#[cfg(debug_assertions)]` to that one call site, so that it remains compiled out in the distributed standard library.
The trailing commas in the previous calls to `debug_assert_nounwind!` were causing the macro to expand to `panic_nouwnind_fmt`, which requires more work to set up its arguments, and that overhead alone is measured between this perf run and the next: https://github.com/rust-lang/rust/pull/120863#issuecomment-1937423502
match lowering: simplify block creation
Match lowering was doing complicated things with block creation. As far as I can tell it was trying to avoid creating unneeded blocks, but of the three places that start out with `otherwise = &mut None`, two of them called `otherwise.unwrap_or_else(|| self.cfg.start_new_block())` anyway. As far as I can tell the only place where this PR makes a difference is in `lower_match_tree`, which did indeed sometimes avoid creating the unreachable final block + FakeRead. Unless this is important I propose we do the naive thing instead.
I have not checked all the graph isomorphisms by hand, but at a glance the test diff looks sensible.
r? `@matthewjasper`
Fold pointer operations in GVN
This PR proposes 2 combinations of cast operations in MIR GVN:
- a chain of `PtrToPtr` or `MutToConstPointer` casts can be folded together into a single `PtrToPtr` cast;
- we attempt to evaluate more ptr ops when there is no provenance.
In particular, this allows to read from static slices.
This is not yet sufficient to see through slice operations that use `PtrComponents` (because that's a union), but still a step forward.
r? `@ghost`
Print kind of coroutine closure
Make sure that we print "async closure" when we have an async closure, rather than calling it generically a ["coroutine-closure"](https://github.com/rust-lang/rust/pull/120361).
Fixes#120886
r? oli-obk
match lowering: consistently lower bindings deepest-first
Currently when lowering match expressions to MIR, we do a funny little dance with the order of bindings. I attempt to explain it in the third commit: we handle refutable (i.e. needing a test) patterns differently than irrefutable ones. This leads to inconsistencies, as reported in https://github.com/rust-lang/rust/issues/120210. The reason we need a dance at all is for situations like:
```rust
fn foo1(x: NonCopyStruct) {
let y @ NonCopyStruct { copy_field: z } = x;
// the above should turn into
let z = x.copy_field;
let y = x;
}
```
Here the `y ```````@```````` binding will move out of `x`, so we need to copy the field first.
I believe that the inconsistency came about when we fixed https://github.com/rust-lang/rust/issues/69971, and didn't notice that the fix didn't extend to refutable patterns. My guess then is that ordering bindings by "deepest-first, otherwise source order" is a sound choice. This PR implements that (at least I hope, match lowering is hard to follow 🥲).
Fixes https://github.com/rust-lang/rust/issues/120210
r? ```````@oli-obk``````` since you merged the original fix to https://github.com/rust-lang/rust/issues/69971
cc ```````@matthewjasper```````
Add FileCheck annotations to MIR-opt SROA tests
Part of #116971, adds FileCheck annotations to SROA MIR-opt tests in `tests/mir-opt/sroa` and a few uncategorized files.
r? cjgillot
raw pointer metadata API: data address -> data pointer
A pointer consists of [more than just an address](https://github.com/rust-lang/rfcs/pull/3559), so let's not equate "pointer" and "address" in these docs.
Rename `pointer` field on `Pin`
A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl:
```rust
use std::{marker::PhantomPinned, ptr};
struct Pinned {
data: i32,
pointer: *const i32,
_pin: PhantomPinned,
}
fn main() {
let mut b = Box::pin(Pinned {
data: 42,
pointer: ptr::null(),
_pin: PhantomPinned,
});
{
let pinned = unsafe { b.as_mut().get_unchecked_mut() };
pinned.pointer = &pinned.data;
}
println!("{}", unsafe { *b.pointer });
}
```
```rust
error[E0658]: use of unstable library feature 'unsafe_pin_internals'
--> <source>:19:30
|
19 | println!("{}", unsafe { *b.pointer });
| ^^^^^^^^^
error[E0277]: `Pinned` doesn't implement `std::fmt::Display`
--> <source>:19:20
|
19 | println!("{}", unsafe { *b.pointer });
| ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `Pinned`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
```
Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with #93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected.
To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.
Use an interpreter in MIR jump threading
This allows to understand assignments of aggregate constants. This case appears more frequently with GVN promoting aggregates to constants.
The internal, unstable field of `Pin` can conflict with fields from the
inner type accessed via the `Deref` impl. Rename it from `pointer` to
`__pointer`, to make it less likely to conflict with anything else.
Sandwich MIR optimizations between DSE.
This PR reorders MIR optimization passes in an attempt to increase their efficiency.
- Stop running CopyProp before GVN, it's useless as GVN will do the same thing anyway. Instead, we perform CopyProp at the end of the pipeline, to ensure we do not emit copy/move chains.
- Run DSE before GVN, as it increases the probability to have single-assignment locals.
- Run DSE after the final CopyProp to turn copies into moves.
r? `@ghost`
Avoid some redundant work in GVN
The first 2 commits are about reducing the perf effect.
Third commit avoids doing redundant work: is a local is SSA, it already has been simplified, and the resulting value is in `self.locals`. No need to call any code on it.
The last commit avoids removing some storage statements.
r? wg-mir-opt
coverage: Add enums to accommodate other kinds of coverage mappings
Extracted from #118305.
LLVM supports several different kinds of coverage mapping regions, but currently we only ever emit ordinary “code” regions. This PR performs the plumbing required to add other kinds of regions as enum variants, but does not add any specific variants other than `Code`.
The main motivation for this change is branch coverage, but it will also allow separate experimentation with gap regions and skipped regions, which might help in producing more accurate and useful coverage reports.
---
``@rustbot`` label +A-code-coverage
Reorder early post-inlining passes.
`RemoveZsts`, `RemoveUnneededDrops` and `UninhabitedEnumBranching` only depend on types, so they should be executed together early after MIR inlining introduces those types.
This does not change the end-result, but this makes the pipeline a bit more consistent.
Merge dead bb pruning and unreachable bb deduplication.
Both routines share the same basic structure: iterate on all bbs to identify work, and then renumber bbs.
We can do both at once.
Run Miri and mir-opt tests without a target linker
Normally, we need a linker for the target to build the standard library. That's only because `std` declares crate-type lib and dylib; building the dylib is what creates a need for the linker.
But for mir-opt tests (and for Miri) we do not need to build a `libstd.so`. So with this PR, when we build the standard library for mir-opt tests, instead of `cargo build` we run `cargo rustc --crate-type=lib` which overrides the configured crate types in `std`'s manifest.
I've also swapped in what seems to me a better hack than `BOOTSTRAP_SKIP_TARGET_SANITY` to prevent cross-interpreting with Miri from checking for a target linker and expanded it to mir-opt tests too. Whether it's actually better is up to a reviewer.
By using FxIndexMap instead of FxHashMap, so that the order of visiting
of locals is deterministic.
We also need to bless
copy_propagation_arg.foo.DestinationPropagation.panic*.diff. Do not
review the diff of the diff. Instead look at the diff file before and
after this commit. Both before and after this commit, 3 statements are
replaced with nop. It's just that due to change in ordering, different
statements are replaced. But the net result is the same.
Migrate memory overlap check from validator to lint
The check attempts to identify potential undefined behaviour, rather
than whether MIR is well-formed. It belongs in the lint not validator.
Follow up to changes from #119077.
Remove `-Zdump-mir-spanview`
The `-Zdump-mir-spanview` flag was added back in #76074, as a development/debugging aid for the initial work on what would eventually become `-Cinstrument-coverage`. It causes the compiler to emit an HTML file containing a function's source code, with various spans highlighted based on the contents of MIR.
When the suggestion was made to [triage and remove unnecessary `-Z` flags (Zulip)](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60-Z.60.20option.20triage), I noted that this flag could potentially be worth removing, but I wanted to keep it around to see whether I found it useful for my own coverage work.
But when I actually tried to use it, I ran into various issues (e.g. it crashes on `tests/coverage/closure.rs`). If I can't trust it to work properly without a full overhaul, then instead of diving down a rabbit hole of trying to fix arcane span-handling bugs, it seems better to just remove this obscure old code entirely.
---
````@rustbot```` label +A-code-coverage
Reevaluate `body.should_skip()` after updating the MIR phase to ensure
that injected MIR is processed correctly.
Update a few custom MIR tests that were ill-formed for the injected
phase.
