Support for infinitely nesting VarZeroVecs #1065
Conversation
Manishearth
force-pushed
the
vzv-ule
branch
from
8f2d233 to
9d31cda
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
Manishearth
force-pushed
the
vzv-ule
branch
from
968bf0b to
ab5e4f1
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
Manishearth
changed the title
| panic!( | ||
| "Called out-of-bounds insert() on VarZeroVec, index {} len {}", | ||
| index, len | ||
| ); |
There was a problem hiding this comment.
Nit: I prefer to return an error code than to panic. I know this differs from std::vec, but we have different requirements than std. The function could return Result<(), Error>
There was a problem hiding this comment.
I don't really agree; I'd like to match std::vec on behavior here. We can also provide a try_insert but I'd rather track fallible APIs separately? Because adding them would involve ZeroMap changes too
| } | ||
|
|
||
| /// Get the number of elements in this vector | ||
| pub fn len(&self) -> usize { |
There was a problem hiding this comment.
Grumble: I don't like defining these functions again and again and again.
Can we omit these for the time being and just ask people to call as_varzerovec() ?
There was a problem hiding this comment.
I think if this is going to be public we should probably not do that
| pub struct VarZeroVecULE<T> { | ||
| marker: PhantomData<[T]>, | ||
| /// The original slice this was constructed from | ||
| entire_slice: [u8], |
There was a problem hiding this comment.
Thought: could VarZeroVecOwned be a Box<VarZeroVecULE<T>> ?
There was a problem hiding this comment.
Not quite because that's not growable. But yes I'd have loved to unify the two, I could use some kind of trait that encapsulates .get_components() but it's not really worth it IMO because that pollutes the public API with a confusing generic type.
| /// There must be no padding bytes involved in this type: [`Self::as_byte_slice()`] MUST return | ||
| /// a slice of initialized bytes provided that `Self` is initialized. |
There was a problem hiding this comment.
- The safety docs are partly here and partly on the
from_byte_slice_uncheckedfunction - Can we / should we say something like,
mem::size_of_val(bytes)must equalmem::size_of_val(self)?
There was a problem hiding this comment.
- Done. I wrote new docs instead of moving them,
from_byte_slice_uncheckedstill has duplicated safety docs, but I think it's good to be redundant for unsafe stuff. - I don't think we need to, everything is totally safe even if the bytes are a subset, it just becomes nearly impossible to implement
parse_byte_slicecorrectly.
There was a problem hiding this comment.
I was thinking about equality when I said that.
(this might be getting a little out of hand)
Things I have not done in this PR, but don't plan to do: