-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Adding ArrayVec
to core
#3316
base: master
Are you sure you want to change the base?
Conversation
There are still thigns to review. Just need some time to address everything
The code duplication is unlikely to be resolvable via either language or compiler improvements. It's somewhat fundamental, as the code won't be 100% identical (it's similar to taking
That is similar to what I proposed, although perhaps I'm misunderstanding. |
The |
Thank you for the clarification, that nuance is out of my league so I leave it to you & the other maintainers to decide
I think it is possible with your implementation (which I mentioned in the doc) but just isn't in your linked example since there's no constructor for
Yes that was exactly my thought! It looks like TinyVec uses Thom's implementation, so maybe that's a good place to start and just add the |
an alternative is the storages proposal (wg-allocators issue) that allows an ArrayVec to be written also, I think also there's two Rendered links in the top comment, the broken one at the bottom should be removed. |
That is extremely clean, great suggestion.
You are correct,
Thanks for the catch, fixed this |
yes. i'd expect
yes, that's what the |
Wow, that concept really blows this right out of the water and I wish I had seen it earlier. It seems a bit like discussion has stalled unfortunately, the linked thread ended over a year ago and this one hasn't seen too much activity I will do some poking on it tomorrow |
assert_eq!(v[0], 7); | ||
|
||
// Many higher-order concepts from `Vec` work as well | ||
v.extend([1, 2, 3].iter().copied()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting one, since it has the same "panic vs truncate vs …" question that collecting has.
It reminds me of the discussions in #3271 (comment) -- maybe there could be a similar API for "extend or push or … within existing capacity" between normal Vec
and ArrayVec
.
This is an overstatement. There's even panics in built-ins, like That doesn't mean these APIs should panic, but I don't think it can be immediately discarded. |
Couldn't the same be done with |
RE panics: I'd probably prefer that we have both RE storage: It would be quite amazing to have I am overall 👍 at least on the idea of having an abstraction like this in |
I'm working on it, and will be bringing forward a project proposal early October. https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Recruiting.3A.20Storage.20API.20Project.20Group |
I think the generic storage idea is neat and might have some use cases, but it’s well into diminishing-returns territory IMO in terms of how complex it is compared to what it brings. It could live in a crate for the (comparatively) few cases where it’s useful, while more specific |
imho storages would be far more useful specifically because it would fit into the standard library's types in the way allocators currently do in nightly, that wouldn't really be possible just using an external crate because you couldn't use it with the standard types then, you'd have to specifically opt into using those storage-enabled types which is much less likely to happen throughout the crate ecosystem. with storages in the standard library 3rd-party crates could easily use them while still working with the standard types: fn third_party_code<S: Storage>(v: &mut Vec<i32, S>) { ... }
// works great:
let mut my_vec = vec![1, 2, 3];
third_party_code(&mut my_vec); that would not work with storages being a 3rd-party crate because the fn third_party_code<S: storages::Storage>(v: &mut storages::Vec<i32, S>) { ... }
// can't work:
let mut my_vec = vec![1, 2, 3];
third_party_code(&mut my_vec); // type mismatch Vec != storages::Vec |
Not really, the cleverness there is actually not the deref but the use of unsizing. Actually, I'm not sure why the deref is needed -- it's surprising that deref is needed at all there. It shouldn't be, but it seems to help method resolution perform the unsizing coersion. |
I love the storages idea a lot, especially because I felt greedy even mentioning the possibility of @CAD97 is there anything publicly available to help shape the storages design, or are you sort of attempting to get something working quietly before discussion adds noise? (completely understandable if so). Re: I will keep this RFC around as a draft while we see storages develop, because having a "desirable" API for this sort of construct will probably help shape the goals there, things like methods to construct the object on an existing memory buffer (and, I suppose, if storages wind up not making it - which I hope isn't the case). And it probably wouldn't be totally unreasonable to provide something like |
What if |
If ArrayVec is stable, that can't be done in a backwards-compatible way most likely. If any downstream has |
See four comments above: #3316 (comment) |
As @CAD97 mentioned above, since the second generic parameter of Unless that parameter is stable, it's not possible to write a conflicting impl on stable Rust, since any impl you can write today on stable Rust uses the default |
it should be possible to upgrade impl<T, const N: usize> !Allocator for [T; N] {} |
(whether (but yes; theoretically, if there is a blanket implementation from |
that seems to error out for me: can you make an example that doesn't error? |
There are several ecosystem crates doing this because they all provide slightly different functionality, although perhaps their functionality merges somewhat with As @SimonSapin indicates above, I doubt
Around these concerns, one likely needs reference analogs of We do need to understand the design space if this is going into core. We do not necessarily need a |
Around representations.. There exist some reasons for using a It appears |
enum SmallVec<T, const N: usize> {
Inline(ArrayVec<T, N>),
Heap(Vec<T>),
} |
Am I the only one who thinks that |
Any thoughts on As I noted above, we should add We maybe need a post-const-generics summary of the real differences between the ecosystem crates (or their ideal targets)? Is there some need for a |
"FooBar" meaning "a Bar backed by Foo" has precedents in std: |
The RFC proposes pub unsafe fn from_raw_parts_mut<'a, T, const N: usize>(
data: *mut [T; N],
len: usize,
) -> &'a mut Self<T, N> but this function is not implementable because the A different type (or generic storage that can be a pointer) would be needed to implement this specific use case. |
Agreed.
Instead, I think that having these
And if accessing the (entire) storage as a fixed-sized array is useful, maybe also:
I’m not sure what the RFC means by “to easily map to C's |
That looks like it would be trivially solved by storages by using |
In this case, would
The more I think about it, the more I think the answer is yes. Which means this RFC needs some big-ish tweaks. Reasoning:
Ah, good catch. Some sort of slice or reference-backed option would be needed to solve this, so I will be updating the RFC. My thoughts on construction aren't currently clearly expressed super well, but basically I am hoping to be able to construct an
That's correct, this is the pattern used when the callee is expected to manipulate items within Even the simple |
Right, that’s what I thought. So what’s needed to support these use cases is not constructing a brand new
|
Thanks for the clarification, this all handles the "Rust is caller" situation (with |
If Rust is called with this kind of C-like API, |
It's unclear if the Someone could build a
Ain't clear if you could make the unsized type
Yes, but I'll caution this always felt more diverse than
It's maybe cool then turning each of the resulting slices into |
That does not make it a proof that this a good idea. Else one would deduce that once a mistake is done in Besides your point is not even comparing completely similar cases: At least a |
i might have missed it in the RFC, but what i'd really like is to have a trait which covers all alloc-independent methods ( that way more portable code can be written which either receives an implementation from the outside or only needs to in the best case that'd exist not just for |
@rursprung FWIW, the |
i might be missing something, but i neither see implementations in this crate nor APIs for most of the actions (e.g. |
It is indeed an experimental crate. I bring it up more as food for thought regarding gaps in current |
Thanks all for reminding me about this PR. Since the time this was drafted, there have been two things I am aware of that are likely better solutions than this proposal:
I do think the ideas in this RFC have a place in core so I won't close this, but I think it's best to wait out one of the two things I linked to see what the best way to do that is (since it's likely that one of them will intrinsically give us a solution) |
Rendered
This RFC seeks to add a representation for variable-length data within a fixed-size array, with an interface similar to
Vec
. Goals are to unite and clarify existing concepts incore
, provide language users with a representation for a common data structure that is not heap-depdent, and ease some FFI relationships.Current use (subject to change) looks like:
Biggest issues to address
core::collections
(new),core::array
(existing), andcore::ext
(nex) have all been offered as suggestionsAny API should mirrorto panic or not to panic
simple operations likepush()
that will never (reasonably) fail forVec
are extremely failable forArrayVec
. I elected to make all potentiall failable methods return aResult
or similar, based on the belief thatcore
should attempt to never panic (inno_std
environments, panicking is generally UB)Vec
's existing API as much as possible, with possible additions for checked operationsArrayVec<[T; N]>
with deref traits to allow for some code reuse, see the comment here. I personally think that the interface has some more potential for confusion thanArrayVec<T, N>
, but the benefits are notable (I don't know enough to know if the code duplication comes from a compiler limitation or a language limitation).ArrayVec
(or perhaps more accurately in this case,BufVec
) being able to be backed by any slice, rather than just strictly arrays. I have yet to come up with a clean possible implementation for this, but think it is worth doing if it can be done without overhead or added confusionLinks