You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I opened a PR that adds support for constant generics in rust-smallvec, can I do same with arrayvec? It is basically a macro controlled by a feature that generates the necessary stuff like in this file.
@c410-f3r Sure. Unfortunately I will probably not be able to look at it before September. Maybe it's best to at least allow the 0.5/master changes to rip out nodrop and go all in on MaybeUninit, land first, too.
The best approach I could think of, to avoid code duplication and keep compatibility with the legacy Array trait, was the use of a declarative macro to expand code according to a const-generics feature. E.g.:
It is a bit verbose but it works. Could there be any other way?
However, as you may have noticed, arrayvec uses the Index trait to define the len type that is currently attached to the Array trait as an associated parameter and this architecture don't play well with the above declarative macro. To make everything work properly, the Index bound should be removed from Array to be incorporated directly into the ArrayVec struct. E.g.:
I don't have a plan for how to handle const generics before it's stable. Looks like you have a cool plan there, that could work, but it doesn't seem appropriate for a crate feature - the whole API changes when the feature is enabled. It looks more like it has to be a fork or a separate version.
On the topic of indices, that is an expected loss. I'd say we don't want an index parameter. Either the index type should depend on N (if possible), or it simply will have to be a "hardcoded" index type. (Maybe have several if we can type alias the Main variants?)
What I said of making the whole API change might be premature - there could be ways with type aliases and other ways to completely mask the const generics?
With that said, once we go all out on stable const generics, we don't need the old Array trait, of course, and we should remove it and forget it -- make the best API we can with const generics.
The problem with this approach is that #96 will still remain (maybe remove Item from Array?) and it will not be possible to define a reasonable type Index for a given N (Index probably won't be relevant with constant generics).
Potentially bad idea: why not make Index a [u8; M] where M is an appropriate number given the size and alignment of the rest of the array? Then, that number could be cast to the appropriate actual integer type using from_ne_bytes. Pretty much all of the maths require to make this work will be optimised out by the compiler and it'll be optimal for each usage.
Sounds good, will work well on x86 at least I guess. For other platforms, there might be alignment issues, e.g. if you have a 256 byte array of bytes, it requires a 16 bit integer as index, but that will potentially not be aligned, leading to more expensive stores/loads.
I just did a write-up on implementing ArrayVec using const generics and found it can be implemented fairly easily by dropping the Array trait and using an array directly.
@bluss I think a better intermediary solution could be to add a const-generics feature that optionally retrofits the existing Array trait to impl for all arrays. I am looking at using this in the hnsw crate right now (currently switching to const generics), but that is a blocker for me, so I am temporarily implementing it with regular arrays and some tricks.
Perhaps the best approach would be to have an unstable 2.0 branch that could be used by folks who want const generics but is kept in an alpha state until those features are merged. A feature isn't the best as the API should change for const generics IMHO.
Activity
bluss commentedon Jul 10, 2019
Yes! Const generics is "on the roadmap" as the item for ArrayVec 2.0. See #48
c410-f3r commentedon Jul 10, 2019
Hi @bluss Glad to know your presence.
I opened a PR that adds support for constant generics in
rust-smallvec, can I do same witharrayvec? It is basically a macro controlled by a feature that generates the necessary stuff like in this file.bluss commentedon Jul 10, 2019
@c410-f3r Sure. Unfortunately I will probably not be able to look at it before September. Maybe it's best to at least allow the 0.5/master changes to rip out nodrop and go all in on MaybeUninit, land first, too.
c410-f3r commentedon Sep 16, 2019
@bluss
The best approach I could think of, to avoid code duplication and keep compatibility with the legacy
Arraytrait, was the use of a declarative macro to expand code according to aconst-genericsfeature. E.g.:Resolves into:
or
It is a bit verbose but it works. Could there be any other way?
However, as you may have noticed,
arrayvecuses theIndextrait to define thelentype that is currently attached to theArraytrait as an associated parameter and this architecture don't play well with the above declarative macro. To make everything work properly, theIndexbound should be removed fromArrayto be incorporated directly into theArrayVecstruct. E.g.:Resolves into:
or
Please, let me know what you think about this whole process.
bluss commentedon Sep 17, 2019
I don't have a plan for how to handle const generics before it's stable. Looks like you have a cool plan there, that could work, but it doesn't seem appropriate for a crate feature - the whole API changes when the feature is enabled. It looks more like it has to be a fork or a separate version.
On the topic of indices, that is an expected loss. I'd say we don't want an index parameter. Either the index type should depend on N (if possible), or it simply will have to be a "hardcoded" index type. (Maybe have several if we can type alias the Main variants?)
c410-f3r commentedon Sep 18, 2019
I see. Stabilization of
const_genericsshouldn't take too long oncelazy normalizationlandsbluss commentedon Sep 18, 2019
What I said of making the whole API change might be premature - there could be ways with type aliases and other ways to completely mask the const generics?
With that said, once we go all out on stable const generics, we don't need the old Array trait, of course, and we should remove it and forget it -- make the best API we can with const generics.
c410-f3r commentedon Sep 19, 2019
Well, it is possible to implement
Arrayfor[T; N]. E.g.:The problem with this approach is that #96 will still remain (maybe remove
ItemfromArray?) and it will not be possible to define a reasonable typeIndexfor a givenN(Indexprobably won't be relevant with constant generics).clarfonthey commentedon Sep 23, 2019
Potentially bad idea: why not make
Indexa[u8; M]whereMis an appropriate number given the size and alignment of the rest of the array? Then, that number could be cast to the appropriate actual integer type usingfrom_ne_bytes. Pretty much all of the maths require to make this work will be optimised out by the compiler and it'll be optimal for each usage.tbu- commentedon Sep 23, 2019
Sounds good, will work well on x86 at least I guess. For other platforms, there might be alignment issues, e.g. if you have a 256 byte array of bytes, it requires a 16 bit integer as index, but that will potentially not be aligned, leading to more expensive stores/loads.
clarfonthey commentedon Sep 23, 2019
That's actually a good point; I wonder if there would be an easy way around that besides forcing alignment.
bluss commentedon Sep 25, 2019
Just to say that it's possible to define a quick Array trait wrapper from outside the crate (using arrayvec from master).
Just in this code, I found ICEs in most of the things I wanted to do, so it doesn't seem to be exactly ready 🙂
(using rustc 1.39.0-nightly (9b9d2aff8 2019-09-19) )
Michael-F-Bryan commentedon Nov 14, 2019
I just did a write-up on implementing
ArrayVecusing const generics and found it can be implemented fairly easily by dropping theArraytrait and using an array directly.Throughout the entire process I didn't hit a single ICE, so maybe it's worth giving another shot?
bluss commentedon Jun 30, 2020
@Michael-F-Bryan Yes, I think we should, if it makes sense. There might be other crates already filling this need?
vadixidav commentedon Aug 16, 2020
@bluss I think a better intermediary solution could be to add a
const-genericsfeature that optionally retrofits the existing Array trait to impl for all arrays. I am looking at using this in thehnswcrate right now (currently switching to const generics), but that is a blocker for me, so I am temporarily implementing it with regular arrays and some tricks.clarfonthey commentedon Aug 18, 2020
Perhaps the best approach would be to have an unstable 2.0 branch that could be used by folks who want const generics but is kept in an alpha state until those features are merged. A feature isn't the best as the API should change for const generics IMHO.