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

nested impl Trait in trait with default definition does not compile #117104

Closed
axos88 opened this issue Oct 23, 2023 · 4 comments · Fixed by #117131
Closed

nested impl Trait in trait with default definition does not compile #117104

axos88 opened this issue Oct 23, 2023 · 4 comments · Fixed by #117131
Assignees
Labels
C-bug Category: This is a bug. F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@axos88
Copy link

axos88 commented Oct 23, 2023

I tried this code:

trait Foo {
    fn foo() -> impl Stream<Item = impl Stream<Item = impl Future<Output = ()>>> {
        if true {
            unimplemented!()
        } else {
            let f = futures::future::ready(());
            let s1 = futures::stream::once(async { f });
            futures::stream::once(async { s1 })
        }
    }
}

I expected to see this happen: compiles, or a helpful error message

Instead, this happened: Confusing error message, there doesn't seem to be a future that could be awaited?...
Interestingly if the default implementation is removed, it compiles. Playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=6d9d947523afbecf604d1971f58bbc48

error[E0271]: type mismatch resolving `<impl Stream<Item = impl Future<Output = ()>> as Stream>::Item == impl Future<Output = ()>`
  --> /home/akos/projects/rust/tarpc/tarpc/src/server/incoming.rs:49:36
   |
49 |     fn foo() -> impl Stream<Item = impl Stream<Item = impl Future<Output = ()>>> {
   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected associated type, found future
   |
   = help: consider `await`ing on both `Future`s
note: required by a bound in `Foo::{opaque#0}`
  --> /home/akos/projects/rust/tarpc/tarpc/src/server/incoming.rs:49:48
   |
49 |     fn foo() -> impl Stream<Item = impl Stream<Item = impl Future<Output = ()>>> {
   |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Foo::{opaque#0}`

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

Meta

rustc --version --verbose:

rustc 1.75.0-nightly (1c05d50c8 2023-10-21)
binary: rustc
commit-hash: 1c05d50c8403c56d9a8b6fb871f15aaa26fb5d07
commit-date: 2023-10-21
host: x86_64-unknown-linux-gnu
release: 1.75.0-nightly
LLVM version: 17.0.3

Backtrace

N/A

@axos88 axos88 added the C-bug Category: This is a bug. label Oct 23, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 23, 2023
@axos88
Copy link
Author

axos88 commented Oct 23, 2023

The error message on the playground is a bit different, seems to be correlating with the actual problem more, but even more confusing:

   Compiling playground v0.0.1 (/playground)
error[E0271]: type mismatch resolving `<impl Stream<Item = impl Future<Output = ()>> as Stream>::Item == impl Future<Output = ()>`
  --> src/lib.rs:27:36
   |
27 |     fn foo() -> impl Stream<Item = impl Stream<Item = impl Future<Output = ()>>> {
   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected associated type, found opaque type
   |
   = note: expected associated type `impl Future<Output = ()>` (trait associated opaque type at <src/lib.rs:27:55>)
                  found opaque type `impl Future<Output = ()>` (opaque type at <src/lib.rs:27:55>)
note: required by a bound in `Foo::{opaque#2}`
  --> src/lib.rs:27:48
   |
27 |     fn foo() -> impl Stream<Item = impl Stream<Item = impl Future<Output = ()>>> {
   |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Foo::{opaque#2}`

For more information about this error, try `rustc --explain E0271`.
error: could not compile `playground` (lib) due to previous error

Expected and found are the same!

@compiler-errors compiler-errors self-assigned this Oct 24, 2023
@compiler-errors
Copy link
Member

Minimal:

use std::ops::Deref;

trait Foo {
    fn foo() -> impl Deref<Target = impl Deref<Target = impl Sized>> {
        &&()
    }
}

fn main() {}

Something funky going on with the default projection predicate installed for the GAT. I'll look into it 🤔

@aliemjay aliemjay added T-types Relevant to the types team, which will review and decide on the PR/issue. F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` labels Oct 24, 2023
@axos88
Copy link
Author

axos88 commented Oct 24, 2023

I'm impressed by the speedy handling of this issue! Was hoping this would get a fix in a few weeks . You're awesome!

@axos88
Copy link
Author

axos88 commented Oct 24, 2023

Once the pr is merged, it will be available the next day on nightly, correct? Btw, the pr failed some tests.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 29, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 2, 2023
Add all RPITITs when augmenting param-env with GAT bounds in `check_type_bounds`

When checking that associated type definitions actually satisfy their associated type bounds in `check_type_bounds`, we construct a "`normalize_param_env`" which adds a projection predicate that allows us to assume that we can project the GAT to the definition we're checking. For example, in:

```rust
type Foo {
  type Bar: Display = i32;
}
```

We would add `<Self as Foo>::Bar = i32` as a projection predicate when checking that `i32: Display` holds.

That `normalize_param_env` was, for some reason, only being used to normalize the predicate before it was registered. This is sketchy, because a nested obligation may require the GAT bound to hold, and also the projection cache is broken and doesn't differentiate projection cache keys that differ by param-envs 😿.

This `normalize_param_env` is also not sufficient when we have nested RPITITs and default trait methods, since we need to be able to assume we can normalize both the RPITIT and all of its child RPITITs to sufficiently prove all of its bounds. This is the cause of rust-lang#117104, which only starts to fail for RPITITs that are nested 3 and above due to the projection-cache bug above.[^1]

## First fix

Use the `normalize_param_env` everywhere in `check_type_bounds`. This is reflected in a test I've constructed that fixes a GAT-only failure.

## Second fix

For RPITITs, install projection predicates for each RPITIT in the same function in `check_type_bounds`. This fixes rust-lang#117104.

not sure who to request, so...
r? `@lcnr` hehe feel free to reassign :3

[^1]: The projection cache bug specifically occurs because we try normalizing the `assumed_wf_types` with the non-normalization param-env. This causes us to insert a projection cache entry that keeps the outermost RPITIT rigid, and it trivially satisifes all its own bounds. Super sketchy![^2]

[^2]: I haven't actually gone and fixed the projection cache bug because it's only marginally related, but I could, and it should no longer be triggered here.
@bors bors closed this as completed in 2520ca8 Nov 3, 2023
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-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants