Make closures carry their own ClosureKind #119258

compiler-errors · 2023-12-23T16:57:05Z

Right now, we use the "movability" field of hir::Closure to distinguish a closure and a coroutine. This is paired together with the CoroutineKind, which is located not in the hir::Closure, but the hir::Body. This is strange and redundant.

This PR introduces ClosureKind with two variants -- Closure and Coroutine, which is put into hir::Closure. The CoroutineKind is thus removed from hir::Body, and Option<Movability> no longer needs to be a stand-in for "is this a closure or a coroutine".

r? eholk

rustbot · 2023-12-23T16:57:14Z

Changes to the size of AST and/or HIR nodes.

cc @nnethercote

compiler/rustc_hir/src/hir.rs

compiler-errors · 2023-12-23T16:58:17Z

I doubt this changes much, but

@bors try @rust-timer queue

bors · 2023-12-23T16:59:27Z

⌛ Trying commit bd0eec7 with merge 46dc808...

Make closures carry their own ClosureKind Right now, we use the "`movability`" field of `hir::Closure` to distinguish a closure and a coroutine. This is paired together with the `CoroutineKind`, which is located not in the `hir::Closure`, but the `hir::Body`. This is strange and redundant. This PR introduces `ClosureKind` with two variants -- `Closure` and `Coroutine`, which is put into `hir::Closure`. The `CoroutineKind` is thus removed from `hir::Body`, and `Option<Movability>` no longer needs to be a stand-in for "is this a closure or a coroutine". r? eholk

bors · 2023-12-23T18:26:07Z

☀️ Try build successful - checks-actions
Build commit: 46dc808 (46dc80894476d4107753bc19dcdb09d846358f9f)

rust-timer · 2023-12-23T20:54:41Z

Finished benchmarking commit (46dc808): comparison URL.

Overall result: no relevant changes - 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 benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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.5%	[0.5%, 0.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results

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
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.5%	[-1.5%, -1.5%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results

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
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	1
All ❌✅ (primary)	-	-	0

Bootstrap: 669.674s -> 670.704s (0.15%)
Artifact size: 312.80 MiB -> 312.78 MiB (-0.01%)

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

compiler/rustc_borrowck/src/diagnostics/region_name.rs

compiler/rustc_hir/src/hir.rs

…it is from movability

rustbot · 2023-12-25T16:56:38Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

rustbot · 2023-12-25T18:05:52Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler/rustc_ast_lowering/src/expr.rs

compiler/rustc_hir/src/hir.rs

Also fixes a bug where we weren't peeling blocks from async bodies

compiler/rustc_middle/src/middle/region.rs

eholk · 2023-12-25T21:24:37Z

@bors r+ rollup

bors · 2023-12-25T21:24:40Z

📌 Commit ba91285 has been approved by eholk

It is now in the queue for this repository.

compiler-errors · 2023-12-25T21:26:05Z

this is enough changes that i'd rather not roll this up

@bors rollup=never

bors · 2023-12-26T04:25:56Z

⌛ Testing commit ba91285 with merge 1ab7831...

bors · 2023-12-26T06:23:16Z

☀️ Test successful - checks-actions
Approved by: eholk
Pushing 1ab7831 to master...

rust-timer · 2023-12-26T08:46:40Z

Finished benchmarking commit (1ab7831): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

Max RSS (memory usage)

Results

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)	1.2%	[1.2%, 1.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-3.2%, -3.2%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

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)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 672.287s -> 671.757s (-0.08%)
Artifact size: 312.50 MiB -> 312.54 MiB (0.01%)

Make closures carry their own ClosureKind Right now, we use the "`movability`" field of `hir::Closure` to distinguish a closure and a coroutine. This is paired together with the `CoroutineKind`, which is located not in the `hir::Closure`, but the `hir::Body`. This is strange and redundant. This PR introduces `ClosureKind` with two variants -- `Closure` and `Coroutine`, which is put into `hir::Closure`. The `CoroutineKind` is thus removed from `hir::Body`, and `Option<Movability>` no longer needs to be a stand-in for "is this a closure or a coroutine". r? eholk

rustbot assigned eholk Dec 23, 2023

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 23, 2023

compiler-errors commented Dec 23, 2023

View reviewed changes

compiler/rustc_hir/src/hir.rs Show resolved Hide resolved

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 Dec 23, 2023

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 Dec 23, 2023

cjgillot reviewed Dec 25, 2023

View reviewed changes

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs Outdated Show resolved Hide resolved

compiler/rustc_borrowck/src/diagnostics/region_name.rs Outdated Show resolved Hide resolved

compiler/rustc_hir/src/hir.rs Outdated Show resolved Hide resolved

compiler-errors added 3 commits December 25, 2023 16:28

Remove unnecessary body_expr_count

bacfa97

Don't pass body into check_closure and child functions

981fc6e

Make closures carry their own ClosureKind, rather than deducing what …

909dd86

…it is from movability

rust-cloud-vms bot force-pushed the closure-kind branch from bd0eec7 to 34daa4f Compare December 25, 2023 16:56

This comment has been minimized.

Sign in to view

compiler-errors mentioned this pull request Dec 25, 2023

Remove movability from TyKind::Coroutine #119174

Merged

eholk reviewed Dec 25, 2023

View reviewed changes

compiler/rustc_ast_lowering/src/expr.rs Show resolved Hide resolved

eholk reviewed Dec 25, 2023

View reviewed changes

compiler/rustc_hir/src/hir.rs Outdated Show resolved Hide resolved

compiler-errors added 3 commits December 25, 2023 21:13

Only regular coroutines have movability

3320c09

Fix clippy's usage of Body's coroutine_kind

27dce30

Also fixes a bug where we weren't peeling blocks from async bodies

Flatten match

ba91285

rust-cloud-vms bot force-pushed the closure-kind branch from 808d5f8 to ba91285 Compare December 25, 2023 21:13

eholk reviewed Dec 25, 2023

View reviewed changes

compiler/rustc_middle/src/middle/region.rs Show resolved Hide resolved

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 25, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 26, 2023

bors merged commit 1ab7831 into rust-lang:master Dec 26, 2023
12 checks passed

rustbot added this to the 1.77.0 milestone Dec 26, 2023

This was referenced Dec 26, 2023

Add range metadata to slice lengths #116542

Draft

Add new temporary lifetime feature gate and super let keyword #119043

Closed

lqd mentioned this pull request Dec 26, 2023

Triage 2023 12 26 rust-lang/rustc-perf#1785

Merged

xFrednet mentioned this pull request Dec 26, 2023

Chore: Bump nightly -> 2023-12-28 rust-marker/marker#315

Open

petrochenkov mentioned this pull request Dec 28, 2023

aligns the behavior to that prior to #118311 #118411

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make closures carry their own ClosureKind #119258

Make closures carry their own ClosureKind #119258

compiler-errors commented Dec 23, 2023

rustbot commented Dec 23, 2023

compiler-errors commented Dec 23, 2023

This comment has been minimized.

bors commented Dec 23, 2023

This comment has been minimized.

bors commented Dec 23, 2023

This comment has been minimized.

rust-timer commented Dec 23, 2023

rustbot commented Dec 25, 2023

This comment has been minimized.

rustbot commented Dec 25, 2023

eholk commented Dec 25, 2023

bors commented Dec 25, 2023

compiler-errors commented Dec 25, 2023

bors commented Dec 26, 2023

bors commented Dec 26, 2023

rust-timer commented Dec 26, 2023

Make closures carry their own ClosureKind #119258

Make closures carry their own ClosureKind #119258

Conversation

compiler-errors commented Dec 23, 2023

rustbot commented Dec 23, 2023

compiler-errors commented Dec 23, 2023

This comment has been minimized.

bors commented Dec 23, 2023

This comment has been minimized.

bors commented Dec 23, 2023

This comment has been minimized.

rust-timer commented Dec 23, 2023

Overall result: no relevant changes - no action needed

rustbot commented Dec 25, 2023

This comment has been minimized.

rustbot commented Dec 25, 2023

eholk commented Dec 25, 2023

bors commented Dec 25, 2023

compiler-errors commented Dec 25, 2023

bors commented Dec 26, 2023

bors commented Dec 26, 2023

rust-timer commented Dec 26, 2023

Overall result: ✅ improvements - no action needed