-
Notifications
You must be signed in to change notification settings - Fork 187
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
Vec: add a "View" that can mutate the vec and erase the const generic. #425
Conversation
impl Unsize<VecView> for Vec
impl Unsize<VecView> for Vec
62f955e
to
ca1512b
Compare
I think I fixed the documentation issues. By having a #[cfg(doc)]
#[doc(hidden)]
pub use vec::VecInner as __private_api_do_not_use; The documentation for the aliases is generated with all the methods. See rust-lang/rust#119015 The miri test fails were because not all methods were forwarded to the |
Yeah this is painful. |
What do you mean by that? What I meant was that without the Which I think is almost as good as it can get. The only "confusing" part is the second line showing it being an alias |
Sorry. It was just a comment.
|
8e22a81
to
7cb0e80
Compare
8800b84
to
5996fab
Compare
impl Unsize<VecView> for Vec
5996fab
to
095c17c
Compare
490c482
to
05096fd
Compare
I made a detailed blog post explaining the motivations and possible approaches to this PR: https://sgued.fr/blog/heapless-howto/ |
05096fd
to
f50232b
Compare
f50232b
to
cdfe308
Compare
Yes, which is why I am asking this now so we don't immediately need a breaking change for
I guess |
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 working as intended. I don't think the naming is a blocker, but this should be discussed further in #442.
Once this is merged I'll make a similar PR for String and the other types in heapless that could benefit from this pattern. |
Thanks, @sosthene-nitrokey! |
Alternative approach to #424, with even better ergonomics and features, but slightly more confusing documentation.
Pros
This approach is more "scalable" in the sense that using it means that other structures that use a
Vec
internally can also have aView
type that is simply the!Sized
version of the same structure. This would be useful for the implementations ofString
,LinearMap
,BinaryHeap
, andheapless-bytes
.This approach also enables implicit conversion between the owned type and the view without needing to make the owned type
Deref
to the view.Cons
The
Vec
andVecView
types being aliases to a private type could be confusing.Not totally sure about the semver compatibility of migrating to type alias for something that used to be a struct. Cargo semver-checks seems to complain.
Closes #424.