Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't manually resolve async closures in rustc_resolve #120322

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Jan 24, 2024

There's a comment here that talks about doing this "[so] closure [args] are detected as upvars rather than normal closure arg usages", but we do upvar analysis on the HIR now:

let mut local_collector = LocalCollector::default();
local_collector.visit_body(body);
let mut capture_collector = CaptureCollector {
tcx,
locals: &local_collector.locals,
upvars: FxIndexMap::default(),
};
capture_collector.visit_body(body);

Removing this ad-hoc logic makes it so that async |x: &str| now introduces an implicit binder, like regular closures.

r? @oli-obk

@rustbot 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 24, 2024
// No need to resolve return type --
// the outer closure return type is `FnRetTy::Default`.

// Now resolve the inner closure
Copy link
Member Author

@compiler-errors compiler-errors Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, this code was literally not doing anything at all special for the inner async block. It's not introducing an inner ribs or anything, lol.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 25, 2024

lol nice

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 25, 2024

📌 Commit 8c2ae80 has been approved by oli-obk

It is now in the queue for this repository.

@bors 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 25, 2024
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jan 26, 2024
…c-closures, r=oli-obk

Don't manually resolve async closures in `rustc_resolve`

There's a comment here that talks about doing this "[so] closure [args] are detected as upvars rather than normal closure arg usages", but we do upvar analysis on the HIR now:

https://github.com/rust-lang/rust/blob/cd6d8f2a04528f827ad3d399581c0f3502b15a72/compiler/rustc_passes/src/upvars.rs#L21-L29

Removing this ad-hoc logic makes it so that `async |x: &str|` now introduces an implicit binder, like regular closures.

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#118803 (Add the `min_exhaustive_patterns` feature gate)
 - rust-lang#119286 (show linker output even if the linker succeeds)
 - rust-lang#119616 (Add a new `wasm32-wasi-preview2` target)
 - rust-lang#120124 (Split assembly tests for ELF and MachO)
 - rust-lang#120201 (Bump some deps with syn 1.0 dependencies)
 - rust-lang#120204 (Builtin macros effectively have implicit #[collapse_debuginfo(yes)])
 - rust-lang#120322 (Don't manually resolve async closures in `rustc_resolve`)
 - rust-lang#120356 (Fix broken markdown in csky-unknown-linux-gnuabiv2.md)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2024
…c-closures, r=oli-obk

Don't manually resolve async closures in `rustc_resolve`

There's a comment here that talks about doing this "[so] closure [args] are detected as upvars rather than normal closure arg usages", but we do upvar analysis on the HIR now:

https://github.com/rust-lang/rust/blob/cd6d8f2a04528f827ad3d399581c0f3502b15a72/compiler/rustc_passes/src/upvars.rs#L21-L29

Removing this ad-hoc logic makes it so that `async |x: &str|` now introduces an implicit binder, like regular closures.

r? ``@oli-obk``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#107464 (Add `str::Lines::remainder`)
 - rust-lang#118803 (Add the `min_exhaustive_patterns` feature gate)
 - rust-lang#119466 (Initial implementation of `str::from_raw_parts[_mut]`)
 - rust-lang#120053 (Specialize `Bytes` on `StdinLock<'_>`)
 - rust-lang#120124 (Split assembly tests for ELF and MachO)
 - rust-lang#120204 (Builtin macros effectively have implicit #[collapse_debuginfo(yes)])
 - rust-lang#120322 (Don't manually resolve async closures in `rustc_resolve`)
 - rust-lang#120356 (Fix broken markdown in csky-unknown-linux-gnuabiv2.md)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#107464 (Add `str::Lines::remainder`)
 - rust-lang#118803 (Add the `min_exhaustive_patterns` feature gate)
 - rust-lang#119466 (Initial implementation of `str::from_raw_parts[_mut]`)
 - rust-lang#120053 (Specialize `Bytes` on `StdinLock<'_>`)
 - rust-lang#120124 (Split assembly tests for ELF and MachO)
 - rust-lang#120204 (Builtin macros effectively have implicit #[collapse_debuginfo(yes)])
 - rust-lang#120322 (Don't manually resolve async closures in `rustc_resolve`)
 - rust-lang#120356 (Fix broken markdown in csky-unknown-linux-gnuabiv2.md)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e400311 into rust-lang:master Jan 26, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
Rollup merge of rust-lang#120322 - compiler-errors:higher-ranked-async-closures, r=oli-obk

Don't manually resolve async closures in `rustc_resolve`

There's a comment here that talks about doing this "[so] closure [args] are detected as upvars rather than normal closure arg usages", but we do upvar analysis on the HIR now:

https://github.com/rust-lang/rust/blob/cd6d8f2a04528f827ad3d399581c0f3502b15a72/compiler/rustc_passes/src/upvars.rs#L21-L29

Removing this ad-hoc logic makes it so that `async |x: &str|` now introduces an implicit binder, like regular closures.

r? ```@oli-obk```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2024
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 added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2024
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 added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2024
…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 added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2024
…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 added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2024
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants