Use a sealed trait to constrain VecWithInitialized #3450
Conversation
Apply Darksonn's rustfmt fix (thanks!) Co-authored-by: Alice Ryhl <alice@ryhl.io>
| pub(crate) fn get_read_buf<'a>(&'a mut self) -> ReadBuf<'a> { | ||
| pub(crate) fn get_read_buf(&mut self) -> ReadBuf<'_> { |
There was a problem hiding this comment.
Can you undo this change? I'd like to keep the lifetime inside the from_raw_parts_mut call.
There was a problem hiding this comment.
Sure. This was a bit of a gratuitous change in passing 😄 . May I ask why you prefer the explicit 'a though?
There was a problem hiding this comment.
It's mostly that std::slice::from_raw_parts_mut detaches the lifetime of the resulting slice from the owner, and I try to avoid that as much as possible.
There was a problem hiding this comment.
Right, from_raw_parts_mut uses an "unbounded" lifetime param. However, inference should select the proper output lifetime when using the '_ wildcard.
But, I reverted this part.
| mod private { | ||
| pub trait Sealed {} | ||
|
|
||
| impl Sealed for Vec<u8> {} | ||
| impl Sealed for &mut Vec<u8> {} | ||
| } | ||
|
|
||
| /// A sealed trait that constrains the generic type parameter in `VecWithInitialized<V>`. That struct's safety relies | ||
| /// on certain invariants upheld by `Vec<u8>`. | ||
| pub(crate) trait VecU8: AsMut<Vec<u8>> + private::Sealed {} |
There was a problem hiding this comment.
The question is if it is better to just make the trait unsafe instead of sealing it.
There was a problem hiding this comment.
An unsafe trait could work as well. I felt that a sealed safe trait would be a bit better in that:
- minimizes
unsafes - sealing and defining a controlled set of impls seemed pretty succinct and to the point
However, I'm open to hearing arguments for making it an unsafe trait instead. Either way, the thing that somewhat irked me with existing code is that we weren't capturing safety via types.
There was a problem hiding this comment.
I mean, either is fine. I was mostly wondering to myself if sealing even works without a crate boundary, but after some more thought, I guess it does.
There was a problem hiding this comment.
Yeah, it's essentially sealing from the rest of the crate. This allows specifying the valid types in the vec_with_initialized module, which is nice since it's the same source file as the uses and where the commentary about Vec's invariants lives.
Co-authored-by: Alice Ryhl <alice@ryhl.io>
Co-authored-by: Alice Ryhl <alice@ryhl.io>
Co-authored-by: Alice Ryhl <alice@ryhl.io>
…yd/tokio into vec_with_init_sealed_trait
|
Thanks! |
Use a sealed trait to constrain
VecWithInitialized's generic type parameter. The safety of that struct relies on invariants upheld byVec<u8>. The struct currently has anunsafe newassociated fn whereV: AsMut<Vec<u8>>.Motivation
Avoids a bit of
unsafe.Solution
This PR defines a crate private
VecU8trait and implements it forVec<u8>and&mut Vec<u8>.cc @Darksonn as we discussed this a bit in #3426