Require explicitly marking closures as coroutines #123792

oli-obk · 2024-04-11T13:17:34Z

instead of relying on patching up the closure to be a coroutine if it happens to contain a yield expression.

I only do this in the 2024 edition, as the gen keyword is only available there.

rustbot · 2024-04-11T13:17:46Z

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

oli-obk · 2024-04-11T13:17:47Z

r? @compiler-errors

oli-obk · 2024-04-11T13:20:01Z

compiler/rustc_ast_lowering/src/item.rs

-                self.with_new_scopes(ident.span, |this| {
+                self.with_new_scopes(span, |this| {


I had to do this change as otherwise the span points to the identifier of functions instead of the fn, so adding gen would have resulted in fn gen foo instead of gen fn foo.

oli-obk · 2024-04-11T13:21:09Z

compiler/rustc_parse/src/parser/expr.rs

-                if this.token.uninterpolated_span().at_least_rust_2024()
-                    // check for `gen {}` and `gen move {}`
-                    // or `async gen {}` and `async gen move {}`
-                    && (this.is_gen_block(kw::Gen, 0)
-                        || (this.check_keyword(kw::Async) && this.is_gen_block(kw::Gen, 1)))
-                {
-                    // FIXME: (async) gen closures aren't yet parsed.
-                    this.parse_gen_block()
-                } else if this.check_keyword(kw::Async) {
+                if this.check_keyword(kw::Async) {
                    // FIXME(gen_blocks): Parse `gen async` and suggest swap
-                    if this.is_gen_block(kw::Async, 0) {
+                    if this.is_gen_block(kw::Async, 0) || this.is_gen_block(kw::Gen, 1) {
+                        // Check for `async {` and `async move {`,
+                        this.parse_gen_block()
+                    } else {
+                        this.parse_expr_closure()
+                    }
+                } else if this.check_keyword(kw::Gen)
+                    && this.token.uninterpolated_span().at_least_rust_2024()
+                {
+                    if this.is_gen_block(kw::Gen, 0) {


This deserves further cleanup, but I think it's already an improvement if all the async stuff always goes through the kw::Async arm.

Also we can now gate the edition check on the keyword check, yay

rustbot · 2024-04-11T13:27:59Z

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

compiler-errors · 2024-04-11T14:05:12Z

Let's not use gen as the keyword for this. That conflicts with gen closures, which we could have as a parallel to async closures, and is confusing given that gen fns and gen blocks also exist.

compiler-errors · 2024-04-11T14:06:32Z

Given that full/general coroutines still are not expected to land anytime soon, I'd rather if we don't add syntax for this at all, and just use an attr tbh

rustbot · 2024-04-12T14:36:51Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in coverage tests.

cc @Zalathar

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

src/tools/rustfmt/tests/source/immovable_coroutines.rs

compiler-errors · 2024-04-12T16:29:04Z

Cool -- I appreciate this new approach. I will review it in detail when I find some time soon...

bors · 2024-04-16T14:12:24Z

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

oli-obk · 2024-04-23T09:47:31Z

@bors r=compiler-errors rollup=iffy

bors · 2024-04-23T09:47:33Z

📌 Commit 2c39d20 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-04-23T13:09:59Z

⌛ Testing commit 2c39d20 with merge f506f21...

…er-errors Require explicitly marking closures as coroutines instead of relying on patching up the closure to be a coroutine if it happens to contain a `yield` expression. I only do this in the 2024 edition, as the `gen` keyword is only available there.

bors · 2024-04-23T14:05:57Z

💔 Test failed - checks-actions

oli-obk · 2024-04-23T16:36:17Z

msvc -.-

@bors r=compiler-errors rollup=iffy

bors · 2024-04-23T16:36:19Z

📌 Commit bf436b5 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-04-23T19:08:46Z

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

…ing `yield` expressions

And suggest adding the `#[coroutine]` to the closure

oli-obk · 2024-04-24T08:06:16Z

@bors r=compiler-errors rollup=iffy

bors · 2024-04-24T08:06:18Z

📌 Commit aef0f40 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-04-24T08:43:34Z

⌛ Testing commit aef0f40 with merge e936289...

bors · 2024-04-24T10:47:17Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing e936289 to master...

rust-timer · 2024-04-24T12:04:08Z

Finished benchmarking commit (e936289): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -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)	3.4%	[3.4%, 3.4%]	1
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.3%	[-5.3%, -5.3%]	1
All ❌✅ (primary)	3.4%	[3.4%, 3.4%]	1

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)	3.5%	[2.8%, 4.2%]	3
Regressions ❌ (secondary)	2.4%	[2.1%, 2.8%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.5%	[2.8%, 4.2%]	3

Binary size

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

Bootstrap: 673.226s -> 672.79s (-0.06%)
Artifact size: 315.42 MiB -> 315.46 MiB (0.01%)

rustbot assigned petrochenkov Apr 11, 2024

This comment was marked as resolved.

Sign in to view

rustbot added PG-exploit-mitigations Project group: Exploit mitigations 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, 2024

rustbot assigned compiler-errors and unassigned petrochenkov Apr 11, 2024

oli-obk commented Apr 11, 2024

View reviewed changes

oli-obk force-pushed the coroutine_closures branch from f0906f5 to b692d5b Compare April 11, 2024 13:27

compiler-errors 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 Apr 11, 2024

oli-obk force-pushed the coroutine_closures branch from b692d5b to 3c91020 Compare April 12, 2024 14:36

oli-obk force-pushed the coroutine_closures branch from 3c91020 to d8ab6bf Compare April 12, 2024 14:43

This comment has been minimized.

Sign in to view

oli-obk force-pushed the coroutine_closures branch from d8ab6bf to 2ddca19 Compare April 12, 2024 15:49

ytmimi reviewed Apr 12, 2024

View reviewed changes

src/tools/rustfmt/tests/source/immovable_coroutines.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

oli-obk force-pushed the coroutine_closures branch from 2ddca19 to 37af228 Compare April 12, 2024 17:34

This comment has been minimized.

Sign in to view

oli-obk removed the PG-exploit-mitigations Project group: Exploit mitigations label Apr 17, 2024

oli-obk force-pushed the coroutine_closures branch from 37af228 to b32acf6 Compare April 17, 2024 15:06

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 23, 2024

oli-obk force-pushed the coroutine_closures branch from 2c39d20 to bf436b5 Compare April 23, 2024 16:36

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

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 23, 2024

oli-obk added 2 commits April 24, 2024 07:54

Add explicit syntax for coroutines instead of relying on closures hav…

a589632

…ing `yield` expressions

Error on using yield without also using #[coroutine] on the closure

aef0f40

And suggest adding the `#[coroutine]` to the closure

oli-obk force-pushed the coroutine_closures branch from bf436b5 to aef0f40 Compare April 24, 2024 08:05

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 24, 2024

bors merged commit e936289 into rust-lang:master Apr 24, 2024
13 checks passed

rustbot added this to the 1.79.0 milestone Apr 24, 2024

oli-obk deleted the coroutine_closures branch April 24, 2024 12:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Require explicitly marking closures as coroutines #123792

Require explicitly marking closures as coroutines #123792

oli-obk commented Apr 11, 2024

This comment was marked as resolved.

rustbot commented Apr 11, 2024

oli-obk commented Apr 11, 2024

oli-obk Apr 11, 2024

oli-obk Apr 11, 2024

rustbot commented Apr 11, 2024

compiler-errors commented Apr 11, 2024 •

edited

compiler-errors commented Apr 11, 2024

rustbot commented Apr 12, 2024

This comment has been minimized.

This comment has been minimized.

compiler-errors commented Apr 12, 2024 •

edited

This comment has been minimized.

bors commented Apr 16, 2024

oli-obk commented Apr 23, 2024

bors commented Apr 23, 2024

bors commented Apr 23, 2024

This comment has been minimized.

bors commented Apr 23, 2024

oli-obk commented Apr 23, 2024

bors commented Apr 23, 2024

bors commented Apr 23, 2024

oli-obk commented Apr 24, 2024

bors commented Apr 24, 2024

bors commented Apr 24, 2024

bors commented Apr 24, 2024

rust-timer commented Apr 24, 2024

		self.with_new_scopes(ident.span, \|this\| {
		self.with_new_scopes(span, \|this\| {

Require explicitly marking closures as coroutines #123792

Require explicitly marking closures as coroutines #123792

Conversation

oli-obk commented Apr 11, 2024

This comment was marked as resolved.

rustbot commented Apr 11, 2024

oli-obk commented Apr 11, 2024

oli-obk Apr 11, 2024

Choose a reason for hiding this comment

oli-obk Apr 11, 2024

Choose a reason for hiding this comment

rustbot commented Apr 11, 2024

compiler-errors commented Apr 11, 2024 • edited

compiler-errors commented Apr 11, 2024

rustbot commented Apr 12, 2024

This comment has been minimized.

This comment has been minimized.

compiler-errors commented Apr 12, 2024 • edited

This comment has been minimized.

bors commented Apr 16, 2024

oli-obk commented Apr 23, 2024

bors commented Apr 23, 2024

bors commented Apr 23, 2024

This comment has been minimized.

bors commented Apr 23, 2024

oli-obk commented Apr 23, 2024

bors commented Apr 23, 2024

bors commented Apr 23, 2024

oli-obk commented Apr 24, 2024

bors commented Apr 24, 2024

bors commented Apr 24, 2024

bors commented Apr 24, 2024

rust-timer commented Apr 24, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

compiler-errors commented Apr 11, 2024 •

edited

compiler-errors commented Apr 12, 2024 •

edited