Simplify `apply_effects_in_range` by nnethercote · Pull Request #159285 · rust-lang/rust

nnethercote · 2026-07-14T12:10:51Z

View all comments

Details in individual commits.

r? @cjgillot

nnethercote · 2026-07-14T12:11:07Z

Shouldn't affect perf, let's check:

@bors try @rust-timer queue

…r=<try> Simplify `apply_effects_in_range`

rust-bors · 2026-07-14T14:21:45Z

☀️ Try build successful (CI)
Build commit: 83bb0c9 (83bb0c96ef4d314870c0d145dff898c1e7326c9e)
Base parent: da80ed0 (da80ed0708a09dc096c184345d6eb42cbcd50a1e)

rust-timer · 2026-07-14T15:02:03Z

Finished benchmarking commit (83bb0c9): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

Cycles

Results (secondary -2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.4%	[-2.7%, -2.1%]	2
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 491.202s -> 491.32s (0.02%)
Artifact size: 389.32 MiB -> 389.43 MiB (0.03%)

nnethercote · 2026-07-14T21:35:34Z

Perf is neutral: the regressions are tiny and few and probably noise.

`ResultsVisitor` has `visit_block_start` which is called on entry to a a block in a forwards analysis and on exit from a block in a backwards analysis. And vice versa for `visit_block_end`. The only visitor that impls these methods is `StateDiffCollector`, which does something in `visit_block_start` for a forwards analysis and the same thing in `visit_block_end` for a backwards analysis. In other words, `StateDiffCollector` wants to always do the same thing on entry to a block and never do anything on exit from a block. This commit replaces `visit_block_{start,end}` with `visit_block_entry`, which is always called on entry to a block. This is simpler overall.

By adding more methods to `Direction`. This makes things more concise, and these new methods will be used more in subsequent commits.

This commit adds `Analysis::apply_effect`, which takes an `EffectIndex` and calls the appropriate `Analysis::apply_*` method. Once that is in place, it is possible to use it with `next_index` to write a simple `apply_effects_in_range` method that can be shared between `Forward` and `Backward`. The end result is much easier to understand.

rustbot · 2026-07-20T03:58:58Z

Some changes occurred in src/tools/cargo

cc @ehuss

rustbot · 2026-07-20T03:59:00Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

nnethercote · 2026-07-20T04:00:05Z

I have added two new commits that address the comments.

@rustbot ready

It's only used by `StateDiffCollector`, and it's just a complicated way to get the entry state, which can instead be done directly (avoiding the creation of a `bottom_value` which was immediately overwritten).

It has a single call site. The commit removes the assertions because they necessary any more due to the assertions and checks at the call site. This then removes the need for `index_precedes`.

nnethercote · 2026-07-20T04:30:52Z

Some changes occurred in src/tools/cargo
Some commits in this PR modify submodules.

These were accidental; I have reverted them.

cjgillot · 2026-07-22T01:15:42Z

Thanks!
@bors r+

rust-bors · 2026-07-22T01:15:45Z

📌 Commit e736635 has been approved by cjgillot

It is now in the queue for this repository.

@cjgillot

…_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

@cjgillot

…_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

…uwer Rollup of 8 pull requests Successful merges: - #159712 (Subtree sync for rustc_codegen_cranelift) - #155697 (Stabilize c-variadic function definitions) - #159285 (Simplify `apply_effects_in_range`) - #159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes) - #159659 (Move `Limit` out of `rustc_hir`) - #159451 (Remove config cloning in compiletest) - #159646 (Increase depth for float infer var fallback hack) - #159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)

@cjgillot

…_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

…uwer Rollup of 12 pull requests Successful merges: - #159712 (Subtree sync for rustc_codegen_cranelift) - #155697 (Stabilize c-variadic function definitions) - #159285 (Simplify `apply_effects_in_range`) - #159596 (unify the AST repr of type const and const RHS) - #159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes) - #159659 (Move `Limit` out of `rustc_hir`) - #159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern) - #158479 (Reject static item as direct const generic arg) - #158738 (next_trait_solver: Recover from GCE const exprs) - #159451 (Remove config cloning in compiletest) - #159646 (Increase depth for float infer var fallback hack) - #159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)

…uwer Rollup of 11 pull requests Successful merges: - #159712 (Subtree sync for rustc_codegen_cranelift) - #155697 (Stabilize c-variadic function definitions) - #159285 (Simplify `apply_effects_in_range`) - #159596 (unify the AST repr of type const and const RHS) - #159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes) - #159659 (Move `Limit` out of `rustc_hir`) - #159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern) - #158738 (next_trait_solver: Recover from GCE const exprs) - #159451 (Remove config cloning in compiletest) - #159646 (Increase depth for float infer var fallback hack) - #159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)

@cjgillot

Rollup merge of #159285 - nnethercote:simplify-apply_effects_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

…uwer Rollup of 11 pull requests Successful merges: - rust-lang/rust#159712 (Subtree sync for rustc_codegen_cranelift) - rust-lang/rust#155697 (Stabilize c-variadic function definitions) - rust-lang/rust#159285 (Simplify `apply_effects_in_range`) - rust-lang/rust#159596 (unify the AST repr of type const and const RHS) - rust-lang/rust#159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes) - rust-lang/rust#159659 (Move `Limit` out of `rustc_hir`) - rust-lang/rust#159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern) - rust-lang/rust#158738 (next_trait_solver: Recover from GCE const exprs) - rust-lang/rust#159451 (Remove config cloning in compiletest) - rust-lang/rust#159646 (Increase depth for float infer var fallback hack) - rust-lang/rust#159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)

rustbot assigned cjgillot Jul 14, 2026

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 Jul 14, 2026

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 14, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jul 14, 2026

Auto merge of #159285 - nnethercote:simplify-apply_effects_in_range, …

83bb0c9

…r=<try> Simplify `apply_effects_in_range`

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 14, 2026

cjgillot reviewed Jul 18, 2026

View reviewed changes

Comment thread compiler/rustc_mir_dataflow/src/framework/graphviz.rs Outdated

Comment thread compiler/rustc_mir_dataflow/src/framework/direction.rs Outdated

cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2026

nnethercote added 3 commits July 20, 2026 13:42

Avoid some IS_FORWARD tests

f2063f9

By adding more methods to `Direction`. This makes things more concise, and these new methods will be used more in subsequent commits.

nnethercote force-pushed the simplify-apply_effects_in_range branch from 3b49173 to cd61331 Compare July 20, 2026 03:58

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 20, 2026

nnethercote added 2 commits July 20, 2026 14:28

Eliminate ResultsVisitor::visit_block_entry

61b5db6

It's only used by `StateDiffCollector`, and it's just a complicated way to get the entry state, which can instead be done directly (avoiding the creation of a `bottom_value` which was immediately overwritten).

Inline and remove Direction::apply_effects_in_range

e736635

It has a single call site. The commit removes the assertions because they necessary any more due to the assertions and checks at the call site. This then removes the need for `index_precedes`.

nnethercote force-pushed the simplify-apply_effects_in_range branch from cd61331 to e736635 Compare July 20, 2026 04:29

rust-bors Bot 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 Jul 22, 2026

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 22, 2026

Rollup merge of rust-lang#159285 - nnethercote:simplify-apply_effects…

8dd9eb7

…_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

JonathanBrouwer mentioned this pull request Jul 22, 2026

Rollup of 4 pull requests #159709

Closed

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 22, 2026

Rollup merge of rust-lang#159285 - nnethercote:simplify-apply_effects…

cc700c5

…_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

JonathanBrouwer mentioned this pull request Jul 22, 2026

Rollup of 8 pull requests #159715

Closed

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 22, 2026

Rollup merge of rust-lang#159285 - nnethercote:simplify-apply_effects…

3378f49

…_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

JonathanBrouwer mentioned this pull request Jul 22, 2026

Rollup of 12 pull requests #159723

Closed

JonathanBrouwer mentioned this pull request Jul 22, 2026

Rollup of 11 pull requests #159728

Merged

rust-bors Bot merged commit a6627d2 into rust-lang:main Jul 22, 2026
13 checks passed

rustbot added this to the 1.99.0 milestone Jul 22, 2026

rust-timer added a commit that referenced this pull request Jul 22, 2026

Unrolled build for #159285

11dd499

Rollup merge of #159285 - nnethercote:simplify-apply_effects_in_range, r=cjgillot Simplify `apply_effects_in_range` Details in individual commits. r? @cjgillot

nnethercote deleted the simplify-apply_effects_in_range branch July 22, 2026 21:10

Uh oh!

Uh oh!

Conversation

nnethercote commented Jul 14, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Jul 14, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Jul 14, 2026

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

nnethercote commented Jul 14, 2026

Uh oh!

Uh oh!

Uh oh!

rustbot commented Jul 20, 2026

Uh oh!

rustbot commented Jul 20, 2026

Uh oh!

This comment has been minimized.

nnethercote commented Jul 20, 2026

Uh oh!

nnethercote commented Jul 20, 2026

Uh oh!

cjgillot commented Jul 22, 2026

Uh oh!

rust-bors Bot commented Jul 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nnethercote commented Jul 14, 2026 •

edited by rustbot

Loading