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

Confusing: Implementation of FnOnce is not general enough #89976

Open
Tracked by #110338
Darksonn opened this issue Oct 17, 2021 · 11 comments
Open
Tracked by #110338

Confusing: Implementation of FnOnce is not general enough #89976

Darksonn opened this issue Oct 17, 2021 · 11 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Darksonn
Copy link
Contributor

Someone on the Tokio discord came across the following bad compiler error message. Given the following code:

use std::sync::Arc;
use futures::stream::{self, StreamExt};

struct MyStruct {}

impl MyStruct {
    async fn import_all(&self, list: Arc<Vec<String>>) {
        stream::iter(&*list)
            .map(|txn| {
                let bd_c = list.clone();
                async move { self.import(bd_c, txn).await }
            })
            .buffer_unordered(10)
            .collect::<Vec<bool>>()
            .await;
    }
    
    async fn import(&self, list: Arc<Vec<String>>, now: &str) -> bool {
        list.iter().any(|v| v == now)
    }
}

#[tokio::main]
async fn main() {
    let ms = MyStruct {};
    
    tokio::spawn(async move {
        ms.import_all(Arc::new(vec![])).await;
    }).await.unwrap();
}

playground

The current output is:

error: implementation of `FnOnce` is not general enough
  --> src/main.rs:27:5
   |
27 |     tokio::spawn(async move {
   |     ^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(&'0 String) -> impl futures::Future` must implement `FnOnce<(&String,)>`, for any lifetime `'0`...
   = note: ...but it actually implements `FnOnce<(&String,)>`

This is an incredibly confusing error message, and it isn't in the right place. This fix is to change import to take an index instead of a reference in the last argument. Interestingly if you remove the main function, the error disappears. If you replace the import_all function with a function returning a boxed future, then the error happens in the import_all function instead.

cc @ryanobjc

@Darksonn Darksonn added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 17, 2021
@JanBeh
Copy link
Contributor

JanBeh commented Oct 21, 2021

I encountered a similar error in a different context, as described here. Not sure if these two cases are related at all.

@shepmaster
Copy link
Member

See also #85516

@jerry-best
Copy link

I just tripped over a very similar error. I resolved it similarly by handing around indexes instead of references, but I would have had no idea what was going on if not for this issue.

@petrosagg
Copy link
Contributor

I just hit this extremely confusing issue. My minimal reproduction seems to imply that depending on the incantation the compiler sometimes infers the lifetime on the closure as generic and sometimes as some specific lifetime. I have no idea why tokio::spawn() affects this though.

use futures::stream::{self, StreamExt};
use std::future::{ready, Ready};

/// Enforce the passed closure is generic over its lifetime
fn type_hint<F>(f: F) -> F
where
    F: for<'a> FnMut(&'a String) -> Ready<()>,
{
    f
}

// Closure correctly gets correctly inferred
async fn working1(input: &[String]) {
    let mut foo = stream::iter(input)
        .map(|_| ready(()))
        .buffer_unordered(5)
        .boxed();
    foo.next().await;
}

// removing the .boxed() makes it fail
async fn broken1(input: &[String]) {
    let mut foo = stream::iter(input)
        .map(|_| ready(()))
        .buffer_unordered(5);

    foo.next().await;
}

// adding a type hint makes it work again
async fn working2(input: &[String]) {
    let mut foo = stream::iter(input)
        .map(type_hint(|_| ready(())))
        .buffer_unordered(5);

    foo.next().await;
}

async fn spawner() {
    tokio::spawn(working1(&[]));
    tokio::spawn(broken1(&[]));
    tokio::spawn(working2(&[]));
    // Directly awaiting works fine though
    broken1(&[]).await;
}

Playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e08e92aa9cdfa05a0b99a66869f9761a

@guswynn
Copy link
Contributor

guswynn commented Mar 22, 2022

@petrosagg it appears to be referring to FnOnce, but the type signature of the FnMut being passed into the map?
this makes me think the compiler is confusing 2 trait bounds somewhere, very cursed

@Niedzwiedzw
Copy link

happened to me today

fn spawn_synchronisation_task(
    daemon: PompaDaemon,
    location: crate::models::Location,
    expected_dates: Vec<Range>,
) {
    tokio::task::spawn(daemon.sync_data_for(location, expected_dates));
}

for some reason this produces a cryptic error:

error: implementation of `FnOnce` is not general enough
   --> prometeusz/src/pompa_states.rs:426:5
    |
426 |     tokio::task::spawn(daemon.sync_data_for(location, expected_dates));
    |     ^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
    |
    = note: closure with signature `fn(&'0 models::Product) -> impl futures::Future<Output = std::result::Result<HashSet<(NaiveDateTime, NaiveDateTime)>, ErrReport>>` must implement `FnOnce<(&models::Product,)>`, for any lifetime `'0`...
    = note: ...but it actually implements `FnOnce<(&models::Product,)>`

@jakubdabek
Copy link
Contributor

@petrosagg tokio::spawn requires the future to be Send, and that's what triggers the error.
On today's nightly (and beta 1.63.0-beta.2) on the playground with the following example the error is different (same one is with tokio::spawn):

use futures::prelude::*;

fn foo() {
    fn assert_send<T: Send>(_: T) {}
    assert_send(bar());
}

async fn bar() -> Vec<i32> {
    let ints = [1, 2, 3];
    futures::stream::iter(&ints)
        .map(|i| async move { i.clone() })
        .buffer_unordered(2)
        .collect()
        .await
}

Error message:

error: higher-ranked lifetime error
 --> src/lib.rs:5:5
  |
5 |     assert_send(bar());
  |     ^^^^^^^^^^^^^^^^^^
  |
  = note: could not prove `impl futures::Future<Output = Vec<i32>>: std::marker::Send`

It explains why the error happened, but gives zero information about what could be wrong, the not general enough one could at least somewhat pinpoint the location.
In this example it can be fixed by using futures::stream::iter(ints) instead.

@petrosagg's example also triggers a different error on nightly/beta:

error: implementation of `Iterator` is not general enough
  --> src/lib.rs:44:5
   |
44 |     assert_send(broken1(&[]));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Iterator` is not general enough
   |
   = note: `Iterator` would have to be implemented for the type `std::slice::Iter<'0, String>`, for any lifetime `'0`...
   = note: ...but `Iterator` is actually implemented for the type `std::slice::Iter<'1, String>`, for some specific lifetime `'1`

error: implementation of `FnOnce` is not general enough
  --> src/lib.rs:44:5
   |
44 |     assert_send(broken1(&[]));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(&'0 String) -> std::future::Ready<()>` must implement `FnOnce<(&String,)>`, for any lifetime `'0`...
   = note: ...but it actually implements `FnOnce<(&String,)>`

@Yossipossi1
Copy link

Thought I'd note that I got a similar error recently (on Rust 1.77.2 stable). @petrosagg's type_hint solution worked for me, just in case anyone else is still scratching their heads over this.

@rbtcollins
Copy link
Contributor

using regular iterators doing chain.peekable will trigger this.

// given that tag and added are &Vec<String> owned by self

    for tag in added.iter().chain(removed.iter()).peekable() {
          writer.write_all(tag.as_bytes()).await?;
   }

currently seeing if I can figure out an appropriate type_hint for this as there are no closures involved in my code at least

@GopherJ
Copy link

GopherJ commented Aug 19, 2024

encountered this issue in a completely different case:

    async fn new_trace_lde<E: FieldElement<BaseField = Felt>>(
        &self,
        trace_info: &TraceInfo,
        main_trace: &ColMatrix<Felt>,
        domain: &StarkDomain<Felt>,
    ) -> (Self::TraceLde<E>, TracePolyTable<E>) {
        WebGPUTraceLde::new(trace_info, main_trace, domain, self.webgpu_hash_fn).await
    }

@nhtyy
Copy link

nhtyy commented Oct 11, 2024

I think this is a minimal reproduction of this bug

use std::convert::identity;
use std::future::Future;

fn main() {
    println!("Hello, world!");
}

fn do_stuff() -> impl Future<Output = ()> + Send {
    async move {
        // any lifetime even static
        let arr = [0; 10];

        let holder = IntoIterWrapper {
            calls: arr.iter().map(|x| x),
        };

        holder.do_stuff().await;
    }
}

pub struct IntoIterWrapper<I> {
    calls: I,
}

impl<I, T> IntoIterWrapper<I>
where
    I: IntoIterator<Item = T>,
{
    pub async fn do_stuff(self) {
        // this must be assigned
        let iter = self.calls.into_iter();
        
        // any await point will make this not compile
        std::future::ready(()).await;
    }
}

notably there are no bounds that will satisfy this on the impl IntoIterWrapper block afaict...
The send is required on do_stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests