-
Notifications
You must be signed in to change notification settings - Fork 107
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
Cloning DSTs by bytes #2275
Labels
customer-request
Documents customer requests.
Comments
Could we just define: pub trait FromBytes {
fn clone(self: Box<Self>) -> Box<Self>
where
Self: KnownLayout
{
/* ... */
}
} Relatedly, we'd probably benefit from a more general pub trait FromBytes {
fn read_from_box<Src>(src: Box<Src>) -> Result<Box<Self>, SizeError<Box<Src>, Self>>
where
Src: ?Sized + IntoBytes + KnownLayout,
Self: KnownLayout
{
...
}
} |
@jswrenn I definitely prefer avoiding another trait, though I think that needs some adjustment:
I agree, though I do have some comments:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Working with DSTs is much improved but is still challenging - especially owned DSTs. Being able to simply perform a bytewise clone into a
Box
would be very helpful. I propose it be derived viaByteClone
.While you can
impl Clone for Box<LocalType>
due to it being#[fundamental]
andderive(ByteClone)
probably should, there's no built-in trait for&self -> Box<Self>
. So, this should also define a localByteClone
trait or something similar to generically define this.Sample:
Box<T: Unalign>
andBox<[u8]>
#2258 and the ability to construct aBox
from existing bytes for?Sized
.derive(ByteEq)
#2274, this bytewise clone can also optimize better than field-wise derived clone forSized + !Copy
types.fn byte_clone
orfn clone_bytes
? I like the readability ofverb-noun
, especially withclone_bytes_to_box
instead ofbyte_clone_box
. Ifclone_bytes
is preferred, then should the trait be namedCloneBytes
?Thenderive(ByteEq)
#2274 (comment) should apply for consistency.ByteHash
is already implemented.The text was updated successfully, but these errors were encountered: