cleanup `mir_borrowck` #139666

lcnr · 2025-04-11T10:13:16Z

Cleanup pulled out of #139587. Best reviewed commit by commit.

r? compiler-errors

This previously incorrectly returned `true` for parent functions whose first statement was `let local = <coroutine>;`. While that didn't cause any bugs as we only ever access `movable_coroutine` for `yield` terminators. It was still wrong.

lcnr · 2025-04-11T10:14:33Z

@bors try @rust-timer queue

cleanup `mir_borrowck` Cleanup pulled out of rust-lang#139587. Best reviewed commit by commit. r? compiler-errors

bors · 2025-04-11T10:15:44Z

⌛ Trying commit 13bd7e0 with merge 92cb866...

lcnr · 2025-04-11T13:37:42Z

@rust-timer build 92cb866

rust-timer · 2025-04-11T20:43:19Z

Finished benchmarking commit (92cb866): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -1.0%, secondary 0.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.8%	[0.4%, 1.3%]	2
Improvements ✅ (primary)	-1.0%	[-1.0%, -1.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.0%	[-1.0%, -1.0%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 781.035s -> 782.175s (0.15%)
Artifact size: 365.98 MiB -> 366.02 MiB (0.01%)

lqd · 2025-04-12T06:38:16Z

compiler/rustc_borrowck/src/lib.rs

@@ -465,16 +473,6 @@ fn do_mir_borrowck<'tcx>(

    mbcx.report_move_errors();

-    // If requested, dump polonius MIR.
-    polonius::dump_polonius_mir(


I put the call here because the dump will use the dataflow results just above

as in, moving this is wrong?

it does not access flow_results or the mbcx, so how does the code above impact this output 🤔

It doesn’t on master right now but will, when I open the PR to add the full Polonius debugger to its MIR dump: it shows the loans going out of NLL scope for example.

No worries, I will move it back if it needs to, but I also don’t know if I’ll open that PR before needing to rework active loans away from these dataflow results, so it’s fine.

compiler-errors · 2025-04-13T16:41:30Z

@bors r+ rollup=maybe

bors · 2025-04-13T16:41:33Z

📌 Commit 420390c has been approved by compiler-errors

It is now in the queue for this repository.

…compiler-errors cleanup `mir_borrowck` Cleanup pulled out of rust-lang#139587. Best reviewed commit by commit. r? compiler-errors

…enton Rollup of 12 pull requests Successful merges: - rust-lang#138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of rust-lang#94678)) - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast) - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate) - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration) - rust-lang#139582 (Various coercion cleanups) - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`) - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`) - rust-lang#139666 (cleanup `mir_borrowck`) - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints) - rust-lang#139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout) - rust-lang#139722 (Move some things to rustc_type_ir) - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 9 pull requests Successful merges: - rust-lang#138336 (Improve `-Z crate-attr` diagnostics) - rust-lang#139636 (Encode dep node edge count as u32 instead of usize) - rust-lang#139666 (cleanup `mir_borrowck`) - rust-lang#139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout) - rust-lang#139699 (Proactively update coroutine drop shim's phase to account for later passes applied during shim query) - rust-lang#139718 (enforce unsafe attributes in pre-2024 editions by default) - rust-lang#139722 (Move some things to rustc_type_ir) - rust-lang#139760 (UI tests: migrate remaining compile time `error-pattern`s to line annotations when possible) - rust-lang#139776 (Switch attrs to `diagnostic::on_unimplemented`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139666 - lcnr:pre-revealing-use-cleanup, r=compiler-errors cleanup `mir_borrowck` Cleanup pulled out of rust-lang#139587. Best reviewed commit by commit. r? compiler-errors

lcnr added 7 commits April 11, 2025 10:41

consistent name for UniversalRegions

923f44c

use input def_id to compute movable_coroutine

8cb7274

This previously incorrectly returned `true` for parent functions whose first statement was `let local = <coroutine>;`. While that didn't cause any bugs as we only ever access `movable_coroutine` for `yield` terminators. It was still wrong.

local_names creation to mbcx creation

848187c

do not buffer #[rustc_regions] dump

0186459

MirBorrowckCtxt::polonius_output to ref

0e294f2

move dump_polonius_mir

2c65469

don't rely on locals_are_invalidated_at_exit

c5fdddc

rustbot assigned compiler-errors Apr 11, 2025

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 Apr 11, 2025

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 Apr 11, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2025

Auto merge of rust-lang#139666 - lcnr:pre-revealing-use-cleanup, r=<try>

92cb866

cleanup `mir_borrowck` Cleanup pulled out of rust-lang#139587. Best reviewed commit by commit. r? compiler-errors

lcnr force-pushed the pre-revealing-use-cleanup branch from 13bd7e0 to 0097e25 Compare April 11, 2025 10:50

This comment has been minimized.

Sign in to view

lcnr force-pushed the pre-revealing-use-cleanup branch from 0097e25 to 640195a Compare April 11, 2025 11:16

lcnr added 3 commits April 11, 2025 15:18

remove redundant fields

8303383

NonGenericOpaqueTypeParam::ty to arg

5d00483

eagerly initialize definitions in sub-fn

420390c

lcnr force-pushed the pre-revealing-use-cleanup branch from 640195a to 420390c Compare April 11, 2025 13:18

lcnr mentioned this pull request Apr 11, 2025

support revealing uses of opaques in closures #139587

Draft

This comment was marked as outdated.

Sign in to view

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 Apr 11, 2025

lqd reviewed Apr 12, 2025

View reviewed changes

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 Apr 13, 2025

ChrisDenton mentioned this pull request Apr 13, 2025

Rollup of 13 pull requests #139754

Closed

ChrisDenton mentioned this pull request Apr 13, 2025

Rollup of 12 pull requests #139755

Closed

jhpratt mentioned this pull request Apr 14, 2025

Rollup of 9 pull requests #139781

Merged

bors merged commit a642232 into rust-lang:master Apr 14, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup `mir_borrowck` #139666

cleanup `mir_borrowck` #139666

lcnr commented Apr 11, 2025

lcnr commented Apr 11, 2025

This comment has been minimized.

bors commented Apr 11, 2025

This comment has been minimized.

This comment was marked as outdated.

lcnr commented Apr 11, 2025

This comment has been minimized.

rust-timer commented Apr 11, 2025

lqd Apr 12, 2025

lcnr Apr 14, 2025

lqd Apr 14, 2025 •

edited

Loading

compiler-errors commented Apr 13, 2025

bors commented Apr 13, 2025

cleanup mir_borrowck #139666

cleanup mir_borrowck #139666

Conversation

lcnr commented Apr 11, 2025

lcnr commented Apr 11, 2025

This comment has been minimized.

bors commented Apr 11, 2025

This comment has been minimized.

This comment was marked as outdated.

lcnr commented Apr 11, 2025

This comment has been minimized.

rust-timer commented Apr 11, 2025

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

lqd Apr 12, 2025

Choose a reason for hiding this comment

lcnr Apr 14, 2025

Choose a reason for hiding this comment

lqd Apr 14, 2025 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Apr 13, 2025

bors commented Apr 13, 2025

cleanup `mir_borrowck` #139666

cleanup `mir_borrowck` #139666

lqd Apr 14, 2025 •

edited

Loading