Skip to content

Yet more rustc_mir_dataflow cleanups - #133155

Merged
bors merged 5 commits into
rust-lang:masterfrom
nnethercote:yet-more-rustc_mir_dataflow-cleanups
Nov 27, 2024
Merged

Yet more rustc_mir_dataflow cleanups#133155
bors merged 5 commits into
rust-lang:masterfrom
nnethercote:yet-more-rustc_mir_dataflow-cleanups

Conversation

@nnethercote

Copy link
Copy Markdown
Contributor

A few more cleanups.

r? @cjgillot

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2024
@rustbot

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt


const IS_BACKWARD: bool = !Self::IS_FORWARD;

fn apply_effects_in_block_and_join_state_into_successors_of<'mir, 'tcx, A>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you shorten the name?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'll just use apply_effects_in_block, I think that's good enough.

@cjgillot

Copy link
Copy Markdown
Contributor

Thanks. r=me with a naming nit

They are always called in succession, so it's simpler if they are merged
into a single function.
Describing some things that took me a long time to understand.
In `MaybeRequiresStorage::apply_before_statement_effect`, call
`transfer_function` directly, as is already done in
`MaybeRequiresStorage::apply_before_terminator_effect`. This makes it clear
that the operation doesn't rely on the `MaybeBorrowedLocals` results.
`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 rust-lang#118230 -- which
removed the old `ResultsRefCursor` and `ResultsCloneCursor` types -- but
in a much simpler way. Hooray!
@nnethercote
nnethercoteforce-pushed the yet-more-rustc_mir_dataflow-cleanups branch from a47e638 to be7c6a3CompareNovember 26, 2024 00:30
@nnethercote

Copy link
Copy Markdown
ContributorAuthor

I updated the name.

@bors r=cjgillot

@bors

bors commented Nov 26, 2024

Copy link
Copy Markdown
Collaborator

📌 Commit be7c6a3 has been approved by cjgillot

It is now in the queue for this repository.

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 26, 2024
…mpiler-errors
Rollup of 12 pull requests
Successful merges:
- rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`)
- rust-lang#133070 (Lexer tweaks)
- rust-lang#133136 (Support ranges in `<[T]>::get_many_mut()`)
- rust-lang#133140 (Inline ExprPrecedence::order into Expr::precedence)
- rust-lang#133155 (Yet more `rustc_mir_dataflow` cleanups)
- rust-lang#133282 (Shorten the `MaybeUninit` `Debug` implementation)
- rust-lang#133326 (Remove the `DefinitelyInitializedPlaces` analysis.)
- rust-lang#133362 (No need to re-sort existential preds in relate impl)
- rust-lang#133367 (Simplify array length mismatch error reporting (to not try to turn consts into target usizes))
- rust-lang#133394 (Bail on more errors in dyn ty lowering)
- rust-lang#133410 (target check_consistency: ensure target feature string makes some basic sense)
- rust-lang#133435 (miri: disable test_downgrade_observe test on macOS)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit f010e2d into rust-lang:masterNov 27, 2024
@rustbotrustbot added this to the 1.85.0 milestone Nov 27, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2024
Rollup merge of rust-lang#133155 - nnethercote:yet-more-rustc_mir_dataflow-cleanups, r=cjgillot
Yet more `rustc_mir_dataflow` cleanups
A few more cleanups.
r? `@cjgillot`
@nnethercote
nnethercote deleted the yet-more-rustc_mir_dataflow-cleanups branch November 27, 2024 03:35
nnethercote added a commit to nnethercote/rust that referenced this pull request Dec 2, 2024
As of rust-lang#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.
Fixesrust-lang#133641.
@matthiaskrgrmatthiaskrgr mentioned this pull request Dec 2, 2024
github-actionsBot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
…mpiler-errors
Rollup of 12 pull requests
Successful merges:
- rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`)
- rust-lang#133070 (Lexer tweaks)
- rust-lang#133136 (Support ranges in `<[T]>::get_many_mut()`)
- rust-lang#133140 (Inline ExprPrecedence::order into Expr::precedence)
- rust-lang#133155 (Yet more `rustc_mir_dataflow` cleanups)
- rust-lang#133282 (Shorten the `MaybeUninit` `Debug` implementation)
- rust-lang#133326 (Remove the `DefinitelyInitializedPlaces` analysis.)
- rust-lang#133362 (No need to re-sort existential preds in relate impl)
- rust-lang#133367 (Simplify array length mismatch error reporting (to not try to turn consts into target usizes))
- rust-lang#133394 (Bail on more errors in dyn ty lowering)
- rust-lang#133410 (target check_consistency: ensure target feature string makes some basic sense)
- rust-lang#133435 (miri: disable test_downgrade_observe test on macOS)
r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@nnethercote@rustbot@cjgillot@bors