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

Higher ranked lifetime error when tryng to see that a future is send. #114046

Open
Tracked by #110338
antialize opened this issue Jul 25, 2023 · 1 comment
Open
Tracked by #110338
Labels
A-async-await Area: Async & Await A-higher-ranked Area: Higher-ranked things (e.g., lifetimes, types, trait bounds aka HRTBs) AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. 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

@antialize
Copy link

antialize commented Jul 25, 2023

I tried this code:

use std::marker::PhantomData;

trait Callable<'a>: Send + Sync {
    fn callable(data: &'a [u8]);
}

trait Getter<'a>: Send + Sync {
    type ItemSize: Send + Sync;

    fn get(data: &'a [u8]);
}

struct List<'a, A: Getter<'a>> {
    data: &'a [u8],
    item_size: A::ItemSize, // Removing this member causes the code to compile
    phantom: PhantomData<A>,
}

struct GetterImpl<'a, T: Callable<'a> + 'a> {
    p: PhantomData<&'a T>,
}

impl<'a, T: Callable<'a> + 'a> Getter<'a> for GetterImpl<'a, T> {
    type ItemSize = ();

    fn get(data: &'a [u8]) {
        <T>::callable(data);
    }
}

struct ConstructableImpl<'a> {
    _data: &'a [u8],
}

impl<'a> Callable<'a> for ConstructableImpl<'a> {
    fn callable(_: &'a [u8]) {}
}

struct StructWithLifetime<'a> {
    marker: &'a PhantomData<u8>,
}

async fn async_method() {}

fn assert_send(_: impl Send + Sync) {}

// This async method ought to be send, but is not
async fn my_send_async_method(_struct_with_lifetime: &mut StructWithLifetime<'_>, data: &Vec<u8>) {
    let _named = List::<'_, GetterImpl<ConstructableImpl<'_>>> {
        data,
        item_size: (),
        phantom: PhantomData,
    };
    assert_send(_named);
    // Moving the await point above the constructed of _named, causes
    // the method to become send, even though _named is Send + Sync
    async_method().await;
}

fn dummy(struct_with_lifetime: &mut StructWithLifetime<'_>, data: &Vec<u8>) {
    assert_send(my_send_async_method(struct_with_lifetime, data));
}

fn main() {}

I expect this code to compile successfully. However the compiler fails to infer that my_send_async_method is SEND, even though it is.

I get the following error message:

$ rustc --version
rustc 1.71.0 (8ede3aae2 2023-07-12)

$ RUST_BACKTRACE=1 cargo build
   Compiling minimal v0.1.0 (/home/jakobt/dev/minimal)
error: implementation of `Getter` is not general enough
  --> src/main.rs:63:5
   |
63 |     assert_send(my_send_async_method(struct_with_lifetime, data));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Getter` is not general enough
   |
   = note: `Getter<'1>` would have to be implemented for the type `GetterImpl<'0, ConstructableImpl<'_>>`, for any two lifetimes `'0` and `'1`...
   = note: ...but `Getter<'2>` is actually implemented for the type `GetterImpl<'2, ConstructableImpl<'_>>`, for some specific lifetime `'2`

error: implementation of `Callable` is not general enough
  --> src/main.rs:63:5
   |
63 |     assert_send(my_send_async_method(struct_with_lifetime, data));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Callable` is not general enough
   |
   = note: `Callable<'_>` would have to be implemented for the type `ConstructableImpl<'0>`, for any lifetime `'0`...
   = note: ...but `Callable<'1>` is actually implemented for the type `ConstructableImpl<'1>`, for some specific lifetime `'1`

error: higher-ranked lifetime error
  --> src/main.rs:63:5
   |
63 |     assert_send(my_send_async_method(struct_with_lifetime, data));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: could not prove `impl Future<Output = ()>: Sync`

error: could not compile `minimal` (bin "minimal") due to 3 previous errors

This does not make sense, GetterImpl (and ConstructableImpl) cannot possible be constructed with two different lifetimes since that goes against the definition.

If we remove the call to async_method().await The assert_send(_named) show that _named is indeed Send and Sync, so the method should also me.

I have minimized this example as much as I can, removing any of the current parts resolves the issue. Removing the lifetime on StructWithLifetime, does not resolve the issue but changes the error message to

error: higher-ranked lifetime error
  --> src/main.rs:61:5
   |
61 |     assert_send(my_send_async_method(struct_with_lifetime, data));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: could not prove `impl Future<Output = ()>: Send`

error: higher-ranked lifetime error
  --> src/main.rs:61:5
   |
61 |     assert_send(my_send_async_method(struct_with_lifetime, data));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: could not prove `impl Future<Output = ()>: Sync`
@antialize antialize added the C-bug Category: This is a bug. label Jul 25, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 25, 2023
@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-async-await Area: Async & Await T-types Relevant to the types team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 4, 2023
@eholk
Copy link
Contributor

eholk commented Sep 11, 2023

This seems like another instance of the issues we're tracking at #110338.

@eholk eholk added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Sep 11, 2023
drmingdrmer added a commit to drmingdrmer/rust-trait-lifetime-how-to that referenced this issue Sep 27, 2023
```rust
error: implementation of `MapApiRO` is not general enough
   --> src/data/impl_static_levels.rs:138:22
    |
138 |             let fu = assert_sync(fu);
    |                      ^^^^^^^^^^^^^^^ implementation of `MapApiRO` is not general enough
    |
    = note: `MapApiRO<String>` would have to be implemented for the type `&'0 Level`, for any lifetime `'0`...
    = note: ...but `MapApiRO<String>` is actually implemented for the type `&'1 Level`, for some specific lifetime `'1`
```

rust-lang/rust#100013
rust-lang/rust#114046
rust-lang/rust#110338
drmingdrmer added a commit to drmingdrmer/databend that referenced this issue Sep 27, 2023
drmingdrmer added a commit to drmingdrmer/databend that referenced this issue Sep 27, 2023
drmingdrmer added a commit to drmingdrmer/databend that referenced this issue Sep 29, 2023
- Add trait `MapKey` and `MapValue` to define behavior of key values
  that are used in `MapApi`.

- Add Ref and RefMut as container of reference to leveled data.

- Collect get() and range() implementation into function.

The MapApi trait can not be generalized to adapt arbitrary lifetime,
such as using `self` instead of `&self`: `MapApiRO { fn get(self, key) }`.
Because there is a known limitation with rust GAT and hihger ranked
lifetime: See: rust-lang/rust#114046

```
error: implementation of `MapApiRO` is not general enough
  --> src/meta/raft-store/src/sm_v002/sm_v002.rs:80:74
   |
80 |       async fn get_kv(&self, key: &str) -> Result<GetKVReply, Self::Error> {
   |  __________________________________________________________________________^
81 | |         let got = self.sm.get_kv(key).await;
82 | |
83 | |         let local_now_ms = SeqV::<()>::now_ms();
84 | |         let got = Self::non_expired(got, local_now_ms);
85 | |         Ok(got)
86 | |     }
   | |_____^ implementation of `MapApiRO` is not general enough
   |
   = note: `MapApiRO<'1, std::string::String>` would have to be implemented for the type `&'0 LevelData`, for any two lifetimes `'0` and `'1`...
   = note: ...but `MapApiRO<'2, std::string::String>` is actually implemented for the type `&'2 LevelData`, for some specific lifetime `'2`
```
BohuTANG pushed a commit to databendlabs/databend that referenced this issue Sep 29, 2023
* refactor: simplify MapApi

- Add trait `MapKey` and `MapValue` to define behavior of key values
  that are used in `MapApi`.

- Add Ref and RefMut as container of reference to leveled data.

- Collect get() and range() implementation into function.

The MapApi trait can not be generalized to adapt arbitrary lifetime,
such as using `self` instead of `&self`: `MapApiRO { fn get(self, key) }`.
Because there is a known limitation with rust GAT and hihger ranked
lifetime: See: rust-lang/rust#114046

```
error: implementation of `MapApiRO` is not general enough
  --> src/meta/raft-store/src/sm_v002/sm_v002.rs:80:74
   |
80 |       async fn get_kv(&self, key: &str) -> Result<GetKVReply, Self::Error> {
   |  __________________________________________________________________________^
81 | |         let got = self.sm.get_kv(key).await;
82 | |
83 | |         let local_now_ms = SeqV::<()>::now_ms();
84 | |         let got = Self::non_expired(got, local_now_ms);
85 | |         Ok(got)
86 | |     }
   | |_____^ implementation of `MapApiRO` is not general enough
   |
   = note: `MapApiRO<'1, std::string::String>` would have to be implemented for the type `&'0 LevelData`, for any two lifetimes `'0` and `'1`...
   = note: ...but `MapApiRO<'2, std::string::String>` is actually implemented for the type `&'2 LevelData`, for some specific lifetime `'2`
```

* refactor: move Ref and RefMut to separate files

* refactor: rename is_not_found() to not_found()

As @ariesdevil kindly suggested.

* refactor: simplify map api names: LevelData to Level

* chore: fix clippy
andylokandy pushed a commit to andylokandy/databend that referenced this issue Nov 27, 2023
* refactor: simplify MapApi

- Add trait `MapKey` and `MapValue` to define behavior of key values
  that are used in `MapApi`.

- Add Ref and RefMut as container of reference to leveled data.

- Collect get() and range() implementation into function.

The MapApi trait can not be generalized to adapt arbitrary lifetime,
such as using `self` instead of `&self`: `MapApiRO { fn get(self, key) }`.
Because there is a known limitation with rust GAT and hihger ranked
lifetime: See: rust-lang/rust#114046

```
error: implementation of `MapApiRO` is not general enough
  --> src/meta/raft-store/src/sm_v002/sm_v002.rs:80:74
   |
80 |       async fn get_kv(&self, key: &str) -> Result<GetKVReply, Self::Error> {
   |  __________________________________________________________________________^
81 | |         let got = self.sm.get_kv(key).await;
82 | |
83 | |         let local_now_ms = SeqV::<()>::now_ms();
84 | |         let got = Self::non_expired(got, local_now_ms);
85 | |         Ok(got)
86 | |     }
   | |_____^ implementation of `MapApiRO` is not general enough
   |
   = note: `MapApiRO<'1, std::string::String>` would have to be implemented for the type `&'0 LevelData`, for any two lifetimes `'0` and `'1`...
   = note: ...but `MapApiRO<'2, std::string::String>` is actually implemented for the type `&'2 LevelData`, for some specific lifetime `'2`
```

* refactor: move Ref and RefMut to separate files

* refactor: rename is_not_found() to not_found()

As @ariesdevil kindly suggested.

* refactor: simplify map api names: LevelData to Level

* chore: fix clippy
@fmease fmease added the A-higher-ranked Area: Higher-ranked things (e.g., lifetimes, types, trait bounds aka HRTBs) label Sep 24, 2024
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-higher-ranked Area: Higher-ranked things (e.g., lifetimes, types, trait bounds aka HRTBs) AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. 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

5 participants