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

Variance should perhaps take into account 'static bounds. #59875

Open
eddyb opened this issue Apr 11, 2019 · 8 comments
Open

Variance should perhaps take into account 'static bounds. #59875

eddyb opened this issue Apr 11, 2019 · 8 comments
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Apr 11, 2019

For example, when trying to build something resembling a pointer w/ optional metadata:

struct Ptr<T: ?Sized + Pointee> {
    addr: usize,
    meta: T::Meta,
}

trait Pointee {
    type Meta: 'static + Copy;
}
impl<T> Pointee for T {
    type Meta = ();
}
impl<T> Pointee for [T] {
    type Meta = usize;
}

Ptr<T> ends up invariant over T even though lifetimes in T cannot possibly affect T::Meta (which has a 'static bound), and that results in errors like these:

error[E0308]: mismatched types
  --> src/lib.rs:16:56
   |
16 | fn covariant<'a>(p: Ptr<&'static ()>) -> Ptr<&'a ()> { p }
   |                                                        ^ lifetime mismatch
   |
   = note: expected type `Ptr<&'a ()>`
              found type `Ptr<&'static ()>`
note: the lifetime 'a as defined on the function body at 16:14...
  --> src/lib.rs:16:14
   |
16 | fn covariant<'a>(p: Ptr<&'static ()>) -> Ptr<&'a ()> { p }
   |              ^^
   = note: ...does not necessarily outlive the static lifetime

cc @rust-lang/wg-traits

@jonas-schievink jonas-schievink added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Apr 11, 2019
@philipc
Copy link
Contributor

philipc commented Apr 11, 2019

Similar problem to #57440. Possible workaround from that:

struct Ptr<T: ?Sized + Pointee, U = <T as Pointee>::Meta> {
    addr: usize,
    meta: U,
    t: std::marker::PhantomData<T>,
}

@eddyb
Copy link
Member Author

eddyb commented Apr 11, 2019

@philipc Ahahahaha, that is clever, I love it! I'll try it and report back.

@eddyb
Copy link
Member Author

eddyb commented Apr 11, 2019

Yes, this compiles:

struct Ptr<T: ?Sized + Pointee<Meta = Meta>, Meta: 'static + Copy = <T as Pointee>::Meta> {
    addr: usize,
    meta: Meta,
    _marker: std::marker::PhantomData<T>,
}

trait Pointee {
    type Meta: 'static + Copy;
}
impl<T> Pointee for T {
    type Meta = ();
}
impl<T> Pointee for [T] {
    type Meta = usize;
}

fn covariant<'a>(p: Ptr<&'static ()>) -> Ptr<&'a ()> { p }

@eddyb
Copy link
Member Author

eddyb commented Apr 11, 2019

Annoyingly enough, wrapping Ptr in a struct is still invariant, i.e. this errors:

struct Newtype<'a>(Ptr<&'a ()>);

fn covariant_newtype<'a>(p: Newtype<'static>) -> Newtype<'a> { p }

I believe this is because variance computation doesn't normalize?

EDIT: this definition is covariant, OTOH (despite being equivalent):

struct Newtype<'a>(Ptr<&'a (), ()>);

@ohadravid
Copy link
Contributor

ohadravid commented May 24, 2019

I tried playing with something similar to fix another invariance problem I encountered, and I think there is another small problem with this workaround:
It makes custom implementations (outside the defining crate) less ergonomic

use othercrate::{InvariantPtr, Ptr}

struct MyType {}

// InvariantPtr is the original code.
// This complies with no problems.
impl PartialEq<MyType> for InvariantPtr<MyType> {
    fn eq(&self, other: &MyType) -> bool {
        unimplemented!()
    }
}

// Ptr is the code with the workaround.
// This fails with:
// error[E0210]: type parameter `<MyType as othercrate::Pointee>::Meta` must be used as the type 
// parameter for some local type (e.g., `MyStruct<<MyType as othercrate::Pointee>::Meta>`)
// ... ::Pointee>::Meta` must be used as the type parameter for some local type
//  
//  = note: only traits defined in the current crate can be implemented for a type parameter
//
impl PartialEq<MyType> for Ptr<MyType> {
    fn eq(&self, other: &MyType) -> bool {
        unimplemented!()
    }
}

Users can still implement this by specifying the actual Meta type (and the compiler will force Meta to be the correct type), but this is strange (and for my use case it's also a breaking change, since existing implementation don't expect the additional param):

impl PartialEq<MyType> for Ptr<MyType, ()> {
    fn eq(&self, other: &MyType) -> bool {
        unimplemented!()
    }
}

(I don't know if that's relevant, but seems worth pointing out since that's another argument for allowing the variance in the issue. Also if you think there's another workaround I'm really interested to know!)

@steffahn
Copy link
Member

steffahn commented Aug 27, 2021

Ptr<T> ends up invariant over T even though lifetimes in T cannot possibly affect T::Meta (which has a 'static bound)

This logic is flawed. Subtyping isn’t only about lifetimes, e.g. HRTB-like fn-pointer (or alternatively trait objects) types come into play, too.

trait Pointee {
    type Meta: 'static + Copy;
}

struct SomethingStatic<T: 'static>(T);
impl<T> Pointee for SomethingStatic<T> {
    type Meta = fn(T);
}

struct CovariantPtr<T>(T);

fn demonstrate_subtyping(
    x: CovariantPtr<SomethingStatic<for<'a> fn(&'a str)>>,
) -> CovariantPtr<SomethingStatic<fn(&'static str)>> {
    x
}

If Ptr<T> was covariant, you could now turn Ptr<SomethingStatic<for<'a> fn(&'a str)> into Ptr<SomethingStatic<fn(&'static str)>> in effect converting fn(for<'a> fn(&'a str)) into fn(fn(&'static str)), which is unsound.

@eddyb
Copy link
Member Author

eddyb commented Aug 27, 2021

Heh, I guess that was back when we still had hope to remove or weaken that subtyping relationship (cc @nikomatsakis).

If we have to keep it, I agree there's no fix here (short of variance annotations on traits).

@Avi-D-coder
Copy link
Contributor

In my experiments, GATs are not very useful without variance annotations.
Other people have also been bumping into this problem.
https://internals.rust-lang.org/t/variance-of-lifetime-arguments-in-gats/14769/34

I'm not sure of a good design, but some sort of variance annotations should be seriously considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants