Fix `FnMut::call_mut`/`Fn::call` shim for async closures that capture references
I adjusted async closures to be able to implement `Fn` and `FnMut` *even if* they capture references, as long as those references did not need to borrow data from the closure captures themselves. See #125259.
However, when I did this, I didn't actually relax an assertion in the `build_construct_coroutine_by_move_shim` shim code, which builds the `Fn`/`FnMut`/`FnOnce` implementations for async closures. Therefore, if we actually tried to *call* `FnMut`/`Fn` on async closures, it would ICE.
This PR adjusts this assertion to ensure that we only capture immutable references in closures if they implement `Fn`/`FnMut`. It also adds a bunch of tests and makes more of the async-closure tests into `build-pass` since we often care about these tests actually generating the right closure shims and stuff. I think it might be excessive to *always* use build-pass here, but 🤷 it's not that big of a deal.
Fixes#127019Fixes#127012
r? oli-obk
In 126578 we ended up with more binary size increases than expected.
This change attempts to avoid inlining large things into small things, to avoid that kind of increase, in cases when top-down inlining will still be able to do that inlining later.
Remove more `PtrToPtr` casts in GVN
This addresses two things I noticed in MIR:
1. `NonNull::<T>::eq` does `(a as *mut T) == (b as *mut T)`, but it could just compare the `*const T`s, so this removes `PtrToPtr` casts that are on both sides of a pointer comparison, so long as they're not fat-to-thin casts.
2. `NonNull::<T>::addr` does `transmute::<_, usize>(p as *const ())`, but so long as `T: Thin` that cast doesn't do anything, and thus we can directly transmute the `*const T` instead.
r? mir-opt
`PtrMetadata` doesn't care about `*const`/`*mut`/`&`/`&mut`, so GVN away those casts in its argument.
This includes updating MIR to allow calling PtrMetadata on references too, not just raw pointers. That means that `[T]::len` can be just `_0 = PtrMetadata(_1)`, for example.
# Conflicts:
# tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-abort.mir
# tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-unwind.mir
Account for things that optimize out in inlining costs
This updates the MIR inlining `CostChecker` to have both bonuses and penalties, rather than just penalties.
That lets us add bonuses for some things where we want to encourage inlining without risking wrapping into a gigantic cost. For example, `switchInt(const …)` we give an inlining bonus because codegen will actually eliminate the branch (and associated dead blocks) once it's monomorphized, so measuring both sides of the branch gives an unrealistically-high cost to it. Similarly, an `unreachable` terminator gets a small bonus, because whatever branch leads there doesn't actually exist post-codegen.
Clean up some comments near `use` declarations
#125443 will reformat all `use` declarations in the repository. There are a few edge cases involving comments on `use` declarations that require care. This PR cleans up some clumsy comment cases, taking us a step closer to #125443 being able to merge.
r? ``@lqd``
Apparently MIR borrowck cares about at least one of these for checking variance.
In runtime MIR, though, there's no need for them as `PtrToPtr` does the same thing.
(Banning them simplifies passes like GVN that no longer need to handle multiple cast possibilities.)
coverage: Replace the old span refiner with a single function
As more and more of the span refiner's functionality has been pulled out into separate early passes, it has finally reached the point where we can remove the rest of the old `SpansRefiner` code, and replace it with a single modestly-sized function.
~~There should be no change to the resulting coverage mappings, as demonstrated by the lack of changes to test output.~~
There is *almost* no change to the resulting coverage mappings. There are some minor changes to `loop` that on inspection appear to be neutral in terms of accuracy, with the old behaviour being a slightly-horrifying implementation detail of the old code, so I think they're acceptable.
Previous work in this direction includes:
- #125921
- #121019
- #119208
As more and more of the span refiner's functionality has been pulled out into
separate early passes, it has finally reached the point where we can remove the
rest of the old `SpansRefiner` code, and replace it with a single
modestly-sized function.
Enable GVN for `AggregateKind::RawPtr`
Looks like I was worried for nothing; this seems like it's much easier than I was originally thinking it would be.
r? `@cjgillot`
This should be useful for `x[..4]`-like things, should those start inlining enough to expose the lengths.
The only non-obvious changes:
- `building/storage_live_dead_in_statics.rs` has a `#[rustfmt::skip]`
attribute to avoid reformating a table of data.
- Two `.mir` files have slight changes involving line numbers.
- In `unusual_item_types.rs` an `EMIT_MIR` annotation is moved to
outside a function, which is the usual spot, because `tidy` complains
if such a comment is indented.
The commit also tweaks the comments in `rustfmt.toml`.
The `mir!` macro has multiple parts:
- An optional return type annotation.
- A sequence of zero or more local declarations.
- A mandatory starting anonymous basic block, which is brace-delimited.
- A sequence of zero of more additional named basic blocks.
Some `mir!` invocations use braces with a "block" style, like so:
```
mir! {
let _unit: ();
{
let non_copy = S(42);
let ptr = std::ptr::addr_of_mut!(non_copy);
// Inside `callee`, the first argument and `*ptr` are basically
// aliasing places!
Call(_unit = callee(Move(*ptr), ptr), ReturnTo(after_call), UnwindContinue())
}
after_call = {
Return()
}
}
```
Some invocations use parens with a "block" style, like so:
```
mir!(
let x: [i32; 2];
let one: i32;
{
x = [42, 43];
one = 1;
x = [one, 2];
RET = Move(x);
Return()
}
)
```
And some invocations uses parens with a "tighter" style, like so:
```
mir!({
SetDiscriminant(*b, 0);
Return()
})
```
This last style is generally used for cases where just the mandatory
starting basic block is present. Its braces are placed next to the
parens.
This commit changes all `mir!` invocations to use braces with a "block"
style. Why?
- Consistency is good.
- The contents of the invocation is a block of code, so it's odd to use
parens. They are more normally used for function-like macros.
- Most importantly, the next commit will enable rustfmt for
`tests/mir-opt/`. rustfmt is more aggressive about formatting macros
that use parens than macros that use braces. Without this commit's
changes, rustfmt would break a couple of `mir!` macro invocations that
use braces within `tests/mir-opt` by inserting an extraneous comma.
E.g.:
```
mir!(type RET = (i32, bool);, { // extraneous comma after ';'
RET.0 = 1;
RET.1 = true;
Return()
})
```
Switching those `mir!` invocations to use braces avoids that problem,
resulting in this, which is nicer to read as well as being valid
syntax:
```
mir! {
type RET = (i32, bool);
{
RET.0 = 1;
RET.1 = true;
Return()
}
}
```
Enable DestinationPropagation by default.
~~Based on https://github.com/rust-lang/rust/pull/115291.~~
This PR proposes to enable the destination propagation pass by default.
This pass is meant to reduce the amount of copies present in MIR.
At the same time, this PR removes the `RenameReturnPlace` pass, as it is currently unsound.
`DestinationPropagation` is not limited to `_0`, but does not handle borrowed locals.
[ACP 362] genericize `ptr::from_raw_parts`
This implements https://github.com/rust-lang/libs-team/issues/362
As such, it can partially undo https://github.com/rust-lang/rust/pull/124795 , letting `slice_from_raw_parts` just call `from_raw_parts` again without re-introducing the unnecessary cast to MIR.
By doing this it also removes a spurious cast from `str::from_raw_parts`. And I think it does a good job of showing the value of the ACP, since the only thing that needed new turbofishing because of this is inside `ptr::null(_mut)`, but only because `ptr::without_provenance(_mut)` doesn't support pointers to extern types, which it absolutely could (without even changing the implementation).
don't inhibit random field reordering on repr(packed(1))
`inhibit_struct_field_reordering_opt` being false means we exclude this type from random field shuffling. However, `packed(1)` types can still be shuffled! The logic was added in https://github.com/rust-lang/rust/pull/48528 since it's pointless to reorder fields in packed(1) types (there's no padding that could be saved) -- but that shouldn't inhibit `-Zrandomize-layout` (which did not exist at the time).
We could add an optimization elsewhere to not bother sorting the fields for `repr(packed)` types, but I don't think that's worth the effort.
This *does* change the behavior in that we may now reorder fields of `packed(1)` structs (e.g. if there are niches, we'll try to move them to the start/end, according to `NicheBias`). We were always allowed to do that but so far we didn't. Quoting the [reference](https://doc.rust-lang.org/reference/type-layout.html):
> On their own, align and packed do not provide guarantees about the order of fields in the layout of a struct or the layout of an enum variant, although they may be combined with representations (such as C) which do provide such guarantees.
