Add support for `gen fn` #118457

eholk · 2023-11-29T20:31:13Z

This builds on #116447 to add support for gen fn functions. For the most part we follow the same approach as desugaring async fn, but replacing Future with Iterator and async {} with gen {} for the body.

The version implemented here uses the return type of a gen fn as the yield type. For example:

gen fn count_to_three() -> i32 {
    yield 1;
    yield 2;
    yield 3;
}

In the future, I think we should experiment with a syntax like gen fn count_to_three() yield i32 { ... }, but that can go in another PR.

cc @oli-obk @compiler-errors

rustbot · 2023-11-29T20:31:23Z

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

eholk · 2023-11-29T20:31:33Z

@rustbot author

compiler-errors · 2023-11-29T20:36:45Z

r? compiler-errors

I'll review this

compiler-errors · 2023-11-29T20:39:33Z

Make sure something reasonable happens when the user writes async gen fn

I would not worry too much about this, since I can add support for async gen fn on top of #118420 if this lands first (which it very likely will)

You can make it a diagnostic tho I guess, or just leave it as a span-bug lol. I don't even think we parse async gen fn front matters correctly...?

compiler/rustc_ast/src/ast.rs

compiler/rustc_ast_lowering/src/item.rs

eholk · 2023-11-29T21:56:28Z

You can make it a diagnostic tho I guess, or just leave it as a span-bug lol. I don't even think we parse async gen fn front matters correctly...?

I don't know, so that's the thing to check :)

The main thing is I want to make sure if you write async gen fn it doesn't silently try to become just an async fn, which is what I suspect will happen with a lot of the code now. If we give an error during parsing, that would be good enough for me.

compiler/rustc_ast_lowering/src/lib.rs

eholk · 2023-11-30T23:50:10Z

compiler/rustc_parse/src/parser/item.rs

@@ -2536,6 +2547,8 @@ impl<'a> Parser<'a> {
                        }
                    }

+                    // FIXME(eholk): add keyword recovery logic for genness


I'm considering leaving this for a later PR that's focused on diagnostic improvements around gen fn, so that this one can focus on the baseline functionality.

Yeah that's fine I guess

Please change the FIXME though to not reference a single person, since this is associated with feature work

rustbot · 2023-12-01T00:41:53Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

eholk · 2023-12-01T00:42:21Z

I'm ready to have this reviewed now.

@rustbot ready

compiler-errors

More nits and reviews

Also raised the question whether this needs to be blocked on the RFC landing or if it is still okay to land this pre-RFC: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Generators.2C.20lang-team.20experiments.2C.20RFCs.2C.20oh.20my!/near/405406740 edit: seems ok

compiler/rustc_builtin_macros/src/test.rs

compiler/rustc_ast_passes/src/ast_validation.rs

compiler/rustc_ast_lowering/src/path.rs

compiler/rustc_ast_lowering/src/lib.rs

compiler/rustc_resolve/src/late.rs

compiler-errors · 2023-12-01T17:45:19Z

compiler/rustc_resolve/src/late.rs

@@ -940,7 +942,8 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
                        this.visit_generics(generics);

                        let declaration = &sig.decl;
-                        let async_node_id = sig.header.asyncness.opt_return_id();
+                        let async_node_id =


Please add a test that makes sure that gen fn captures lifetimes in its signature.

I added gen_fn_lifetime_capture.rs, but I'm not sure it completely exercises everything it should.

compiler/rustc_resolve/src/def_collector.rs

compiler-errors · 2023-12-01T17:46:51Z

compiler/rustc_parse/src/parser/item.rs

@@ -2536,6 +2547,8 @@ impl<'a> Parser<'a> {
                        }
                    }

+                    // FIXME(eholk): add keyword recovery logic for genness


Yeah that's fine I guess

Please change the FIXME though to not reference a single person, since this is associated with feature work

bors · 2023-12-02T14:33:49Z

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

eholk

I think I've addressed all your comments. I'd appreciate a double check on the lifetime capture test though.

compiler/rustc_ast/src/mut_visit.rs

compiler/rustc_ast_lowering/src/item.rs

compiler/rustc_ast_lowering/src/lib.rs

compiler/rustc_resolve/src/def_collector.rs

compiler/rustc_resolve/src/late.rs

src/tools/rustfmt/src/closures.rs

eholk · 2023-12-04T22:07:57Z

compiler/rustc_resolve/src/late.rs

@@ -940,7 +942,8 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
                        this.visit_generics(generics);

                        let declaration = &sig.decl;
-                        let async_node_id = sig.header.asyncness.opt_return_id();
+                        let async_node_id =


I added gen_fn_lifetime_capture.rs, but I'm not sure it completely exercises everything it should.

compiler-errors

In the future, do you mind cleaning up this PR history a bit? Commits like 09f0741 don't really seem necessary -- you could've probably just amended that commit into whatever bad merge you performed in the first place.

compiler-errors · 2023-12-05T15:48:12Z

@bors r+ rollup=never

bors · 2023-12-05T15:48:15Z

📌 Commit 2c8dbd9 has been approved by compiler-errors

It is now in the queue for this repository.

eholk · 2023-12-05T17:17:55Z

Thanks for the r+!

I'll clean things up better in the future. The various "Fix CI" commits could also be squashed into something else.

bors · 2023-12-05T18:37:19Z

⌛ Testing commit 2c8dbd9 with merge 56278a6...

bors · 2023-12-05T21:06:42Z

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

rust-timer · 2023-12-05T22:43:48Z

Finished benchmarking commit (56278a6): 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.5%	[3.5%, 3.5%]	1
Regressions ❌ (secondary)	2.1%	[2.1%, 2.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.5%	[3.5%, 3.5%]	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: 675.106s -> 675.495s (0.06%)
Artifact size: 314.16 MiB -> 314.16 MiB (0.00%)

rustbot assigned b-naber Nov 29, 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 Nov 29, 2023

rustbot 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 Nov 29, 2023

rustbot assigned compiler-errors and unassigned b-naber Nov 29, 2023

compiler-errors reviewed Nov 29, 2023

View reviewed changes

compiler/rustc_ast/src/ast.rs Outdated Show resolved Hide resolved

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

This comment has been minimized.

Sign in to view

eholk force-pushed the genfn branch from c67193b to d116f17 Compare November 30, 2023 23:03

This comment has been minimized.

Sign in to view

eholk commented Nov 30, 2023

View reviewed changes

compiler/rustc_ast_lowering/src/lib.rs Outdated Show resolved Hide resolved

eholk commented Nov 30, 2023

View reviewed changes

eholk changed the title ~~WIP: Add support for gen fn~~ Add support for gen fn Dec 1, 2023

eholk marked this pull request as ready for review December 1, 2023 00:41

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

This comment has been minimized.

Sign in to view

compiler-errors requested changes Dec 1, 2023

View reviewed changes

rustbot 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 Dec 1, 2023

eholk mentioned this pull request Dec 1, 2023

Tracking Issue for gen blocks and functions #117078

Open

14 tasks

eholk added 5 commits December 4, 2023 11:23

Add a basic test for gen fn

3887b16

Merge Async and Gen into CoroutineKind

48d5f1f

Option<CoroutineKind>

f9d1f92

Fix some broken tests

26f9954

Address code review feedback

50ef800

eholk force-pushed the genfn branch from 017f6bd to 50ef800 Compare December 4, 2023 22:34

eholk commented Dec 4, 2023

View reviewed changes

Remove bad merge

09f0741

This comment has been minimized.

Sign in to view

eholk added 2 commits December 4, 2023 16:37

Update doctest

ac4d0f2

Fix build

2c8dbd9

compiler-errors approved these changes Dec 5, 2023

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

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

bors merged commit 56278a6 into rust-lang:master Dec 5, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Dec 5, 2023

eholk deleted the genfn branch December 5, 2023 21:11

eholk restored the genfn branch December 5, 2023 21:12

This was referenced Dec 5, 2023

Do NOT Review: CoAlloc: Allocator + Global API + Vec #108761

Closed

Tracking import use types for more accurate redundant import checking #117772

Merged

Collect lang items from AST, get rid of GenericBound::LangItemTrait #118396

Merged

compiler-errors mentioned this pull request Jan 2, 2024

Add gen blocks and reserve keyword in Rust 2024 rust-lang/rfcs#3513

Merged

yoshuawuyts mentioned this pull request Jan 3, 2024

Rename AsyncIterator back to Stream, introduce an AFIT-based AsyncIterator trait #119550

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for `gen fn` #118457

Add support for `gen fn` #118457

eholk commented Nov 29, 2023 •

edited

Loading

rustbot commented Nov 29, 2023

eholk commented Nov 29, 2023

compiler-errors commented Nov 29, 2023

compiler-errors commented Nov 29, 2023 •

edited

Loading

This comment has been minimized.

eholk commented Nov 29, 2023

This comment has been minimized.

eholk Nov 30, 2023

compiler-errors Dec 1, 2023

eholk Dec 4, 2023

rustbot commented Dec 1, 2023

eholk commented Dec 1, 2023

This comment has been minimized.

compiler-errors left a comment •

edited

Loading

compiler-errors Dec 1, 2023

compiler-errors Dec 1, 2023

eholk Dec 4, 2023

compiler-errors Dec 1, 2023

bors commented Dec 2, 2023

eholk left a comment

eholk Dec 4, 2023

This comment has been minimized.

compiler-errors left a comment

compiler-errors commented Dec 5, 2023

bors commented Dec 5, 2023

eholk commented Dec 5, 2023

bors commented Dec 5, 2023

bors commented Dec 5, 2023

rust-timer commented Dec 5, 2023

Add support for gen fn #118457

Add support for gen fn #118457

Conversation

eholk commented Nov 29, 2023 • edited Loading

rustbot commented Nov 29, 2023

eholk commented Nov 29, 2023

compiler-errors commented Nov 29, 2023

compiler-errors commented Nov 29, 2023 • edited Loading

This comment has been minimized.

eholk commented Nov 29, 2023

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rustbot commented Dec 1, 2023

eholk commented Dec 1, 2023

This comment has been minimized.

compiler-errors left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Dec 2, 2023

eholk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors commented Dec 5, 2023

bors commented Dec 5, 2023

eholk commented Dec 5, 2023

bors commented Dec 5, 2023

bors commented Dec 5, 2023

rust-timer commented Dec 5, 2023

Overall result: no relevant changes - no action needed

Add support for `gen fn` #118457

Add support for `gen fn` #118457

eholk commented Nov 29, 2023 •

edited

Loading

compiler-errors commented Nov 29, 2023 •

edited

Loading

compiler-errors left a comment •

edited

Loading