coverage: Use a query to find counters/expressions that must be zero
As of #133446, this query (`coverage_ids_info`) determines which counter/expression IDs are unused. So with only a little extra work, we can take the code that was using that information to determine which coverage counters/expressions must be zero, and move that inside the query as well.
There should be no change in compiler output.
coverage: Prefer to visit nodes whose predecessors have been visited
In coverage instrumentation, we need to traverse the control-flow graph and decide what kind of counter (physical counter or counter-expression) should be used for each node that needs a counter.
The existing traversal order is complex and hard to tweak. This new traversal order tries to be a bit more principled, by always preferring to visit nodes whose predecessors have already been visited, which is a good match for how the counter-creation code ends up dealing with a node's in-edges and out-edges.
For several of the coverage tests, this ends up being a strict improvement in reducing the size of the coverage metadata, and also reducing the number of physical counters needed.
(The new traversal should hopefully also allow some further code simplifications in the future.)
---
This is made possible by the separate simplification pass introduced by #133849. Without that, almost any change to the traversal order ends up increasing the size of the expression table or the number of physical counters.
The words "before" and "after" have an obvious temporal meaning, e.g.
`seek_before_primary_effect`,
`visit_statement_{before,after}_primary_effect`. But "before" is also
used to name the effect that occurs before the primary effect of a
statement/terminator; this is `Effect::Before`. This leads to the
confusing possibility of talking about things happening "before/after
the before event".
This commit removes this awkward overloading of "before" by renaming
`Effect::Before` as `Effect::Early`. It also renames some of the
`Analysis` and `ResultsVisitor` methods to be more consistent.
Here are the before and after names:
- `Effect::{Before,Primary}` -> `Effect::{Early,Primary}`
- `apply_before_statement_effect` -> `apply_early_statement_effect`
- `apply_statement_effect` -> `apply_primary_statement_effect`
- `visit_statement_before_primary_effect` -> `visit_after_early_statement_effect`
- `visit_statement_after_primary_effect` -> `visit_after_primary_statement_effect`
(And s/statement/terminator/ for all the terminator events.)
It uses `MaybeInitializedPlaces` and `MaybeUninitializedPlaces`, but
calls the results `live` and `dead`. This is very confusing given that
there are also analyses called `MaybeLiveLocals` and `MaybeStorageLive`
and `MaybeStorageDead`.
This commit changes it to use `maybe_init` and `maybe_uninit`.
Introduce `MixedBitSet`
`ChunkedBitSet` is good at avoiding excessive memory usage for programs with very large functgions where dataflow bitsets have very large domain sizes. But it's overly heavyweight for small bitsets, because any non-empty `ChunkedBitSet` takes up at least 256 bytes.
This PR introduces `MixedBitSet`, which is a simple bitset that uses `BitSet` for small/medium bitsets and `ChunkedBitSet` for large bitsets. It's a speed and memory usage win.
r? `@Mark-Simulacrum`
This query (`coverage_ids_info`) already determines which counter/expression
IDs are unused, so it only takes a little extra effort to also determine which
counters/expressions must have a value of zero.
It's a performance win because `MixedBitSet` is faster and uses less
memory than `ChunkedBitSet`.
Also reflow some overlong comment lines in
`lint_tail_expr_drop_order.rs`.
remove `Ty::is_copy_modulo_regions`
Using these functions is likely incorrect if an `InferCtxt` is available, I moved this function to `TyCtxt` (and added it to `LateContext`) and added a note to the documentation that one should prefer `Infer::type_is_copy_modulo_regions` instead.
I didn't yet move `is_sized` and `is_freeze`, though I think we should move these as well.
r? `@compiler-errors` cc #132279
The `Borrowed` variant is no longer used. This commit removes it, along
with the `as_results_cursor` method that produces it, and renames
`as_results_cursor_mut` as `as_results_cursor`.
coverage: Store coverage source regions as `Span` until codegen
Historically, coverage spans were converted into line/column coordinates during the MIR instrumentation pass.
This PR moves that conversion step into codegen, so that coverage spans spend most of their time stored as `Span` instead.
In addition to being conceptually nicer, this also reduces the size of coverage mappings in MIR, because `Span` is smaller than 4x u32.
---
There should be no changes to coverage output.
Remove -Zfuel.
I'm not sure this feature is used. I only found 2 references in a google search, both referring to its introduction.
Meanwhile, it's a global mutable state, untracked by incremental compilation, so incompatible with it.
`ResultsCursor` currently owns its `Results`. But sometimes the
`Results` is needed again afterwards. So there is
`ResultsCursor::into_results` for extracting the `Results`, which leads
to some awkwardness.
This commit adds `ResultsHandle`, a `Cow`-like type that can either
borrow or own a a `Results`. `ResultsCursor` now uses it. This is good
because some `ResultsCursor`s really want to own their `Results`, while
others just want to borrow it.
We end with with a few more lines of code, but get some nice cleanups.
- `ResultsCursor::into_results` and `Formatter::into_results` are
removed.
- `write_graphviz_results` now just borrows a `Results`, instead of the
awkward "take ownership of a `Results` and then return it unchanged"
pattern.
This reinstates the cursor flexibility that was lost in #118230 -- which
removed the old `ResultsRefCursor` and `ResultsCloneCursor` types -- but
in a much simpler way. Hooray!
Reduce false positives of tail-expr-drop-order from consumed values (attempt #2)
r? `@nikomatsakis`
Tracked by #123739.
Related to #129864 but not replacing, yet.
Related to #130836.
This is an implementation of the approach suggested in the [Zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/temporary.20drop.20order.20changes). A new MIR statement `BackwardsIncompatibleDrop` is added to the MIR syntax. The lint now works by inspecting possibly live move paths before at the `BackwardsIncompatibleDrop` location and the actual drop under the current edition, which should be one before Edition 2024 in practice.
take 2
open up coroutines
tweak the wordings
the lint works up until 2021
We were missing one case, for ADTs, which was
causing `Result` to yield incorrect results.
only include field spans with significant types
deduplicate and eliminate field spans
switch to emit spans to impl Drops
Co-authored-by: Niko Matsakis <nikomat@amazon.com>
collect drops instead of taking liveness diff
apply some suggestions and add explantory notes
small fix on the cache
let the query recurse through coroutine
new suggestion format with extracted variable name
fine-tune the drop span and messages
bugfix on runtime borrows
tweak message wording
filter out ecosystem types earlier
apply suggestions
clippy
check lint level at session level
further restrict applicability of the lint
translate bid into nop for stable mir
detect cycle in type structure
the behavior of the type system not only depends on the current
assumptions, but also the currentnphase of the compiler. This is
mostly necessary as we need to decide whether and how to reveal
opaque types. We track this via the `TypingMode`.
There are a handful of tier 2 and tier 3 targets that cause a LLVM crash
or linker error when generating code that contains `f16` or `f128`. The
cranelift backend also does not support these types. To work around
this, every function in `std` or `core` that contains these types must
be marked `#[inline]` in order to avoid sending any code to the backend
unless specifically requested.
However, this is inconvenient and easy to forget. Introduce a check for
these types in the frontend that automatically inlines any function
signatures that take or return `f16` or `f128`.
Note that this is not a perfect fix because it does not account for the
types being passed by reference or as members of aggregate types, but
this is sufficient for what is currently needed in the standard library.
Fixes: https://github.com/rust-lang/rust/issues/133035
Closes: https://github.com/rust-lang/rust/pull/133037
coverage: Restrict empty-span expansion to only cover `{` and `}`
Coverage instrumentation has some tricky code for converting a coverage-relevant `Span` into a set of start/end line/byte-column coordinates that will be embedded in the CGU's coverage metadata.
A big part of this complexity is special code for handling empty spans, which are expanded into non-empty spans (if possible) because LLVM's coverage reporter does not handle empty spans well.
This PR simplifies that code by restricting it to only apply in two specific situations: when the character after the empty span is `{`, or the character before the empty span is `}`.
(As an added benefit, this means that the expanded spans no longer extend awkwardly beyond the end of a physical line, which was common under the previous implementation.)
Along the way, this PR also removes some unhelpful code for dealing with function source code spread across multiple files. Functions currently can't have coverage spans in multiple files, and if that ever changes (e.g. to properly support expansion regions) then this code will need to be completely overhauled anyway.
Functions currently can't have mappings in multiple files, and if that ever
changes (e.g. to properly support expansion regions), this code will need to be
completely overhauled anyway.
coverage: Simplify parts of coverage graph creation
This is a combination of three semi-related simplifications to how coverage graphs are created, grouped into one PR to avoid conflicts.
There are no observable changes to the output of any of the coverage tests.
Now that `Results` is the only impl of `ResultsVisitable`, the trait can
be removed. This simplifies things by removining unnecessary layers of
indirection and abstraction.
- `ResultsVisitor` is simpler.
- Its type parameter changes from `R` (an analysis result) to the
simpler `A` (an analysis).
- It no longer needs the `Domain` associated type, because it can use
`A::Domain`.
- Occurrences of `R` become `Results<'tcx, A>`, because there is now
only one kind of analysis results.
- `save_as_intervals` also changes type parameter from `R` to `A`.
- The `results.reconstruct_*` method calls are replaced with
`results.analysis.apply_*` method calls, which are equivalent.
- `Direction::visit_results_in_block` is simpler, with a single generic
param (`A`) instead of two (`D` and `R`/`F`, with a bound connecting
them). Likewise for `visit_results`.
- The `ResultsVisitor` impls for `MirBorrowCtxt` and
`StorageConflictVisitor` are now specific about the type of the
analysis results they work with. They both used to have a type param
`R` but they weren't genuinely generic. In both cases there was only a
single results type that made sense to instantiate them with.
We only need to take action when the next block cannot be added to the current
chain, but the logic is much simpler if we express it in terms of when the
block _can_ be added.
continue `TypingMode` refactor
There are still quite a few places which (indirectly) rely on the `Reveal` of a `ParamEnv`, but we're slowly getting there
r? `@compiler-errors`
Mark `simplify_aggregate_to_copy` mir-opt as unsound
Mark the `simplify_aggregate_to_copy` mir-opt added in #128299 as unsound as it seems to miscompile the MCVE reported in https://github.com/rust-lang/rust/issues/132353. The mir-opt can be re-enabled once this case is fixed.
```rs
fn pop_min(mut score2head: Vec<Option<usize>>) -> Option<usize> {
loop {
if let Some(col) = score2head[0] {
score2head[0] = None;
return Some(col);
}
}
}
fn main() {
let min = pop_min(vec![Some(1)]);
println!("min: {:?}", min);
// panic happens here on beta in release mode
// but not in debug mode
min.unwrap();
}
```
This MCVE is included as a `run-pass` ui regression test in the first commit. I built the ui test with a nightly manually, and can reproduce the behavioral difference with `-C opt-level=0` and `-C opt-level=1`. Locally, this ui test will fail unless it was run on a compiler built with the second commit marking the mir-opt as unsound thus disabling it by default.
This PR **partially reverts** commit e7386b3, reversing changes made to 02b1be1. The mir-opt implementation is just marked as unsound but **not** reverted to make reland reviews easier. Test changes are **reverted if they were not pure additions**. Tests added by the original PR received `-Z unsound-mir-opts` compile-flags.
cc `@DianQK` `@cjgillot` (PR author and reviewer of #128299)
They represent a lot of abstraction and indirection, but they're only
used for `ConstAnalysis`, and apparently won't be used for any other
analyses in the future. This commit inlines and removes them, which
makes `ConstAnalysis` easier to read and understand.
Rename `rustc_abi::Abi` to `BackendRepr`
Remove the confabulation of `rustc_abi::Abi` with what "ABI" actually means by renaming it to `BackendRepr`, and rename `Abi::Aggregate` to `BackendRepr::Memory`. The type never actually represented how things are passed, as that has to have `PassMode` considered, at minimum, but rather it just is how we represented some things to the backend. This conflation arose because LLVM, the primary backend at the time, would lower certain IR forms using certain ABIs. Even that only somewhat was true, as it broke down when one ventured significantly afield of what is described by the System V AMD64 ABI either by using different architectures, ABI-modifying IR annotations, the same architecture **with different ISA extensions enabled**, or other... unexpected delights.
Unfortunately both names are still somewhat of a misnomer right now, as people have written code for years based on this misunderstanding. Still, their original names are even moreso, and for better or worse, this backend code hasn't received as much maintenance as the rest of the compiler, lately. Actually arriving at a correct end-state will simply require us to disentangle a lot of code in order to fix, much of it pointlessly repeated in several places. Thus this is not an "actual fix", just a way to deflect further misunderstandings.
This is a standard pattern:
```
MyAnalysis.into_engine(tcx, body).iterate_to_fixpoint()
```
`into_engine` and `iterate_to_fixpoint` are always called in pairs, but
sometimes with a builder-style `pass_name` call between them. But a
builder-style interface is overkill here. This has been bugging me a for
a while.
This commit:
- Merges `Engine::new` and `Engine::iterate_to_fixpoint`. This removes
the need for `Engine` to have fields, leaving it as a trivial type
that the next commit will remove.
- Renames `Analysis::into_engine` as `Analysis::iterate_to_fixpoint`,
gives it an extra argument for the optional pass name, and makes it
call `Engine::iterate_to_fixpoint` instead of `Engine::new`.
This turns the pattern from above into this:
```
MyAnalysis.iterate_to_fixpoint(tcx, body, None)
```
which is shorter at every call site, and there's less plumbing required
to support it.
The initial naming of "Abi" was an awful mistake, conveying wrong ideas
about how psABIs worked and even more about what the enum meant.
It was only meant to represent the way the value would be described to
a codegen backend as it was lowered to that intermediate representation.
It was never meant to mean anything about the actual psABI handling!
The conflation is because LLVM typically will associate a certain form
with a certain ABI, but even that does not hold when the special cases
that actually exist arise, plus the IR annotations that modify the ABI.
Reframe `rustc_abi::Abi` as the `BackendRepr` of the type, and rename
`BackendRepr::Aggregate` as `BackendRepr::Memory`. Unfortunately, due to
the persistent misunderstandings, this too is now incorrect:
- Scattered ABI-relevant code is entangled with BackendRepr
- We do not always pre-compute a correct BackendRepr that reflects how
we "actually" want this value to be handled, so we leave the backend
interface to also inject various special-cases here
- In some cases `BackendRepr::Memory` is a "real" aggregate, but in
others it is in fact using memory, and in some cases it is a scalar!
Our rustc-to-backend lowering code handles this sort of thing right now.
That will eventually be addressed by lifting duplicated lowering code
to either rustc_codegen_ssa or rustc_target as appropriate.
coverage: Don't rely on the custom traversal to find enclosing loops
This opens up the possibility of modifying or removing the custom graph traversal used in coverage counter creation, without losing access to the heuristics that care about a node's enclosing loops.
Actually changing the traversal is left for future work, because this PR on its own doesn't change the emitted coverage mappings at all.
Effects cleanup
- removed extra bits from predicates queries that are no longer needed in the new system
- removed the need for `non_erasable_generics` to take in tcx and DefId, removed unused arguments in callers
r? compiler-errors
- removed extra bits from predicates queries that are no longer needed in the new system
- removed the need for `non_erasable_generics` to take in tcx and DefId, removed unused arguments in callers
Then we can rename the _raw functions to drop their suffix, and instead
explicitly use is_stable_const_fn for the few cases where that is really what
you want.
Move `cmp_in_dominator_order` out of graph dominator computation
Dominator-order information is only needed for coverage graphs, and is easy enough to collect by just traversing the graph again.
This avoids wasted work when computing graph dominators for any other purpose.
Dominator-order information is only needed for coverage graphs, and is easy
enough to collect by just traversing the graph again.
This avoids wasted work when computing graph dominators for any other purpose.
coverage: Make counter creation handle node/edge counters more uniformly
Similar to #130380, this is another round of small improvements informed by my ongoing attempts to overhaul coverage counter creation.
One of the big benefits is getting rid of the awkward special-case that would sometimes attach an edge counter to a node instead. That was needed by the code that chooses which out-edge should be given a counter expression, but we can avoid that by making the corresponding check a little smarter.
I've also renamed several things to be simpler and more consistent, which should help with future changes.
Continue to get rid of `ty::Const::{try_}eval*`
This PR mostly does:
* Removes all of the `try_eval_*` and `eval_*` helpers from `ty::Const`, and replace their usages with `try_to_*`.
* Remove `ty::Const::eval`.
* Rename `ty::Const::normalize` to `ty::Const::normalize_internal`. This function is still used in the normalization code itself.
* Fix some weirdness around the `TransmuteFrom` goal.
I'm happy to split it out further; for example, I could probably land the first part which removes the helpers, or the changes to codegen which are more obvious than the changes to tools.
r? BoxyUwU
Part of https://github.com/rust-lang/rust/issues/130704
Dont ICE when computing coverage of synthetic async closure body
I'm not totally certain if this is *right*, but at least it doesn't ICE.
The issue is that we end up generating two MIR bodies for each async closure, since the `FnOnce` and `Fn`/`FnMut` implementations have different borrowing behavior of their captured variables. They should ideally both contribute to the coverage, since those MIR bodies are (*to the user*) the same code and should have no behavioral differences.
This PR at least suppresses the ICEs, and then I guess worst case we can fix this the right way later.
r? Zalathar or re-roll
Fixes#131190
Make destructors on `extern "C"` frames to be executed
This would make the example in #123231 print "Noisy Drop". I didn't mark this as fixing the issue because the behaviour is yet to be spec'ed.
Tracking:
- https://github.com/rust-lang/rust/issues/74990
Don't check unsize goal in MIR validation when opaques remain
Similarly to `mir_assign_valid_types`, let's just skip when there are opaques. Fixes#130921.
coverage: Multiple small tweaks to counter creation
I've been experimenting with some larger changes to how coverage counters are assigned to parts of the control-flow graph, and while none of that is ready yet, along the way I've repeatedly found myself wanting these smaller tweaks as a base.
There are no changes to compiler output.
Don't use Immediate::offset to transmute pointers to integers
This applies the relatively new `assert_matches_abi` check in the `offset` operation on immediates, which makes sure that if offsets are used to alter the layout (which is possible because the field layout is arbitrarily picked by the caller), this is not done in a way that breaks the invariant of the `Immediate` type.
This leads to ICEs in a GVN mir-opt test, so the second commit fixes GVN.
Fixes https://github.com/rust-lang/rust/issues/131064.
This makes it possible for other parts of counter-assignment to check whether a
node is guaranteed to end up with some kind of counter.
Switching from `impl Fn` to a concrete `&BitSet` just avoids the hassle of
trying to store a closure in a struct field, and currently there's no
foreseeable need for this information to not be a bitset.
Disable jump threading `UnOp::Not` for non-bool
Fix#131195, where jumpthreading was optimizing `!a == b` into `a != b` for non-bool, where this is definitely not true.
This code can sometimes witness malformed coverage attributes in builds that
are going to fail, so use `span_delayed_bug` to avoid an inappropriate ICE in
that case.
Add `File` constructors that return files wrapped with a buffer
In addition to the light convenience, these are intended to raise visibility that buffering is something you should consider when opening a file, since unbuffered I/O is a common performance footgun to Rust newcomers.
ACP: https://github.com/rust-lang/libs-team/issues/446
Tracking Issue: #130804
Apply `EarlyOtherwiseBranch` to scalar value
In the future, I'm thinking of hoisting discriminant via GVN so that we only need to write very little code here.
r? `@cjgillot`
Encode `coroutine_by_move_body_def_id` in crate metadata
We synthesize the MIR for a by-move body for the `FnOnce` implementation of async closures. It can be accessed with the `coroutine_by_move_body_def_id` query. We weren't encoding this query in the metadata though, nor were we properly recording that synthetic MIR in `mir_keys`, so the `optimized_mir` wasn't getting encoded either!
Stacked on top is a fix to consider `DefKind::SyntheticCoroutineBody` to return true in several places I missed. Specifically, we should consider the def-kind in `fn DefKind::is_fn_like()`, since that's what we were using to make sure we ensure `query mir_inliner_callees` before the MIR gets stolen for the body. This led to some CI failures that were caught by miri but which I added a test for.
Remove semi-nondeterminism of `DefPathHash` ordering from inliner
Déjà vu or something because I kinda thought I had put this PR up before. I recall a discussion somewhere where I think it was `@saethlin` mentioning that this check was no longer needed since we have "proper" cycle detection. Putting that up as a PR now.
This may slighlty negatively affect inlining, since the cycle breaking here means that we still inlined some cycles when the def path hashes were ordered in certain ways, this leads to really bad nondeterminism that makes minimizing ICEs and putting up inliner bugfixes difficult.
r? `@cjgillot` or `@saethlin` or someone else idk
coverage: Clarify some parts of coverage counter creation
This is a series of semi-related changes that are trying to make the `counters` module easier to read, understand, and modify.
For example, the existing code happens to avoid ever using the count for a `TerminatorKind::Yield` node as the count for its sole out-edge (since doing so would be incorrect), but doesn't do so explicitly, so seemingly-innocent changes can result in baffling test failures.
This PR also takes the opportunity to simplify some debug-logging code that was making its surrounding code disproportionately hard to read.
There should be no changes to the resulting coverage instrumentation/mappings, as demonstrated by the absence of changes to the coverage test suite.
Given that we directly access the graph predecessors/successors in so many
other places, and sometimes must do so to satisfy the borrow checker, there is
little value in having this trivial helper method.
- Look up the node's predecessors only once
- Get rid of some overly verbose logging
- Explain why some nodes need physical counters
- Extract a helper method to create and set a physical node counter
Simplify the canonical clone method and the copy-like forms to copy
Fixes#128081.
The optimized clone method ends up as the following MIR:
```
_2 = copy ((*_1).0: i32);
_3 = copy ((*_1).1: u64);
_4 = copy ((*_1).2: [i8; 3]);
_0 = Foo { a: move _2, b: move _3, c: move _4 };
```
We can transform this to:
```
_0 = copy (*_1);
```
r? `@cjgillot`
Don't call closure_by_move_body_def_id on FnOnce async closures in MIR validation
Refactors the check in #129847 to not unncessarily call the `closure_by_move_body_def_id` query for async closures that don't *need* a by-move body.
Fixes#130167
- Replace non-standard names like 's, 'p, 'rg, 'ck, 'parent, 'this, and
'me with vanilla 'a. These are cases where the original name isn't
really any more informative than 'a.
- Replace names like 'cx, 'mir, and 'body with vanilla 'a when the lifetime
applies to multiple fields and so the original lifetime name isn't
really accurate.
- Put 'tcx last in lifetime lists, and 'a before 'b.
coverage: Simplify creation of sum counters
A small and self-contained improvement, extracted from some larger changes that I'm still working on.
Ultimately I want to avoid creating these sum counter-expressions in some cases (in favour of just adding physical counters directly to the nodes we care about), so a good incremental move towards that is splitting the “gather edge counters” step out from the ”build a sum of those counters” step.
Creating an extra intermediate vector should have negligible cost (and coverage isn't exercised by the benchmark suite anyway). The removed logging is redundant with the `#[instrument(..)]` logging we already have on the underlying method calls.
some const cleanup: remove unnecessary attributes, add const-hack indications
I learned that we use `FIXME(const-hack)` on top of the "const-hack" label. That seems much better since it marks the right place in the code and moves around with the code. So I went through the PRs with that label and added appropriate FIXMEs in the code. IMO this means we can then remove the label -- Cc ``@rust-lang/wg-const-eval.``
I also noticed some const stability attributes that don't do anything useful, and removed them.
r? ``@fee1-dead``
coverage: Clean up terminology in counter creation
Some of the terminology in this module is confusing, or has drifted out of sync with other parts of the coverage code.
This PR therefore renames some variables and methods, and adjusts comments and debug logging statements, to make things clearer and more consistent.
No functional changes, other than some small tweaks to debug logging.
In all cases the struct can own the relevant thing instead of having a
reference to it. This makes the code simpler, and in some cases removes
a struct lifetime.
These are all functions with a single callsite, where having a separate
function does nothing to help with readability. These changes make the
code a little shorter and easier to read.
There are four related dataflow structs: `MaybeInitializedPlaces`,
`MaybeUninitializedPlaces`, and `EverInitializedPlaces`,
`DefinitelyInitializedPlaces`. They all have a `&Body` and a
`&MoveData<'tcx>` field. The first three use different lifetimes for the
two fields, but the last one uses the same lifetime for both.
This commit changes the first three to use the same lifetime, removing
the need for one of the lifetimes. Other structs that also lose a
lifetime as a result of this are `LivenessContext`, `LivenessResults`,
`InitializationData`.
It then does similar things in various other structs.
Currently it constructs two vectors `calls_to_terminated` and
`cleanups_to_remove` in the main loop, and then processes them after the
main loop. But the processing can be done in the main loop, avoiding the
need for the vectors.
Do not call query to compute coroutine layout for synthetic body of async closure
There is code in the MIR validator that attempts to prevent query cycles when inlining a coroutine into itself, and will use the coroutine layout directly from the body when it detects that's the same coroutine as the one that's being validated. After #128506, this logic didn't take into account the fact that the coroutine def id will differ if it's the "by-move body" of an async closure. This PR implements that.
Fixes#129811
coverage: Count await when the Future is immediately ready
Currently `await` is only counted towards coverage if the containing
function is suspended and resumed at least once. This is because it
expands to code which contains a branch on the discriminant of `Poll`.
By treating it like a branching macro (e.g. `assert!`), these
implementation details will be hidden from the coverage results.
I added a test to ensure the fix works in simple cases, but the heuristic of picking only the first await-related covspan might be unreliable. I plan on testing more thoroughly with a real codebase over the next couple of weeks.
closes#98712
Make `Ty::boxed_ty` return an `Option`
Looks like a good place to use Rust's type system.
---
Most of 4ac7bcbaad/compiler/rustc_middle/src/ty/sty.rs (L971-L1963) looks like it could be moved to `TyKind` (then I guess `Ty` should be made to deref to `TyKind`).
Currently `await` is only counted towards coverage if the containing
function is suspended and resumed at least once. This is because it
expands to code which contains a branch on the discriminant of `Poll`.
By treating it like a branching macro (e.g. `assert!`), these
implementation details will be hidden from the coverage results.
Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir
First, we add a missing match for `DefKind::SyntheticCoroutineBody` in `dump_mir`. Fixes#129703. The second commit (directly below) serves as a test.
Second, we reorder the `dump_mir` in `coroutine_by_move_body_def_id` to be *after* we adjust the body source, and change the disambiguator so it reads more like any other MIR body. This also serves as a test for the ICE, since we're dumping the MIR of a body with `DefKind::SyntheticCoroutineBody`.
Third, we change the parenting of the synthetic MIR body to have the *coroutine-closure* (i.e. async closure) as its parent, so we don't have long strings of `{closure#0}-{closure#0}-{closure#0}`.
try-job: test-various
Move `SanityCheck` and `MirPass`
They are currently in `rustc_middle`. This PR moves them to `rustc_mir_transform`, which makes more sense.
r? ``@cjgillot``
Because that's now the only crate that uses it.
Moving stuff out of `rustc_middle` is always welcome.
I chose to use `impl crate::MirPass`/`impl crate::MirLint` (with
explicit `crate::`) everywhere because that's the only mention of
`MirPass`/`MirLint` used in all of these files. (Prior to this change,
`MirPass` was mostly imported via `use rustc_middle::mir::*` items.)
The actual implementation remains in `rustc_mir_dataflow`, but this
commit moves the `MirPass` impl to `rustc_mir_transform` and changes it
to a `MirLint` (fixing a `FIXME` comment).
(I originally tried moving the full implementation from
`rustc_mir_dataflow` but I had some trait problems with `HasMoveData`
and `RustcPeekAt` and `MaybeLiveLocals`. This commit was much smaller
and simpler, but still will allow some follow-up cleanups.)
