Introduce support for `async` bound modifier on `Fn*` traits #120392

compiler-errors · 2024-01-26T17:55:17Z

Adds async to the list of TraitBoundModifiers, which instructs AST lowering to map the trait to an async flavor of the trait. For now, this is only supported for Fn* to AsyncFn*, and I expect that this manual mapping via lang items will be replaced with a better system in the future.

The motivation for adding these bounds is to separate the users of async closures from the exact trait desugaring of their callable bounds. Instead of users needing to be concerned with the AsyncFn trait, they should be able to write async Fn() and it will desugar to whatever underlying trait we decide is best for the lowering of async closures.

Note: rustfmt support can be done in the rustfmt repo after a subtree sync.

rustbot · 2024-01-26T17:55:26Z

r? @davidtwco

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

compiler-errors · 2024-01-26T17:56:31Z

@rustbot label: I-lang-nominated

Hi @rust-lang/lang. Does this warrant a new experiment, or can I consider it to be part of the existing lang team experiment for AsyncFn traits? Any thoughts about this? Feel free to unnominate if you believe this doesn't need further team discussion.

compiler-errors · 2024-01-26T17:58:25Z

Oh, also cc @rust-lang/wg-async as well.

rustbot · 2024-01-26T19:12:32Z

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

cc @nnethercote

compiler-errors · 2024-01-26T19:15:46Z

@bors try @rust-timer queue

bors · 2024-01-26T19:16:55Z

⌛ Trying commit d2bb2cc with merge cb0d84e...

…, r=<try> Introduce support for `async` bound modifier on `Fn*` traits Adds `async` to the list of `TraitBoundModifiers`, which instructs AST lowering to map the trait to an async flavor of the trait. For now, this is only supported for `Fn*` to `AsyncFn*`, and I expect that this manual mapping via lang items will be replaced with a better system in the future. The motivation for adding these bounds is to separate the users of async closures from the exact trait desugaring of their callable bounds. Instead of users needing to be concerned with the `AsyncFn` trait, they should be able to write `async Fn()` and it will desugar to whatever underlying trait we decide is best for the lowering of async closures.

bors · 2024-01-26T20:44:25Z

☀️ Try build successful - checks-actions
Build commit: cb0d84e (cb0d84e48bea10d939e67f737b1bf3e373c00616)

rust-timer · 2024-01-26T21:58:11Z

Finished benchmarking commit (cb0d84e): comparison URL.

Overall result: ❌ regressions - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +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.2%	[0.2%, 0.2%]	3
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	3

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)	4.9%	[4.9%, 4.9%]	1
Improvements ✅ (primary)	-2.1%	[-3.7%, -0.4%]	2
Improvements ✅ (secondary)	-2.7%	[-5.0%, -0.7%]	3
All ❌✅ (primary)	-2.1%	[-3.7%, -0.4%]	2

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: 660.662s -> 659.815s (-0.13%)
Artifact size: 308.15 MiB -> 308.17 MiB (0.01%)

compiler-errors · 2024-01-26T22:11:54Z

Perf effect seems unremarkable.

compiler/rustc_ast/src/ast.rs

davidtwco

Implementation looks good, r=me after relevant approvals, etc.

Rework support for async closures; allow them to return futures that borrow from the closure's captures This PR implements a new lowering for async closures via `TyKind::CoroutineClosure` which handles the curious relationship between the closure and the coroutine that it returns. I wrote up a bunch in [this hackmd](https://hackmd.io/`@compiler-errors/S1HvqQxca)` which will be copied to the dev guide after this PR lands, and hopefully left sufficient comments in the source code explaining why this change is as large as it is. This also necessitates that they begin implementing the `AsyncFn`-family of traits, rather than the `Fn`-family of traits -- if you need `Fn` implementations, you should probably use the non-sugar `|| async {}` syntax instead. Notably this PR does not yet implement `async Fn()` syntax sugar for bounds, but I expect to add those soon (**edit:** rust-lang#120392). For now, users must use `AsyncFn()` traits directly, which necessitates adding the `async_fn_traits` feature gate as well. I will add this as a follow-up very soon. r? oli-obk This is based on top of rust-lang#120322, but that PR is minimal.

fmease

rustc_ast_pretty still needs to be updated to accommodate the addition.

compiler-errors · 2024-01-31T17:08:39Z

T-lang went over this in a meeting today saying there is no concern here. I'm gonna approve this as gated under the async_closure feature gate when it's green.

@rustbot label: -I-lang-nominated

compiler/rustc_ast_pretty/src/pprust/state.rs

compiler-errors · 2024-01-31T18:12:16Z

@bors r=davidtwco,fmease

bors · 2024-01-31T18:12:19Z

📌 Commit 3913c9a has been approved by davidtwco,fmease

It is now in the queue for this repository.

…i-obk Rework support for async closures; allow them to return futures that borrow from the closure's captures This PR implements a new lowering for async closures via `TyKind::CoroutineClosure` which handles the curious relationship between the closure and the coroutine that it returns. I wrote up a bunch in [this hackmd](https://hackmd.io/`@compiler-errors/S1HvqQxca)` which will be copied to the dev guide after this PR lands, and hopefully left sufficient comments in the source code explaining why this change is as large as it is. This also necessitates that they begin implementing the `AsyncFn`-family of traits, rather than the `Fn`-family of traits -- if you need `Fn` implementations, you should probably use the non-sugar `|| async {}` syntax instead. Notably this PR does not yet implement `async Fn()` syntax sugar for bounds, but I expect to add those soon (**edit:** rust-lang#120392). For now, users must use `AsyncFn()` traits directly, which necessitates adding the `async_fn_traits` feature gate as well. I will add this as a follow-up very soon. r? oli-obk This is based on top of rust-lang#120322, but that PR is minimal.

bors · 2024-02-06T00:45:14Z

⌛ Testing commit 3913c9a with merge f3b9d47...

bors · 2024-02-06T04:14:33Z

☀️ Test successful - checks-actions
Approved by: davidtwco,fmease
Pushing f3b9d47 to master...

rust-timer · 2024-02-06T06:06:09Z

Finished benchmarking commit (f3b9d47): comparison URL.

Overall result: ❌ regressions - 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.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	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)	-	-	0
Improvements ✅ (primary)	-1.3%	[-1.3%, -1.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.3%	[-1.3%, -1.3%]	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: 661.246s -> 662.71s (0.22%)
Artifact size: 308.13 MiB -> 308.15 MiB (0.01%)

…i-obk Rework support for async closures; allow them to return futures that borrow from the closure's captures This PR implements a new lowering for async closures via `TyKind::CoroutineClosure` which handles the curious relationship between the closure and the coroutine that it returns. I wrote up a bunch in [this hackmd](https://hackmd.io/`@compiler-errors/S1HvqQxca)` which will be copied to the dev guide after this PR lands, and hopefully left sufficient comments in the source code explaining why this change is as large as it is. This also necessitates that they begin implementing the `AsyncFn`-family of traits, rather than the `Fn`-family of traits -- if you need `Fn` implementations, you should probably use the non-sugar `|| async {}` syntax instead. Notably this PR does not yet implement `async Fn()` syntax sugar for bounds, but I expect to add those soon (**edit:** rust-lang#120392). For now, users must use `AsyncFn()` traits directly, which necessitates adding the `async_fn_traits` feature gate as well. I will add this as a follow-up very soon. r? oli-obk This is based on top of rust-lang#120322, but that PR is minimal.

internal: Parse (nightly) `const` and `async` trait bounds Both of these bound modifiers were added recently: * `const` trait bounds: rust-lang/rust#119099 * `async` trait bounds: rust-lang/rust#120392 The latter will certainly will not do the right thing; namely, `async Fn` needs to be mapped to the `AsyncFn` trait. IDK how to do that, so advice would be appreciated, though perhaps we could land this first so the parser isn't complaining about these bounds?

rustbot assigned davidtwco Jan 26, 2024

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 Jan 26, 2024

rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jan 26, 2024

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the async-bound-modifier branch from aefa8d7 to 792e3b5 Compare January 26, 2024 18:23

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the async-bound-modifier branch from 792e3b5 to d2bb2cc Compare January 26, 2024 19:12

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 Jan 26, 2024

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 26, 2024

compiler-errors mentioned this pull request Jan 26, 2024

Rework support for async closures; allow them to return futures that borrow from the closure's captures #120361

Merged

fmease reviewed Jan 26, 2024

View reviewed changes

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

davidtwco approved these changes Jan 29, 2024

View reviewed changes

traviscross added A-async-await Area: Async & Await WG-async Working group: Async & await labels Jan 31, 2024

fmease requested changes Jan 31, 2024

View reviewed changes

rust-cloud-vms bot force-pushed the async-bound-modifier branch from d2bb2cc to 7abcf33 Compare January 31, 2024 16:41

compiler-errors added 4 commits January 31, 2024 16:59

Add async bound modifier to enable async Fn bounds

0eb2adb

Add tests

cd2fd34

Better error message in ed 2015

54db272

Error on incorrect item kind in async bound

3913c9a

rust-cloud-vms bot force-pushed the async-bound-modifier branch from 7abcf33 to 3913c9a Compare January 31, 2024 16:59

rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Jan 31, 2024

fmease approved these changes Jan 31, 2024

View reviewed changes

compiler/rustc_ast_pretty/src/pprust/state.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 Jan 31, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 6, 2024

bors merged commit f3b9d47 into rust-lang:master Feb 6, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Feb 6, 2024

bors mentioned this pull request Feb 6, 2024

Implement new effects desugaring #120639

Merged

4 tasks

rustbot removed the perf-regression Performance regression. label Feb 6, 2024

compiler-errors mentioned this pull request Feb 16, 2024

internal: Parse (nightly) const and async trait bounds rust-lang/rust-analyzer#16588

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce support for `async` bound modifier on `Fn*` traits #120392

Introduce support for `async` bound modifier on `Fn*` traits #120392

compiler-errors commented Jan 26, 2024 •

edited

Loading

rustbot commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

This comment has been minimized.

This comment has been minimized.

rustbot commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

This comment has been minimized.

bors commented Jan 26, 2024

bors commented Jan 26, 2024

This comment has been minimized.

rust-timer commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

davidtwco left a comment

fmease left a comment

compiler-errors commented Jan 31, 2024

compiler-errors commented Jan 31, 2024

bors commented Jan 31, 2024

bors commented Feb 6, 2024

bors commented Feb 6, 2024

rust-timer commented Feb 6, 2024

Introduce support for async bound modifier on Fn* traits #120392

Introduce support for async bound modifier on Fn* traits #120392

Conversation

compiler-errors commented Jan 26, 2024 • edited Loading

rustbot commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

This comment has been minimized.

This comment has been minimized.

rustbot commented Jan 26, 2024

compiler-errors commented Jan 26, 2024

This comment has been minimized.

bors commented Jan 26, 2024

bors commented Jan 26, 2024

This comment has been minimized.

rust-timer commented Jan 26, 2024

Overall result: ❌ regressions - ACTION NEEDED

compiler-errors commented Jan 26, 2024

davidtwco left a comment

Choose a reason for hiding this comment

fmease left a comment

Choose a reason for hiding this comment

compiler-errors commented Jan 31, 2024

compiler-errors commented Jan 31, 2024

bors commented Jan 31, 2024

bors commented Feb 6, 2024

bors commented Feb 6, 2024

rust-timer commented Feb 6, 2024

Overall result: ❌ regressions - no action needed

Introduce support for `async` bound modifier on `Fn*` traits #120392

Introduce support for `async` bound modifier on `Fn*` traits #120392

compiler-errors commented Jan 26, 2024 •

edited

Loading