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

Type alias impl trait doesn’t properly enforce outlives relations. #84657

Closed
steffahn opened this issue Apr 28, 2021 · 2 comments · Fixed by #95519
Closed

Type alias impl trait doesn’t properly enforce outlives relations. #84657

steffahn opened this issue Apr 28, 2021 · 2 comments · Fixed by #95519
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-lifetimes Area: Lifetimes / regions A-traits Area: Trait system A-typesystem Area: The type system C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@steffahn
Copy link
Member

#![feature(min_type_alias_impl_trait)]

use std::marker::PhantomData;

trait ProofForConversion<'a, 'b> {
    fn convert<T: ?Sized>(_: PhantomData<Self>, r: &'a T) -> &'b T;
}

impl<'a, 'b> ProofForConversion<'a, 'b> for &'b &'a () {
    fn convert<T: ?Sized>(_: PhantomData<Self>, r: &'a T) -> &'b T {
        r
    }
}

type Converter<'a, 'b> = impl ProofForConversion<'a, 'b>;

// Even _defining_use with an explicit `'a: 'b` compiles fine, too.
fn _defining_use<'a, 'b>(x: &'b &'a ()) -> Converter<'a, 'b> {
    x
}

fn extend_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
    Converter::<'a, 'b>::convert(PhantomData, x)
}

fn main() {
    let d;
    {
        let x = "Hello World".to_string();
        d = extend_lifetime(&x);
    }
    println!("{}", d);
}
��C

(playground)

I would’ve expected rustc to complain about the _defining_use not matching the type Converter<'a, 'b> with a suggestion to turn it into type Converter<'a: 'b, 'a>.

@rustbot modify labels: A-typesystem, A-lifetimes, A-traits, A-impl-trait, F-type_alias_impl_trait, T-compiler, requires-nightly
and someone please add “I-unsound 💥” (and/or tell me if there is a way to do it myself).

@steffahn steffahn added the C-bug Category: This is a bug. label Apr 28, 2021
@rustbot rustbot added A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-lifetimes Area: Lifetimes / regions A-traits Area: Trait system A-typesystem Area: The type system F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 28, 2021
@jonas-schievink jonas-schievink added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Apr 28, 2021
@jackh726 jackh726 added F-min_type_alias_impl_trait and removed F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` labels Jun 29, 2021
@oli-obk oli-obk added F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` and removed F-min_type_alias_impl_trait labels Mar 30, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Mar 31, 2022

One thing that is of note here is that we properly enforce type bounds, but not lifetime bounds, as the equivalent program with types fails to compile right where we expect that:

#![feature(type_alias_impl_trait)]

use std::marker::PhantomData;

trait Trait {
    fn foo<T, U>(t: T) -> U;
}

trait ProofForConversion<X> {
    fn convert<T, U>(_: PhantomData<Self>, r: T) -> U;
}

impl<X: Trait> ProofForConversion<X> for () {
    fn convert<T, U>(_: PhantomData<Self>, r: T) -> U {
        X::foo(r)
    }
}

type Converter<T> = impl ProofForConversion<T>;

fn _defining_use<T: Trait>() -> Converter<T> {
    ()
}

@oli-obk
Copy link
Contributor

oli-obk commented Mar 31, 2022

Aha! If we change

impl<'a, 'b> ProofForConversion<'a, 'b> for &'b &'a () {

to have an explicit 'a: 'b bound, then we get the right error:

error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements
  --> $DIR/outlives_bounds2.rs:15:26
   |
LL | type Converter<'a, 'b> = impl ProofForConversion<'a, 'b>;
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: first, the lifetime cannot outlive the lifetime `'a` as defined here...
  --> $DIR/outlives_bounds2.rs:15:16
   |
LL | type Converter<'a, 'b> = impl ProofForConversion<'a, 'b>;
   |                ^^
note: ...but the lifetime must also be valid for the lifetime `'b` as defined here...
  --> $DIR/outlives_bounds2.rs:15:20
   |
LL | type Converter<'a, 'b> = impl ProofForConversion<'a, 'b>;
   |                    ^^
note: ...so that the types are compatible
  --> $DIR/outlives_bounds2.rs:15:26
   |
LL | type Converter<'a, 'b> = impl ProofForConversion<'a, 'b>;
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: expected `ProofForConversion<'a, 'b>`
              found `ProofForConversion<'_, '_>`

error: aborting due to previous error

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

so we're just missing a check of the implied bounds on the impl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-lifetimes Area: Lifetimes / regions A-traits Area: Trait system A-typesystem Area: The type system C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Development

Successfully merging a pull request may close this issue.

5 participants