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

Incorrect lifetime bound check in async + impl_trait_in_assoc_type #114572

Closed
xiaoyawei opened this issue Aug 7, 2023 · 4 comments · Fixed by #121679
Closed

Incorrect lifetime bound check in async + impl_trait_in_assoc_type #114572

xiaoyawei opened this issue Aug 7, 2023 · 4 comments · Fixed by #121679
Assignees
Labels
C-bug Category: This is a bug. F-impl_trait_in_assoc_type `#![feature(impl_trait_in_assoc_type)]` F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` WG-async Working group: Async & await

Comments

@xiaoyawei
Copy link

xiaoyawei commented Aug 7, 2023

I tried this code:

#![feature(type_alias_impl_trait)]
#![feature(impl_trait_in_assoc_type)]

use std::{fmt::Display, future::Future};

pub trait SharedDisplay: Display + Send + 'static {}

pub struct Message<T: AsRef<dyn SharedDisplay>> {
    inner: T,
}

pub trait AsyncPrinter: Send + Sync {
    type AsyncOutput<'a>: Future<Output = String> + Send
    where
        Self: 'a;

    fn to_string(&self, msg: Message<Box<dyn SharedDisplay>>) -> Self::AsyncOutput<'_>;
}

struct FooPrinter;

impl AsyncPrinter for FooPrinter {
    type AsyncOutput<'a> = impl Future<Output = String> + Send + 'a where Self: 'a;

    fn to_string(&self, msg: Message<Box<dyn SharedDisplay>>) -> Self::AsyncOutput<'_> {
        async move { msg.inner.to_string() }
    }
}

I expected to see this happen: The code correctly compiles

Instead, this happened: The code fails to compile with lifetime bound check errors.

More information that might be helpful

I did a bisect and found that the above code compiles in nightly-2023-03-14 but fails in nightly-2023-03-15, possibly due to #108909 but I didn't dig deep to find the root cause.

Also, by changing the implementation of the to_string function of FooPrinter by wrapping the async block with a smart pointer (see below), the code successfully compiles

fn to_string(&self, msg: Message<Box<dyn SharedDisplay>>) -> Self::AsyncOutput<'_> {
    Box::pin(async move { msg.inner.to_string() })
}

Let me know if more info is needed, or actually my codes fails to compile by design.

Meta

rustc --version --verbose:

rustc 1.73.0-nightly (474709a9a 2023-08-03)
binary: rustc
commit-hash: 474709a9a2a74a8bcf0055fadb335d0ca0d2d939
commit-date: 2023-08-03
host: x86_64-apple-darwin
release: 1.73.0-nightly
LLVM version: 16.0.5
Backtrace

error[E0478]: lifetime bound not satisfied
  --> src/lib.rs:23:28
   |
23 |     type AsyncOutput<'a> = impl Future<Output = String> + Send + 'a where Self: 'a;
   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: lifetime parameter instantiated with the lifetime `'a` as defined here
  --> src/lib.rs:23:22
   |
23 |     type AsyncOutput<'a> = impl Future<Output = String> + Send + 'a where Self: 'a;
   |                      ^^
   = note: but lifetime parameter must outlive the static lifetime

@xiaoyawei xiaoyawei added the C-bug Category: This is a bug. label Aug 7, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 7, 2023
@compiler-errors
Copy link
Member

This is impl_trait_in_assoc_type, not RPITIT. This doesn't have to do with #108909 -- I'll do a full bisection later.

@compiler-errors compiler-errors changed the title Incorrect lifetime bound check in RPITIT Incorrect lifetime bound check in async + impl_trait_in_assoc_type Aug 8, 2023
@compiler-errors
Copy link
Member

searched nightlies: from nightly-2023-02-01 to nightly-2023-04-01
regressed nightly: nightly-2023-03-15
searched commit range: 22f247c...1716932
regressed commit: 669e751

bisected with cargo-bisect-rustc v0.6.5

Host triple: aarch64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2023-02-01 --end=2023-04-01 --prompt 

This regressed in #104833. Strange, but perhaps a case that #105300 didn't end up fixing? cc @aliemjay

@compiler-errors compiler-errors added A-async-await Area: Async & Await F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` WG-async Working group: Async & await and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 8, 2023
@lqd
Copy link
Member

lqd commented Aug 8, 2023

Note for bisection, impl_trait_in_assoc_type was added a month after the expected nightly-2023-03-15 and points to #104833 locally.

(dang, too slow)

@aliemjay aliemjay self-assigned this Aug 8, 2023
@aliemjay
Copy link
Member

aliemjay commented Aug 9, 2023

Minimized:

#![feature(type_alias_impl_trait)]
struct Static<T: 'static>(T);
type Output<'a> = impl Sized + 'a;
fn relate<'a>(msg: Static<&'static u8>) -> Output<'a> {
    msg
}

The hidden type is inferred to be Output<'a> = Static<&'a str>, but that's not a well-formed type. This is not a problem for RPIT only because we skip the well-formedness check of the hidden type.

@eholk eholk removed the A-async-await Area: Async & Await label Sep 11, 2023
@workingjubilee workingjubilee added the F-impl_trait_in_assoc_type `#![feature(impl_trait_in_assoc_type)]` label Nov 8, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 29, 2024
stricter hidden type wf-check [based on rust-lang#115008]

Original work by `@aliemjay` in rust-lang#115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang#114728
Fixes rust-lang#114572

Instead of adding the `WellFormed` obligations when relating opaque types, I add always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
@bors bors closed this as completed in 09bc67b Mar 6, 2024
bors added a commit to rust-lang/miri that referenced this issue Mar 6, 2024
stricter hidden type wf-check [based on #115008]

Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang/rust#114728
Fixes rust-lang/rust#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
stricter hidden type wf-check [based on #115008]

Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang/rust#114728
Fixes rust-lang/rust#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
stricter hidden type wf-check [based on #115008]

Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang/rust#114728
Fixes rust-lang/rust#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`
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-impl_trait_in_assoc_type `#![feature(impl_trait_in_assoc_type)]` F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` WG-async Working group: Async & await
Projects
None yet
7 participants