Current `SwitchInt` handling has complicated control flow.
- The dataflow engine calls `Analysis::apply_switch_int_edge_effects`,
passing in an "applier" that impls `SwitchIntEdgeEffects`.
- `apply_switch_int_edge_effects` possibly calls `apply` on the applier,
passing it a closure.
- The `apply` method calls the closure on each `SwitchInt` edge.
- The closure operates on the edge.
I.e. control flow goes from the engine, to the analysis, to the applier
(which came from the engine), to the closure (which came from the
analysis). It took me a while to work this out.
This commit changes to a simpler structure that maintains the important
characteristics.
- The dataflow engine calls `Analysis::get_switch_int_data`.
- `get_switch_int_data` returns an `Option<Self::SwitchIntData>` value.
- If that returned value was `Some`, the dataflow engine calls
`Analysis::apply_switch_int_edge_effect` on each edge, passing the
`Self::SwitchIntData` value.
- `Analysis::apply_switch_int_edge_effect` operates on the edge.
I.e. control flow goes from the engine, to the analysis, to the
engine, to the analysis.
Added:
- The `Analysis::SwitchIntData` assoc type and the
`Analysis::get_switch_int_data` method. Both only need to be
defined by analyses that look at `SwitchInt` terminators.
- The `MaybePlacesSwitchIntData` struct, which has three fields.
Changes:
- `Analysis::apply_switch_int_edge_effects` becomes
`Analysis::apply_switch_int_edge_effect`, which is a little simpler
because it's dealing with a single edge instead of all edges.
Removed:
- The `SwitchIntEdgeEffects` trait, and its two impls:
`BackwardSwitchIntEdgeEffectsApplier` (which has six fields) and
`ForwardSwitchIntEdgeEffectsApplier` structs (which has four fields).
- The closure.
The new structure is more concise and simpler.
In most places, the `early` method is listed before the corresponding
`primary` method, like you'd expect. This commit fixes two places where
that isn't the case.
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 just uses `BitSet` for small/medium sizes (<= 2048 bits) and
`ChunkedBitSet` for larger sizes. This is good because `ChunkedBitSet`
is slow and memory-hungry at smaller sizes.
As of #133155 `Formatter:new` uses `as_results_cursor` to create a
non-mutable results reference, and then later that is accessed via
`deref_mut` which results in a runtime abort. Changing to
`as_results_cursor_mut` fixes it.
Fixes#133641.
Remove the `DefinitelyInitializedPlaces` analysis.
Its only use is in the `tests/ui/mir-dataflow/def_inits-1.rs` where it is tested via `rustc_peek_definite_init`.
Also, it's probably buggy. It's supposed to be the inverse of `MaybeUninitializedPlaces`, and it mostly is, except that `apply_terminator_effect` is a little different, and `apply_switch_int_edge_effects` is missing. Unlike `MaybeUninitializedPlaces`, which is used extensively in borrow checking, any bugs in `DefinitelyInitializedPlaces` are easy to overlook because it is only used in one small test.
This commit removes the analysis. It also removes
`rustc_peek_definite_init`, `Dual` and `MeetSemiLattice`, all of which are no longer needed.
r? ``@cjgillot``
`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!
Its only use is in the `tests/ui/mir-dataflow/def_inits-1.rs` where it
is tested via `rustc_peek_definite_init`.
Also, it's probably buggy. It's supposed to be the inverse of
`MaybeUninitializedPlaces`, and it mostly is, except that
`apply_terminator_effect` is a little different, and
`apply_switch_int_edge_effects` is missing. Unlike
`MaybeUninitializedPlaces`, which is used extensively in borrow
checking, any bugs in `DefinitelyInitializedPlaces` are easy to overlook
because it is only used in one small test.
This commit removes the analysis. It also removes
`rustc_peek_definite_init`, `Dual` and `MeetSemiLattice`, all of which
are no longer needed.
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.
The results of most analyses end up in a `Results<'tcx, A>`, where `A`
is the analysis. It's then possible to traverse the results via a
`ResultsVisitor`, which relies on the `ResultsVisitable` trait. (That
trait ends up using the same `apply_*` methods that were used when
computing the analysis, albeit indirectly.)
This pattern of "compute analysis results, then visit them" is common.
But there is one exception. For borrow checking we compute three
separate analyses (`Borrows`, `MaybeUninitializedPlaces`, and
`EverInitializedPlaces`), combine them into a single `BorrowckResults`,
and then do a single visit of that `BorrowckResults` with
`MirBorrowckResults`. `BorrowckResults` is just different enough from
`Results` that it requires the existence of `ResultsVisitable`, which
abstracts over the traversal differences between `Results` and
`BorrowckResults`.
This commit changes things by introducing `Borrowck` and bundling the
three borrowck analysis results into a standard `Results<Borrowck>`
instead of the exceptional `BorrowckResults`. Once that's done, the
results can be visited like any other analysis results.
`BorrowckResults` is removed, as is `impl ResultsVisitable for
BorrowckResults`. (It's instructive to see how similar the added `impl
Analysis for Borrowck` is to the removed `impl ResultsVisitable for
BorrowckResults`. They're both doing exactly the same things.)
Overall this increases the number of lines of code and might not seem
like a win. But it enables the removal of `ResultsVisitable` in the next
commit, which results in many simplifications.
It's no longer needed. `Engine::iterate_to_fixpoint` can be inlined into
`Analysis::iterate_to_fixpoint` and removed. The commit also renames
`engine.rs` as `results.rs`.
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.
`GenKillAnalysis` has very similar methods to `Analysis`, but the first
two have a notable difference: the second argument is `&mut impl
GenKill<Self::Idx>` instead of `&mut Self::Domain`. But thanks to the
previous commit, this difference is no longer necessary.
This is an alternative to `Engine::new_generic` for gen/kill analyses.
It's supposed to be an optimization, but it has negligible effect.
The commit merges `Engine::new_generic` into `Engine::new`.
This allows the removal of various other things: `GenKillSet`,
`gen_kill_statement_effects_in_block`, `is_cfg_cyclic`.
`GenKillAnalysis` has five methods that take a transfer function arg:
- `statement_effect`
- `before_statement_effect`
- `terminator_effect`
- `before_terminator_effect`
- `call_return_effect`
All the transfer function args have type `&mut impl GenKill<Self::Idx>`,
except for `terminator_effect`, which takes the simpler `Self::Domain`.
But only the first two need to be `impl GenKill`. The other
three can all be `Self::Domain`, just like `Analysis`. So this commit
changes the last two to take `Self::Domain`, making `GenKillAnalysis`
and `Analysis` more similar.
(Another idea would be to make all these methods `impl GenKill`. But
that doesn't work: `MaybeInitializedPlaces::terminator_effect` requires
the arg be `Self::Domain` so that `self_is_unwind_dead(place, state)`
can be called on it.)
They both now only ever contain a `Results<'tcx, A>`.
This means `AnalysisResults` can be removed, as can many
`borrow`/`borrow_mut` calls. Also `Results` no longer needs a
`PhantomData` because `'tcx` is now named by `entry_sets`.
It's only implemented for analyses that implement `Copy`, which means
it's basically a complicated synonym for `Copy`. So this commit removes
it and uses `Copy` directly. (That direct use will be removed in a later
commit.)
