-
Notifications
You must be signed in to change notification settings - Fork 189
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
The deduped versions of the *View
types break variance:
#501
Comments
I just tested, it's not related to pub(crate) trait SealedStorage<T> {
type Buffer: ?Sized + Borrow<[T]> + BorrowMut<[T]>;
} |
😱 |
see discussion in https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/variance.20of.20GATs.20when.20the.20type.20is.20known I think this is fundamentally unfixable 😭 |
What do you think about this approach: mod storage {
use std::mem::MaybeUninit;
pub trait VecStorage<T>: VecSealedStorage<T> {}
pub trait VecSealedStorage<T> {
// part of the sealed trait so that no trait is publicly implemented by `OwnedVecStorage` besides `Storage`
fn borrow(&self) -> &[MaybeUninit<T>];
fn borrow_mut(&mut self) -> &mut [MaybeUninit<T>];
}
// One sealed layer of indirection to hide the internal details (The MaybeUninit).
pub struct VecStorageInner<T: ?Sized> {
buffer: T,
}
// These are the public types
pub type OwnedVecStorage<T, const N: usize> = VecStorageInner<[MaybeUninit<T>; N]>;
pub type ViewVecStorage<T> = VecStorageInner<[MaybeUninit<T>]>;
impl<T, const N: usize> VecSealedStorage<T> for OwnedVecStorage<T, N> {
fn borrow(&self) -> &[MaybeUninit<T>] {
&self.buffer
}
fn borrow_mut(&mut self) -> &mut [MaybeUninit<T>] {
&mut self.buffer
}
}
impl<T, const N: usize> VecStorage<T> for OwnedVecStorage<T, N> {}
impl<T> VecSealedStorage<T> for ViewVecStorage<T> {
fn borrow(&self) -> &[MaybeUninit<T>] {
&self.buffer
}
fn borrow_mut(&mut self) -> &mut [MaybeUninit<T>] {
&mut self.buffer
}
}
impl<T> VecStorage<T> for ViewVecStorage<T> {}
}
use std::marker::PhantomData;
pub use storage::{OwnedVecStorage, VecStorage, ViewVecStorage};
pub struct VecInner<T, S: VecStorage<T> + ?Sized> {
// This phantomdata is required because otherwise rustc thinks that `T` is not used
phantom: PhantomData<T>,
len: usize,
buffer: S,
}
pub type Vec<T, const N: usize> = VecInner<T, OwnedVecStorage<T, N>>;
pub type VecView<T> = VecInner<T, ViewVecStorage<T>>;
fn test_variance<'a: 'b, 'b>(x: Vec<&'a (), 42>) -> Vec<&'b (), 42> {
x
}
fn test_variance_view<'a: 'b, 'b, 'c>(x: &'c VecView<&'a ()>) -> &'c VecView<&'b ()> {
x
} It keeps the variance, and it allows us to keep the internal details of The only downside over the current approach is that we need a separate mechanism for each container. I don't think that can be worked around. |
Overall I also want to say I don't think the Honestly here I don't really understand why the variance is broken when using the current approach, even after reading the zulip thread. |
oh yes! that could work. It's unfortunate that it requires duplicating the Storage structs/traits for each container. Perhaps with a macro to generate them it wouldn't be that bad? An alternative would be
that's awesome to hear 🤩
Yes, I agree, unfortunately 🥲
rustc calculates variance for the struct "eagerly", by looking just at the raw struct definition, before substituting any generic params. It takes just pub struct VecInner<T, S: Storage> {
len: usize,
buffer: S::Buffer<MaybeUninit<T>>,
} then it deduces that
then it just uses these deductions everywhere. It doesn't take into account substitutions. |
Yes, I understand why This is the kind of thing that feels like an arbitrary limitation of the compiler rather than something that makes sense in the language itself. Though I understand I wouldn't want to be the one having to implement this in the compiler or even review a PR that does it... |
the compiler does treat |
I meant treating the type alias |
The following compiles with
v0.8.0
and #425, but not with latest main:@Dirbaio
I think this is caused the intermediary
Generic Associated Type
, but I could not find where this is documented.The text was updated successfully, but these errors were encountered: