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

async closure parameter type inference does not work anymore #127425

Closed
peku33 opened this issue Jul 6, 2024 · 7 comments · Fixed by #127482
Closed

async closure parameter type inference does not work anymore #127425

peku33 opened this issue Jul 6, 2024 · 7 comments · Fixed by #127482
Labels
C-bug Category: This is a bug. F-async_closure `#![feature(async_closure)]`

Comments

@peku33
Copy link

peku33 commented Jul 6, 2024

When using async closures, rust since around 1.80 fails to infer parameter type in most scenarios.

#![feature(async_closure)]

use anyhow::Error;
use futures::{stream::repeat_with, StreamExt};

fn accept_str(_: &str) {}
fn accept_string(_: &String) {}

#[tokio::main(flavor = "current_thread")]
async fn main() -> Result<(), Error> {
    // when using closure returning async block - everything works
    repeat_with(|| "foo".to_owned())
        .take(1)
        .for_each(|value| async move {
            accept_str(&value); // ok
            accept_str(value.as_str()); // ok
            accept_string(&value); // ok
        })
        .await;

    // when using async closure, most options fail
    repeat_with(|| "foo".to_owned())
        .take(1)
        .for_each(async move |value| {
            // error on whole closure, rust thinks that type of value is `str`
            // accept_str(&value);

            // type annotations needed, cannot infer type
            // accept_str(value.as_str());

            // this works
            accept_string(&value); // ok
        })
        .await;

    // can be fixed by providing type hint on async closure parameter
    repeat_with(|| "foo".to_owned())
        .take(1)
        .for_each(async move |value: String| {
            accept_str(&value); // ok
            accept_str(value.as_str()); // ok
            accept_string(&value); // ok
        })
        .await;

    Ok(())
}

I expected to see this happen: All options used to work in past versions of rust

Instead, this happened: Without explicit type hint it fails

Version it worked on

It used to work in the past, around 1.79

Version with regression

rustc 1.81.0-nightly (524d806c6 2024-07-05)
binary: rustc
commit-hash: 524d806c62a82ecc0cf8634b94997ae506f4d6f9
commit-date: 2024-07-05
host: x86_64-pc-windows-msvc
release: 1.81.0-nightly
LLVM version: 18.1.7
@peku33 peku33 added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jul 6, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 6, 2024
@compiler-errors compiler-errors removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Jul 6, 2024
@compiler-errors
Copy link
Member

We don't typically mark nightly features with regression-* labels, so I'm removing those.

Are you certain this regressed in 1.80? This should have regressed in #120361, when I reimplemented async closures completely.

@compiler-errors compiler-errors added the F-async_closure `#![feature(async_closure)]` label Jul 6, 2024
@peku33
Copy link
Author

peku33 commented Jul 6, 2024

I am not 100% sure when it happened. I used to work with 1.7x a couple of months ago. Then I left my project for a moment. When I came back, after updating to 1.81 it no longer works. IIRC before leaving it was around 1.79, but I'm not sure.

@compiler-errors
Copy link
Member

compiler-errors commented Jul 6, 2024

This limitation to async closure signature inference has to do with the fact that the combinators you're using (e.g. StreamExt::for_each) take regular Fn trait bounds. Is there a particular reason you're using async closures here? They're really not gaining you anything other than a dependency on the nightly compiler.

@peku33
Copy link
Author

peku33 commented Jul 6, 2024

I posted a minimal example, indeed async does not make much sense here, but in project I work on it does. As a workaround I've just annotated parameters with original types, but I still believe this is a bug.

@GrigorenkoPV
Copy link
Contributor

This should have regressed in #120361, when I reimplemented async closures completely.

This indeed regressed in #120361.

@traviscross traviscross removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 6, 2024
@bors bors closed this as completed in 72199b2 Jul 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 8, 2024
Rollup merge of rust-lang#127482 - compiler-errors:closure-two-par-sig-inference, r=oli-obk

Infer async closure signature from (old-style) two-part `Fn` + `Future` bounds

When an async closure is passed to a function that has a "two-part" `Fn` and `Future` trait bound, like:

```rust
use std::future::Future;

fn not_exactly_an_async_closure(_f: F)
where
    F: FnOnce(String) -> Fut,
    Fut: Future<Output = ()>,
{}
```

The we want to be able to extract the signature to guide inference in the async closure, like:

```rust
not_exactly_an_async_closure(async |string| {
    for x in string.split('\n') { ... }
    //~^ We need to know that the type of `string` is `String` to call methods on it.
})
```

Closure signature inference will see two bounds: `<?F as FnOnce<Args>>::Output = ?Fut`, `<?Fut as Future>::Output = String`. We need to extract the signature by looking through both projections.

### Why?

I expect the ecosystem's move onto `async Fn` trait bounds (which are not affected by this PR, and already do signature inference fine) to be slow. In the mean time, I don't see major overhead to supporting this "old–style" of trait bounds that were used to model async closures.

r? oli-obk
Fixes rust-lang#127468
Fixes rust-lang#127425
@peku33
Copy link
Author

peku33 commented Jul 15, 2024

While the problem from first message in issue seems to be fixed, there are still cases when this fails.

One of examples is when using try_for_each instead of for_each, when closure is expected to return Result:

#![feature(async_closure)]

use anyhow::Error;
use futures::{stream::repeat_with, StreamExt, TryStreamExt};

fn accept_str(_: &str) {}
fn accept_string(_: &String) {}

#[tokio::main(flavor = "current_thread")]
async fn main() -> Result<(), Error> {

    repeat_with(|| "foo".to_owned())
        .take(1)
        .map(Result::<_, Error>::Ok)
        .try_for_each(async move |value| {
            // error on whole closure, rust thinks that type of value is `str`
            accept_str(&value);

            // type annotations needed, cannot infer type
            accept_str(value.as_str());

            // this works
            accept_string(&value); // ok
            
            Ok(())
        })
        .await?;

    Ok(())
}

Should I open new issue, or this one will be reopened?

@compiler-errors
Copy link
Member

Please open a new issue, since it's likely a different root cause

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-async_closure `#![feature(async_closure)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants