SmallVec<[T; N]> is invariant over T #146
Comments
|
For reference, this is similar to bluss/arrayvec#96, and is caused by rust-lang/rust#21726. |
|
The following trick (can't remember where I found it): enum SmallVec<A: Array<Item=Item>, Item=<A as Array>::Item> {
Inline(MaybeUninit<A>),
Heap((NonNull<Item>, usize)),
}fixes the variance issue (playground link). |
We want the lifetimes of the patterns contained in the matrix and the candidate `PatStack` to be the same so that they can be mixed together. A lot of this would not be necessary if `SmallVec` was covariant in its type argument (see servo/rust-smallvec#146).
|
I just spent a few hours debugging a lifetime compilation error related to this after replacing |
|
Fortunately, I expect this issue will be fixed in smallvec 2.0 when we replace the That work depends on the |
|
Having stumbled upon a lifetime problem caused by this, I found the very new |
|
|
|
Oh wait, it looks like |
## Summary When you try to remove an internal representation leaking into another type and end up rewriting a simple version of `smallvec`. The goal of this PR is to replace the `Box<[&'a str]>` with `Box<QualifiedName>` to avoid that the internal `QualifiedName` representation leaks (and it gives us a nicer API too). However, doing this when `QualifiedName` uses `SmallVec` internally gives us all sort of funny lifetime errors. I was lost but @BurntSushi came to rescue me. He figured out that `smallvec` has a variance problem which is already tracked in servo/rust-smallvec#146 To fix the variants problem, I could use the smallvec-2-alpha-4 or implement our own smallvec. I went with implementing our own small vec for this specific problem. It obviously isn't as sophisticated as smallvec (only uses safe code), e.g. it doesn't perform any size optimizations, but it does its job. Other changes: * Removed `Imported::qualified_name` (the version that returns a `String`). This can be replaced by calling `ToString` on the qualified name. * Renamed `Imported::call_path` to `qualified_name` and changed its return type to `&QualifiedName`. * Renamed `QualifiedName::imported` to `user_defined` which is the more common term when talking about builtins vs the rest/user defined functions. ## Test plan `cargo test`
) ## Summary When you try to remove an internal representation leaking into another type and end up rewriting a simple version of `smallvec`. The goal of this PR is to replace the `Box<[&'a str]>` with `Box<QualifiedName>` to avoid that the internal `QualifiedName` representation leaks (and it gives us a nicer API too). However, doing this when `QualifiedName` uses `SmallVec` internally gives us all sort of funny lifetime errors. I was lost but @BurntSushi came to rescue me. He figured out that `smallvec` has a variance problem which is already tracked in servo/rust-smallvec#146 To fix the variants problem, I could use the smallvec-2-alpha-4 or implement our own smallvec. I went with implementing our own small vec for this specific problem. It obviously isn't as sophisticated as smallvec (only uses safe code), e.g. it doesn't perform any size optimizations, but it does its job. Other changes: * Removed `Imported::qualified_name` (the version that returns a `String`). This can be replaced by calling `ToString` on the qualified name. * Renamed `Imported::call_path` to `qualified_name` and changed its return type to `&QualifiedName`. * Renamed `QualifiedName::imported` to `user_defined` which is the more common term when talking about builtins vs the rest/user defined functions. ## Test plan `cargo test`
SmallVec<[T; N]>is invariant overT.Although
[T; N]is covariant overTas expected.This is due to
SmallVec<A>having field of type<A as Array>::Itemin underlying union.I propose to change that field type to
Aand removeA: Arraybound from type declaration.The text was updated successfully, but these errors were encountered: