Add more trait implementations for [T; $N] #29395
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
huonw
added
the
T-libs-api
|
r? @alexcrichton (Aaron might still be on leave?) |
src/libcore/array.rs
Outdated
There was a problem hiding this comment.
Good catch! Fixed.
|
Didn't we have to back off on more of these impls because it makes the compiler deeply unhappy? Can't remember the details. Possibly that was a bunch of |
|
I don’t know, other than libcoretest compiles and passes with this change. |
|
I personally have pushed back on impls like this in the past, it tends to have a huge bloat on metadata size for very little gain. The real solution here is to get this impls to be generic over the size somehow, as we all know, but I'm not convinced that adding new impls every other day will get us much farther than where we are right now. |
|
A good amount of the metadata bloat was due to a few issues I fixed. Could we measure it now? |
|
Yeah if we've gotten the file size under control then these wouldn't be too bad to add I don't think. |
A number of traits are implemented for `[T; $N]` in a macro with `$N` between 0 and 32. Ideally, the language would support a generic integer `N` in a single `impl`. In the meantime, the macro is what we have. This adds more values on `$N` in the macro: powers of 2 and 10 up to 2^30. This doubles the total number of `impl`s generated by this macro. I’m experimenting with a `String`-like type whose backing storage is generic on `T: AsMut<[u8]>`. Buffers like `[u8; 16 * 1024]` are reasonable to allocate on the stack (`std::io::copy` does it) but would previously not implement `AsMut<[u8]>`.
SimonSapin
force-pushed
the
moar-array-impls
branch
from
034f5fa to
db9d507
Compare
|
@SimonSapin, can you measure the impact of these impls on the size of the libcore artifact? |
|
Do you mean the size of |
|
On commit 05eb81a (which this PR branches from), |
|
I personally feel that tacking on 2MB of extra metadata for this sort of data isn't worth it right now, but I'll bring this up in the next triage meeting to see what others think (although discussion can certainly happen before that!) |
|
I’m a bit surprised this is this much data, but oh well. (I’m not so attached to this PR as I ended up making a custom trait anyway.) |
|
The libs team discussed this during triage today and the decision was to close this for now. We're fine adding impls for a fixed number of arrays (e.g. the 0-32 we already have), but adding more numbers like this moves into the "a little bit too arbitrary" territory that we'd prefer to hold off for a more proper generic mechanism. Thanks regardless for the PR though @SimonSapin! |
|
+1 to the wish for either a larger arbitrary range (at least 0-64) and/or inclusion of common CS and cryptographic primitive sizes (256, 384, 512, 1024, 2048, 4096). seems like both would be an easy solution to a common usability problem until const generics land. |
A number of traits are implemented for
[T; $N]in a macro with$Nbetween 0 and 32.Ideally, the language would support a generic integer
Nin a singleimpl. In the meantime, the macro is what we have.This adds more values on
$Nin the macro: powers of 2 and 10 up to 2^30. This doubles the total number ofimpls generated by this macro.I’m experimenting with a
String-like type whose backing storage is generic onT: AsMut<[u8]>. Buffers like[u8; 16 * 1024]are reasonable to allocate on the stack (std::io::copydoes it) but would previously not implementAsMut<[u8]>.