Skip to content

Conversation

nnethercote
Copy link
Contributor

Analysis gets put into Results with EntryStates, by iterate_to_fixpoint. This has two problems:

  • Results is passed various places where only Analysis is needed.
  • EntryStates is passed around mutably everywhere even though it is immutable.

This commit mostly separates Analysis from Results and fixes these two problems.

r? @davidtwco

Every `Results` contains an `Analysis`, but these methods only need the
`Analysis`. No point passing them more data than they need.
`Results` contains and `Analysis` and an `EntryStates`. The unfortunate
thing about this is that the analysis needs to be mutable everywhere
(`&mut Analysis`) which forces the `Results` to be mutable everywhere,
even though `EntryStates` is immutable everywhere.

To fix this, this commit renames `Results` as `AnalysisAndResults`,
renames `EntryStates` as `Results`, and separates the analysis and
results as much as possible. (`AnalysisAndResults` doesn't get much use,
it's mostly there to facilitate method chaining of
`iterate_to_fixpoint`.)

`Results` is immutable everywhere, which:
- is a bit clearer on how the data is used,
- avoids an unnecessary clone of entry states in
  `locals_live_across_suspend_points`, and
- moves the results outside the `RefCell` in Formatter.

The commit also reformulates `ResultsHandle` as the generic `CowMut`,
which is simpler than `ResultsHandle` because it doesn't need the
`'tcx` lifetime and the trait bounds. It also which sits nicely
alongside the new use of `Cow` in `ResultsCursor`.
@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide 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 Apr 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 24, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

@nnethercote
Copy link
Contributor Author

@Jarcho: I hope this doesn't interfere with the uncommitted clippy analyses that you have. From our previous conversation, they required Analysis to be mutable and this PR doesn't change that.

@Jarcho
Copy link
Contributor

Jarcho commented Apr 24, 2025

This doesn't look like it will interfere with anything I have.

@davidtwco
Copy link
Member

@bors r+

Apologies for the delay in getting to this

@bors
Copy link
Collaborator

bors commented May 7, 2025

📌 Commit 92799b6 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors 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 May 7, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request May 7, 2025
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#140234 (Separate dataflow analysis and results)
 - rust-lang#140614 (Correct warning message in restricted visibility)
 - rust-lang#140671 (Parser: Recover error from named params while parse_path)
 - rust-lang#140700 (Don't crash on error codes passed to `--explain` which exceed our internal limit of 9999 )
 - rust-lang#140706 ([rustdoc] Ensure that temporary doctest folder is correctly removed even if doctests failed)
 - rust-lang#140734 (Fix regression from rust-lang#140393 for espidf / horizon / nuttx / vita)
 - rust-lang#140741 (add armv5te-unknown-linux-gnueabi target maintainer)
 - rust-lang#140745 (run-make-support: set rustc dylib path for cargo wrapper)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 7, 2025
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#140234 (Separate dataflow analysis and results)
 - rust-lang#140614 (Correct warning message in restricted visibility)
 - rust-lang#140671 (Parser: Recover error from named params while parse_path)
 - rust-lang#140700 (Don't crash on error codes passed to `--explain` which exceed our internal limit of 9999 )
 - rust-lang#140706 ([rustdoc] Ensure that temporary doctest folder is correctly removed even if doctests failed)
 - rust-lang#140734 (Fix regression from rust-lang#140393 for espidf / horizon / nuttx / vita)
 - rust-lang#140741 (add armv5te-unknown-linux-gnueabi target maintainer)
 - rust-lang#140745 (run-make-support: set rustc dylib path for cargo wrapper)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 82c99c4 into rust-lang:master May 8, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone May 8, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 8, 2025
Rollup merge of rust-lang#140234 - nnethercote:separate-Analysis-and-Results, r=davidtwco

Separate dataflow analysis and results

`Analysis` gets put into `Results` with `EntryStates`, by `iterate_to_fixpoint`. This has two problems:
- `Results` is passed various places where only `Analysis` is needed.
- `EntryStates` is passed around mutably everywhere even though it is immutable.

This commit mostly separates `Analysis` from `Results` and fixes these two problems.

r? `@davidtwco`
@nnethercote nnethercote deleted the separate-Analysis-and-Results branch May 8, 2025 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustc-dev-guide Area: rustc-dev-guide S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant 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.

5 participants