rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
93.58k stars 12.05k forks source link

Weird type inference around impl Trait and async #124757

Open Pzixel opened 1 week ago

Pzixel commented 1 week ago

Following code doesn't work on all the compilers I've checked, e.g. 1.80.0-nightly

#![allow(warnings)]
use std::{collections::HashSet, future::Future};

trait MyTrait {
    fn blah(&self, x: impl Iterator<Item = u32>) -> impl Future<Output = ()> + Send;
}

fn foo<T: MyTrait + Send + Sync>(val: T, unique_x: HashSet<u32>) -> impl Future<Output = ()> + Send {
    let cached = HashSet::new();
    async move {
        let xs = unique_x.union(&cached)
            // .copied() // works
            .map(|x| *x) // error
            ;
        let blah = val.blah(xs.into_iter()).await;
    }
}

This is surprising because a lot of alterations to this code fixes the issue, for example replacing map(|x| *x) withcopied(), which is seemingly an equivalent replacement. Moreover, replacing impl Future with Self::Future also solves the problem:

#![allow(warnings)]
use std::{collections::HashSet, future::Future};

trait MyTrait {
    type F: Future<Output = ()> + Send;
    fn blah(&self, x: impl Iterator<Item = u32>) -> Self::F;
}

fn foo<T: MyTrait + Send + Sync>(val: T, unique_x: HashSet<u32>) -> impl Future<Output = ()> + Send {
    let cached = HashSet::new();
    async move {
        let xs = unique_x.union(&cached).map(|x| *x);
        let blah = val.blah(xs.into_iter()).await;
    }
}

Also the error text is very cryptic:

error: implementation of `FnOnce` is not general enough
  --> src/main.rs:10:5
   |
10 | /     async move {
11 | |         let xs = unique_x.union(&cached).map(|x| x.len().to_string())
12 | |             //.collect::<Vec<_>>()
13 | |             ;
14 | |         let blah = val.blah(xs.into_iter()).await;
15 | |     }
   | |_____^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(&'0 String) -> String` must implement `FnOnce<(&'1 String,)>`, for any two lifetimes `'0` and `'1`...
   = note: ...but it actually implements `FnOnce<(&String,)>`

And because of how async works it can easily leak across the functions:

#![allow(warnings)]
use std::{collections::HashSet, future::Future};

trait MyTrait {
    fn blah(&self, x: impl Iterator<Item = String>) -> impl Future<Output = ()> + Send;
}

async fn foo<T: MyTrait + Send + Sync>(val: T, unique_x: HashSet<String>) {
    let cached = HashSet::new();
    let xs = unique_x.union(&cached).map(|x| x.len().to_string());
    let blah = val.blah(xs.into_iter()).await;
}

fn bar<T: MyTrait + Send + Sync>(val: T, unique_x: HashSet<String>) -> impl Future<Output = ()> + Send  {
    foo(val, unique_x)
}

This function reports

error: implementation of `FnOnce` is not general enough
  --> src/main.rs:15:5
   |
15 |     foo(val, unique_x)
   |     ^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(&'0 String) -> String` must implement `FnOnce<(&'1 String,)>`, for any two lifetimes `'0` and `'1`...
   = note: ...but it actually implements `FnOnce<(&String,)>`

In a function without a single lifetime argument. You may get some very confusing on the very top of your async stack because somewhere underneath there is one future that violates the Send constraint.

I'm not entirely sure if this is a diganostic error or a compiler issue, but i believe that the original code with impl Future should have been managed to infer that the resulting future is Send. the fact that this doesn't work:

trait MyTrait {
    fn blah(&self, x: impl Iterator<Item = String>) -> impl Future<Output = ()> + Send;
}

But this does

trait MyTrait {
    type F: Future<Output = ()> + Send;
    fn blah(&self, x: impl Iterator<Item = String>) -> Self::F;
}

Makes me think that this have to be a compiler issue

compiler-errors commented 1 week ago

This is #110338, probably #110339.

Pzixel commented 1 week ago

110339 looks different, at least I couldn't fix it with extra async blocks, #110338 is a very big umbrella issue, so maybe some of issues listed in there relate to this.