Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return #129072

compiler-errors · 2024-08-14T01:46:44Z

In #127482, I implemented the functionality to infer an async closure signature when passed into a function that has Fn + Future where clauses that look like:

fn whatever(callback: F)
where
  F: Fn(Arg) -> Fut,
  Fut: Future<Output = Out>,

However, #127781 demonstrates that this is still incomplete to address the cases users care about. So let's not bail when we fail to find a Future bound, and try our best to just use the args from the Fn bound if we find it. This is fine since most users of closures only really care about the argument types for inference guidance, since we require the receiver of a . method call to be known in order to probe methods.

When I experimented with programmatically rewriting || async {} to async || {} in #127827, this also seems to have fixed ~5000 regressions (probably all coming from usages TryFuture/TryStream from futures-rs): the before and after crater runs.

Fixes #127781.

rustbot · 2024-08-14T01:46:51Z

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

nnethercote · 2024-08-14T03:05:49Z

I'm not a good person to review a PR involving types and a "SUBTLE" comment :)

r? @lcnr

lcnr · 2024-08-14T06:18:54Z

compiler/rustc_hir_typeck/src/closure.rs

+        let return_ty =
+            return_ty.unwrap_or_else(|| self.next_ty_var(cause_span.unwrap_or(DUMMY_SP)));


why directly use the input_tys but create a new infer var for the return type? You could also reuse that one, can't u

The return type vid that is in scope is the type of the future, but we want the type of the future's output. We don't have it, so we need to make a new type var here.

comment please :3

lcnr

sgtm

lcnr · 2024-08-14T06:35:09Z

@rustbot author

…ing Future bound

lcnr · 2024-08-15T10:06:35Z

@bors r+ rollup

bors · 2024-08-15T10:06:38Z

📌 Commit 4290943 has been approved by lcnr

It is now in the queue for this repository.

…c-closure-inference, r=lcnr Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return In rust-lang#127482, I implemented the functionality to infer an async closure signature when passed into a function that has `Fn` + `Future` where clauses that look like: ``` fn whatever(callback: F) where F: Fn(Arg) -> Fut, Fut: Future<Output = Out>, ``` However, rust-lang#127781 demonstrates that this is still incomplete to address the cases users care about. So let's not bail when we fail to find a `Future` bound, and try our best to just use the args from the `Fn` bound if we find it. This is *fine* since most users of closures only really care about the *argument* types for inference guidance, since we require the receiver of a `.` method call to be known in order to probe methods. When I experimented with programmatically rewriting `|| async {}` to `async || {}` in rust-lang#127827, this also seems to have fixed ~5000 regressions (probably all coming from usages `TryFuture`/`TryStream` from futures-rs): the [before](rust-lang#127827 (comment)) and [after](rust-lang#127827 (comment)) crater runs. Fixes rust-lang#127781.

…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#128348 (Unconditionally allow shadow call-stack sanitizer for AArch64) - rust-lang#128922 (rust-analyzer: use in-tree `pattern_analysis` crate) - rust-lang#128935 (More work on `zstd` compression) - rust-lang#129072 (Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return) - rust-lang#129101 (Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass) - rust-lang#129106 (Remove redundant type ops: `Eq`/`Subtype`) - rust-lang#129122 (Remove duplicated `Rustdoc::output` method from `run-make-support` lib) - rust-lang#129124 (rustdoc-json: Use FxHashMap from rustdoc_json_types) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#128348 (Unconditionally allow shadow call-stack sanitizer for AArch64) - rust-lang#129065 (Use `impl PartialEq<TokenKind> for Token` more.) - rust-lang#129072 (Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return) - rust-lang#129096 (Print more verbose error for commands that capture output) - rust-lang#129101 (Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass) - rust-lang#129106 (Remove redundant type ops: `Eq`/`Subtype`) - rust-lang#129122 (Remove duplicated `Rustdoc::output` method from `run-make-support` lib) - rust-lang#129124 (rustdoc-json: Use FxHashMap from rustdoc_json_types) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#129072 - compiler-errors:more-powerful-async-closure-inference, r=lcnr Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return In rust-lang#127482, I implemented the functionality to infer an async closure signature when passed into a function that has `Fn` + `Future` where clauses that look like: ``` fn whatever(callback: F) where F: Fn(Arg) -> Fut, Fut: Future<Output = Out>, ``` However, rust-lang#127781 demonstrates that this is still incomplete to address the cases users care about. So let's not bail when we fail to find a `Future` bound, and try our best to just use the args from the `Fn` bound if we find it. This is *fine* since most users of closures only really care about the *argument* types for inference guidance, since we require the receiver of a `.` method call to be known in order to probe methods. When I experimented with programmatically rewriting `|| async {}` to `async || {}` in rust-lang#127827, this also seems to have fixed ~5000 regressions (probably all coming from usages `TryFuture`/`TryStream` from futures-rs): the [before](rust-lang#127827 (comment)) and [after](rust-lang#127827 (comment)) crater runs. Fixes rust-lang#127781.

rustbot assigned nnethercote Aug 14, 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 Aug 14, 2024

compiler-errors added the F-async_closure `#![feature(async_closure)]` label Aug 14, 2024

rustbot assigned lcnr and unassigned nnethercote Aug 14, 2024

compiler-errors force-pushed the more-powerful-async-closure-inference branch from 5a022a3 to 84d6723 Compare August 14, 2024 03:28

lcnr reviewed Aug 14, 2024

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 Aug 14, 2024

Infer async closure args from Fn bound even if there is no correspond…

4290943

…ing Future bound

compiler-errors force-pushed the more-powerful-async-closure-inference branch from 84d6723 to 4290943 Compare August 14, 2024 19:33

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 Aug 15, 2024

GuillaumeGomez mentioned this pull request Aug 15, 2024

Rollup of 8 pull requests #129126

Closed

matthiaskrgr mentioned this pull request Aug 15, 2024

Rollup of 8 pull requests #129130

Merged

bors merged commit 53bf554 into rust-lang:master Aug 15, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return #129072

Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return #129072

compiler-errors commented Aug 14, 2024

rustbot commented Aug 14, 2024

nnethercote commented Aug 14, 2024

lcnr Aug 14, 2024 •

edited

Loading

compiler-errors Aug 14, 2024

lcnr Aug 14, 2024

lcnr left a comment

lcnr commented Aug 14, 2024

lcnr commented Aug 15, 2024

bors commented Aug 15, 2024

		let return_ty =
		return_ty.unwrap_or_else(\|\| self.next_ty_var(cause_span.unwrap_or(DUMMY_SP)));

Infer async closure args from Fn bound even if there is no corresponding Future bound on return #129072

Infer async closure args from Fn bound even if there is no corresponding Future bound on return #129072

Conversation

compiler-errors commented Aug 14, 2024

rustbot commented Aug 14, 2024

nnethercote commented Aug 14, 2024

lcnr Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors Aug 14, 2024

Choose a reason for hiding this comment

lcnr Aug 14, 2024

Choose a reason for hiding this comment

lcnr left a comment

Choose a reason for hiding this comment

lcnr commented Aug 14, 2024

lcnr commented Aug 15, 2024

bors commented Aug 15, 2024

Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return #129072

Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return #129072

lcnr Aug 14, 2024 •

edited

Loading