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

Spurious "implementation of FnOnce is not general enough" error #99492

Open
mystenmark opened this issue Jul 20, 2022 · 18 comments
Open

Spurious "implementation of FnOnce is not general enough" error #99492

mystenmark opened this issue Jul 20, 2022 · 18 comments
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. P-medium Medium priority S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@mystenmark
Copy link

mystenmark commented Jul 20, 2022

Hello rustaceans, I'll start with the brief overview and provide repro instructions at the end:

Given the following code:

    pub async fn spawn_execute_process(self: Arc<Self>) -> JoinHandle<()> {
        tokio::task::spawn(async move {
            execution_process(&self).await;
        })
    }

The current output is:

error: implementation of `FnOnce` is not general enough
   --> crates/sui-core/src/authority_active.rs:327:9
    |
327 |         tokio::task::spawn(async move {
    |         ^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
    |
    = note: closure with signature `fn(&'0 (u64, sui_types::base_types::TransactionDigest)) -> u64` must implement `FnOnce<(&(u64, sui_types::base_types::TransactionDigest),)>`, for any lifetime `'0`...
    = note: ...but it actually implements `FnOnce<(&(u64, sui_types::base_types::TransactionDigest),)>`

This is pretty fishy looking already - as you can see there is no closure in the code above, and the async block doesn't look anything like (u64, TransactionDigest) -> u64

I did however add such a closure in another file. And sure enough if I remove that closure from that other file, this problem goes away.

Unfortunately I don't have time to try to pare this down to a minimal repro right now, but I can provide a repo link.

Instructions:

  • git clone https://github.com/mystenmark/sui.git
  • cd sui
  • git checkout rust-bug-repro (commit description is "Reproduce the error at this commit")
  • cargo check
  • Observe the error
  • git checkout rust-bug-repro-workaround (commit description is "Make the error magically go away")
  • cargo check
  • Observe that the error is gone

As you can see the commit that fixes the error isn't even in the same file, nor does it alter any public function signatures or type defintions: mystenmark/sui@73302e1

Lastly, this is happens on both rust 1.62.0 and 1.62.1 - the branch above uses 1.62.1

@mystenmark mystenmark 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 Jul 20, 2022
mystenmark added a commit to mystenmark/sui that referenced this issue Jul 20, 2022
@mystenmark
Copy link
Author

Update: I found an even simpler workaround for the bug, which may be telling: mystenmark/sui@82e936b

(available as branch rust-bug-repro-workaround-2)

mystenmark added a commit to MystenLabs/sui that referenced this issue Jul 20, 2022
mystenmark added a commit to MystenLabs/sui that referenced this issue Jul 20, 2022
mystenmark added a commit to MystenLabs/sui that referenced this issue Jul 21, 2022
* Use a single NodeSyncState owned by ActiveAuthority

* Refactor node_sync to support execution driver

* Execution driver uses NodeSync for cert execution

* Work around rust-lang/rust#99492

* Increase limit so tests pass
punwai pushed a commit to MystenLabs/sui that referenced this issue Jul 27, 2022
@tmandry
Copy link
Member

tmandry commented Aug 11, 2022

@rustbot label t-compiler I-nominated A-async-await

@tmandry tmandry added A-async-await Area: Async & Await I-compiler-nominated Nominated for discussion during a compiler team meeting. labels Aug 11, 2022
@eholk
Copy link
Contributor

eholk commented Aug 15, 2022

@rustbot label AsyncAwait-triaged

@rustbot rustbot added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Aug 15, 2022
@tmandry
Copy link
Member

tmandry commented Aug 15, 2022

t-compiler: I nominated this for discussion because of the following:

This is pretty fishy looking already - as you can see there is no closure in the code above, and the async block doesn't look anything like (u64, TransactionDigest) -> u64

I did however add such a closure in another file. And sure enough if I remove that closure from that other file, this problem goes away.

This points to deeper compiler issues and is probably at least P-high.

@rustbot label +I-prioritize +E-needs-mcve +E-needs-bisection

@rustbot rustbot added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 15, 2022
@steffahn
Copy link
Member

steffahn commented Aug 15, 2022

Reduced (but can presumably be reduced further from here):

use futures::{stream, StreamExt};
use std::sync::Arc;
use tokio::task::JoinHandle;
async fn execution_process<A>(active_authority: &ActiveAuthority<A>)
where
    A: Send + Sync + 'static,
{
    stream::empty::<Result<(), ()>>()
        .zip(stream::iter(Vec::<u64>::new().iter().map(|seq| *seq)))
        .filter_map(|(result, seq)| async move { result.ok().map(|_| seq) })
        .collect::<Vec<_>>()
        .await;
}
struct ActiveAuthority<A> {
    node_sync_state: A,
}
impl<A> ActiveAuthority<A>
where
    A: Send + Sync + 'static,
{
    async fn spawn_execute_process(self: Arc<Self>) -> JoinHandle<()> {
        tokio::task::spawn(async move {
            execution_process(&self).await;
        })
    }
}
[dependencies]
futures = "0.3.23"
tokio = { version = "1.20.1", features = ["rt"] }

output 1.62.1

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

output 1.63.0

error: higher-ranked lifetime error
  --> src/main.rs:22:9
   |
22 | /         tokio::task::spawn(async move {
23 | |             execution_process(&self).await;
24 | |         })
   | |__________^
   |
   = note: could not prove impl futures::Future<Output = ()>: std::marker::Send

E.g. using .copied() instead of .map(|seq| *seq) makes the code compile again, for some reason.

@steffahn
Copy link
Member

Reducing further, I came across a second similar error message. This is what I’ve got at the moment:

use std::marker::PhantomData;

#[rustfmt::skip]
pub struct Struct<I, T, F, B>(
    PhantomData<fn() -> (I, T, F, B)>,
    PhantomData<<Self as It>::Item>,
)
where
    I: It<Item = T>, // xxx
    F: FnMut(T) -> B, // yyy
;

impl<I, T, F, B> Struct<I, T, F, B>
where
    I: It<Item = T>, // xxx
    F: FnMut(T) -> B, // yyy
{
    fn new(_: F) -> Self {
        Self(PhantomData, PhantomData)
    }
}
impl<B, I, T, F> It for Struct<I, T, F, B>
where
    I: It<Item = T>, // xxx
    F: FnMut(T) -> B, // yyy
{
    type Item = B;
}

pub trait It {
    type Item;
}

async fn execution_process() {
    let _x = Struct::<Empty<&u64>, &u64, _, u64>::new(|seq: &u64| *seq);
    async {}.await;
}

fn spawn_execute_process() {
    spawn(execution_process());
}

fn spawn<T>(_: T)
where
    T: Send, // zzz
{
}

struct FnReturning<T>(fn() -> T);

pub struct Empty<T>(PhantomData<FnReturning<T>>);

impl<T> It for Empty<T> {
    type Item = T;
}
error[E0308]: mismatched types
  --> src/lib.rs:40:5
   |
40 |     spawn(execution_process());
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
   |
   = note: expected reference `&u64`
              found reference `&u64`
note: the lifetime requirement is introduced here
  --> src/lib.rs:45:8
   |
45 |     T: Send, // zzz
   |        ^^^^

error: implementation of `FnOnce` is not general enough
  --> src/lib.rs:40:5
   |
40 |     spawn(execution_process());
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(&'0 u64) -> u64` must implement `FnOnce<(&'1 u64,)>`, for any two lifetimes `'0` and `'1`...
   = note: ...but it actually implements `FnOnce<(&u64,)>`

For more information about this error, try `rustc --explain E0308`.

Commenting out all the // xxx lines or all the // yyy lines makes either error go away. Commenting out the // zzz line makes all errors disappear.

In this reduced example, the errors stay the same between 1.62 and 1.63. As well as older versions and nightly, so there’s nothing to bisect (at least in this reduced example; the original did produce a “higher-ranked lifetime error” instead starting from 1.63; I don’t know if there’s much use in bisecting that though…).

@rustbot label -E-needs-mcve, -E-needs-bisection

@rustbot rustbot removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Aug 15, 2022
@steffahn
Copy link
Member

Here’s a further reduction to the non-closure-involving error

use std::marker::PhantomData;

pub struct Struct<I, T>(PhantomData<fn() -> <Self as It>::Item>)
where
    Self: It;

impl<I> It for Struct<I, I::Item>
where
    I: It,
{
    type Item = ();
}

pub trait It {
    type Item;
}

fn f() -> impl Send {
    async {
        let _x = Struct::<Empty<&'static ()>, _>(PhantomData);
        async {}.await;
    }
}

pub struct Empty<T>(PhantomData<fn() -> T>);

impl<T> It for Empty<T> {
    type Item = T;
}

error in 1.62

error[E0308]: mismatched types
  --> src/main.rs:18:11
   |
18 | fn f() -> impl Send {
   |           ^^^^^^^^^ one type is more general than the other
   |
   = note: expected reference `&()`
              found reference `&()`

error in 1.63

error[E0308]: mismatched types
  --> src/main.rs:19:5
   |
19 | /     async {
20 | |         let _x = Struct::<Empty<&'static ()>, _>(PhantomData);
21 | |         async {}.await;
22 | |     }
   | |_____^ one type is more general than the other
   |
   = note: expected reference `&()`
              found reference `&()`

For more information about this error, try `rustc --explain E0308`.

@steffahn
Copy link
Member

The original code example from #90656 seems to be behaving similar to the code in this issue, including the change to a “higher-ranked lifetime error” in 1.63.

@steffahn
Copy link
Member

steffahn commented Aug 15, 2022

I did however add such a closure in another file. And sure enough if I remove that closure from that other file, this problem goes away.

This points to deeper compiler issues and is probably at least P-high.

@tmandry

Note that even though the code that changed was “in another file”, this action-at-a-distance is explainable due to auto-traits. The : Send requirement was in one place, and the async fn body it ultimately depends on in another one. You can see in my initial reduction (which did involve one step of inlining some other call into execution_process during the reduction) how directly related the two pieces of code are where the error message appears and where the change that “fixes” the program applies. So I don’t see any particularly bad “deeper compiler issue” at fault here.

@tmandry
Copy link
Member

tmandry commented Aug 16, 2022

Ah, I see – thanks @steffahn! At first read it looked like these were totally unrelated, but I hadn't looked too closely at the code.

In that case I'm going to put this back through async triage and skip the t-compiler nomination for now. Maybe this is the same as another issue, like #90656 as you said.

@rustbot label -AsyncAwait-Triaged -I-compiler-nominated -I-prioritize

@rustbot rustbot removed AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. I-compiler-nominated Nominated for discussion during a compiler team meeting. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 16, 2022
@tmandry
Copy link
Member

tmandry commented Aug 29, 2022

These issues (this and #90656) seem pretty deep in the type / borrow checker and not all that specific to async. I think we should involve the types team for these. See the MCVE and surrounding conversation for more context.

@rustbot label +T-types +I-types-nominated +AsyncAwait-Triaged

(Not sure if nominating is the correct process here, please let me know if not)

@rustbot rustbot added AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. I-types-nominated Nominated for discussion during a types team meeting. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Aug 29, 2022
@nikomatsakis
Copy link
Contributor

We've had some spurious errors similar to this around async functions for a while -- I have to find the citations -- is it likely that this is a duplicate? Searching the repo I see a lot of related issues, I can't quite find the one I'm thinking of. It also feels related to the implied bounds on GATs, in that the "right fix" is similar.

@damooo
Copy link

damooo commented Sep 4, 2022

I got the same eror for a custom struct saying implementation of RepDerefLayer is not general enough` in an async block, with following note.

    = note: `RdfSourceConnegRepDerefLayer<(dyn RepDerefService<RepData = either::Either<Pin<Box<dyn Stream<Item = Result<bytes::Bytes, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>, Pin<Box<dyn Stream<Item = Result<Quad, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>>, Error = HttpApiProblem, Response = Representation<either::Either<Pin<Box<dyn Stream<Item = Result<bytes::Bytes, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>, Pin<Box<dyn Stream<Item = Result<Quad, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>>>> + '0)>` must implement `RepDerefLayer<(dyn RepDerefService<RepData = either::Either<Pin<Box<dyn Stream<Item = Result<bytes::Bytes, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>, Pin<Box<dyn Stream<Item = Result<Quad, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>>, Error = HttpApiProblem, Response = Representation<either::Either<Pin<Box<dyn Stream<Item = Result<bytes::Bytes, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>, Pin<Box<dyn Stream<Item = Result<Quad, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>>>> + '1)>`, for any two lifetimes `'0` and `'

but it actually implements `RepDerefLayer<dyn RepDerefService<RepData = either::Either<Pin<Box<dyn Stream<Item = Result<bytes::Bytes, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>, Pin<Box<dyn Stream<Item = Result<Quad, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>>, Error = HttpApiProblem, Response = Representation<either::Either<Pin<Box<dyn Stream<Item = Result<bytes::Bytes, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>, Pin<Box<dyn Stream<Item = Result<Quad, Box<dyn StdError + Sync + std::marker::Send>>> + std::marker::Send>>>>>>`

Essentially it says not general enough over lifetime param in a BoxError Box<dyn StdError + Sync + std::marker::Send>, where in original type definition, it as type alias with a +'static bound on dyn Error. Error happens at high level of code, and cannot reduce to minimal repro yet, neither any working workaround.

@damooo
Copy link

damooo commented Sep 4, 2022

For each type alias with a dyn object in it's definition explicitly marked with +'static lifetime, It raises a implementation not general enough , suggesting that it should be implemented for 'a bound. This happens when resultant future is required to be Send.

@jackh726
Copy link
Member

jackh726 commented Oct 7, 2022

We talked about this today in a types team meeting (https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2022-10-07.20Planning.20meeting/near/302848792).

We don't think we learn anything new from this issue (that isn't already a known limitation). We have ongoing work that should help this, but nothing immediate.

@jackh726 jackh726 added S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue P-medium Medium priority and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-types-nominated Nominated for discussion during a types team meeting. labels Oct 7, 2022
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 14, 2022
…imulacrum

Add a few known-bug tests

The labels of these tests should be changed from `S-bug-has-mcve` to `S-bug-has-test` once this is merged.

cc:
rust-lang#101518
rust-lang#99492
rust-lang#90950
rust-lang#89196
rust-lang#104034
rust-lang#101350
rust-lang#103705
rust-lang#103899

I couldn't reproduce the failures in rust-lang#101962 and rust-lang#100772 (so either these have started passing, or I didn't repro properly), so leaving those out for now.

rust-lang#102065 was a bit more complicated, since it uses `rustc_private` and I didn't want to mess with that.
@jackh726 jackh726 added S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. and removed S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue labels Nov 14, 2022
@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@alexkirsz
Copy link
Contributor

Are there any known workarounds for this issue? I'm running into this error message when using trait objects, similar to #99492 (comment). This occurred when adding an explicit Send bound to an impl Future return type while trying to debug a higher-ranked lifetime error,

@cbiffle
Copy link
Contributor

cbiffle commented Mar 12, 2024

We don't think we learn anything new from this issue (that isn't already a known limitation).

@jackh726 would you mind linking to that known limitation? I read the Zulip thread you listed, and it's not in there. We're still hitting this and trying to figure out how to work around it and/or if there's anything we can do to help.

@sunshowers
Copy link
Contributor

We don't think we learn anything new from this issue (that isn't already a known limitation).

@jackh726 would you mind linking to that known limitation? I read the Zulip thread you listed, and it's not in there. We're still hitting this and trying to figure out how to work around it and/or if there's anything we can do to help.

I believe #64552 is the parent issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. P-medium Medium priority S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests