-
Notifications
You must be signed in to change notification settings - Fork 230
Vec: add const-erased "view" #424
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
Conversation
ac48463 to
3a62322
Compare
| // NOTE order is important for optimizations. the `len` first layout lets the compiler optimize | ||
| // `new` to: reserve stack space and zero the first word. With the fields in the reverse order | ||
| // the compiler optimizes `new` to `memclr`-ing the *entire* stack space, including the `buffer` | ||
| // field which should be left uninitialized. Optimizations were last checked with Rust 1.60 |
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.
I think this is not relevant anymore. The unsized type is always necessarily the last so the optimization should take place.
From what I've seen there is no current plan to stabilize a manual way to implement
This works in providing all the API as we want, but it comes at the cost of having the documentation be completely lost because it is generated on the |
|
The build error happens on main for me too. |
Fix #371
This PR adds a
VecView<T>struct that is!Sizedand wraps aVec<T, N>, providing the same functionality (except those that require taking the vec by-value). It also makes the implementation for all methods onVec<T, N>delegate to theVecViewimplementation, reducing the need for monomorphised code for eachvalue of Nused.Example:
This change should brings the following benefits:
Better binaries
heapless-bytestoo and then everywhere else.Better ergonomics
Since
Vecalways requires theNto be specified, this makes generic code much less convenient.For example a lot of our firmware write responses to buffers passed by the caller. To make that code generic implies having functions like:
With this PR it could instead become:
Which is much simpler to use.
This also allows using this method in a trait with sacrificing object safety. This is the main driver for this feature, before the binary/compile-time improvements.
TODO
VecViewandVec. In the PR all methods are documented twice, this could lead to the docs getting out of sync. Soluctions could be to implement the functionality onVecInnerand use a macro to generate the method for bothVecandVecView, or having one of the documentation refer to the other.Vecof heapless, but it is certain that other consumers of this crate could benefit from the same gains for other structures in heapless.CoerceUnsizedtraits.I'm not sure about this one because the traits are unstable, but I would aim for the possibility of coercing a
Vecinto aVecViewwith the same ease as it is to convert a&[T; N]to a&[T]. I don't really know where the language is going with this and whether the#[repr(transparent)]wrapper could turn out to be a blocker for that functionality.