Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rustc_mir_dataflow cleanups, including some renamings #133938

Merged
merged 9 commits into from
Dec 14, 2024

Conversation

nnethercote
Copy link
Contributor

Some opinionated commits in this collection, let's see how we go.

r? @cjgillot

@rustbot rustbot 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 Dec 5, 2024
@nnethercote
Copy link
Contributor Author

This will have some conflicts with #133328, but I can resolve them if/when necessary.

@nnethercote nnethercote marked this pull request as ready for review December 5, 2024 23:28
@rustbot
Copy link
Collaborator

rustbot commented Dec 5, 2024

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@bors
Copy link
Contributor

bors commented Dec 9, 2024

☔ The latest upstream changes (presumably #133891) made this pull request unmergeable. Please resolve the merge conflicts.

They are only present because it's currently defined in terms of the
domains of `Borrows` and `MaybeUninitializedPlaces` and
`EverInitializedPlaces` via associated types. This commit introduces
typedefs for those domains, avoiding the lifetimes.
Via the `Analysis::Domain` associated types, instead of the direct type
name.
Currently they are called (most common) `state`, or `trans`, or (rare)
`on_entry`. I think `trans` is short for "transfer function", which
perhaps made more sense when `GenKillAnalysis` existed. Using `state`
everywhere now is more consistent.
"Set" doesn't make much sense here, we refer to domain values as "state"
everywhere else. (This name confused me for a while.)
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`.
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.)
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.
@nnethercote nnethercote force-pushed the rustc_mir_dataflow-renamings branch from 783ac4f to dddc09d Compare December 10, 2024 01:16
@nnethercote
Copy link
Contributor Author

I rebased.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 13, 2024

r? @oli-obk

bors r plus

@bors
Copy link
Contributor

bors commented Dec 13, 2024

📌 Commit dddc09d has been approved by oli-obk

It is now in the queue for this repository.

@rustbot rustbot assigned oli-obk and unassigned cjgillot Dec 13, 2024
@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 Dec 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 13, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#133900 (Advent of `tests/ui` (misc cleanups and improvements) [1/N])
 - rust-lang#133937 (Keep track of parse errors in `mod`s and don't emit resolve errors for paths involving them)
 - rust-lang#133938 (`rustc_mir_dataflow` cleanups, including some renamings)
 - rust-lang#134058 (interpret: reduce usage of TypingEnv::fully_monomorphized)
 - rust-lang#134130 (Stop using driver queries in the public API)
 - rust-lang#134140 (Add AST support for unsafe binders)
 - rust-lang#134229 (Fix typos in docs on provenance)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 13, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#133900 (Advent of `tests/ui` (misc cleanups and improvements) [1/N])
 - rust-lang#133937 (Keep track of parse errors in `mod`s and don't emit resolve errors for paths involving them)
 - rust-lang#133938 (`rustc_mir_dataflow` cleanups, including some renamings)
 - rust-lang#134058 (interpret: reduce usage of TypingEnv::fully_monomorphized)
 - rust-lang#134130 (Stop using driver queries in the public API)
 - rust-lang#134140 (Add AST support for unsafe binders)
 - rust-lang#134229 (Fix typos in docs on provenance)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c6ebe6f into rust-lang:master Dec 14, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
Rollup merge of rust-lang#133938 - nnethercote:rustc_mir_dataflow-renamings, r=oli-obk

`rustc_mir_dataflow` cleanups, including some renamings

Some opinionated commits in this collection, let's see how we go.

r? `@cjgillot`
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 15, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#133900 (Advent of `tests/ui` (misc cleanups and improvements) [1/N])
 - rust-lang#133937 (Keep track of parse errors in `mod`s and don't emit resolve errors for paths involving them)
 - rust-lang#133938 (`rustc_mir_dataflow` cleanups, including some renamings)
 - rust-lang#134058 (interpret: reduce usage of TypingEnv::fully_monomorphized)
 - rust-lang#134130 (Stop using driver queries in the public API)
 - rust-lang#134140 (Add AST support for unsafe binders)
 - rust-lang#134229 (Fix typos in docs on provenance)

r? `@ghost`
`@rustbot` modify labels: rollup
@nnethercote nnethercote deleted the rustc_mir_dataflow-renamings branch December 15, 2024 21:56
github-merge-queue bot pushed a commit to model-checking/kani that referenced this pull request Dec 16, 2024
Culprit PRs:
- rust-lang/rust#133938, specifically
rust-lang/rust@1d56943
- rust-lang/rust#134295

For coroutine closures, I opened #3783 to track feature support--adding
support for this appears non-trivial, and I didn't want to block
toolchain upgrades on it.

Resolves #3781

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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