Add VarZeroCow #5809
Add VarZeroCow #5809
Conversation
Manishearth
force-pushed
the
varzerocow
branch
2 times, most recently
from
79fe437 to
53f0515
Compare
Manishearth
force-pushed
the
varzerocow
branch
from
53f0515 to
804fbf9
Compare
| #[cfg(feature = "serde")] | ||
| impl<'a, 'de: 'a, V: VarULE + ?Sized> serde::Deserialize<'de> for VarZeroCow<'a, V> | ||
| where | ||
| Box<V>: serde::Deserialize<'de>, |
There was a problem hiding this comment.
Thought: Since the Deserialize impl almost always requires invoking EncodeAsVarULE, it would be nice if we could encapsulate that in VarZeroCow's impl.
But, I don't see a super easy way to do that. VarULE doesn't have an associated Sized type that we can put the bound on. We could add an additional type parameter to VarZeroCow, but maybe that is overkill.
There was a problem hiding this comment.
I don't think we need to do that, and I'd like to leave the flexibility on this to individual types.
| @@ -47,6 +48,35 @@ pub struct VarZeroCow<'a, V: ?Sized> { | |||
| unsafe impl<'a, V: ?Sized> Send for VarZeroCow<'a, V> {} | |||
| unsafe impl<'a, V: ?Sized> Sync for VarZeroCow<'a, V> {} | |||
|
|
|||
| impl<'a, V: ?Sized> Clone for VarZeroCow<'a, V> { | |||
There was a problem hiding this comment.
Suggestion: maybe convert it to an enum PrivateCow<'a, V> { Owned(Box<V>), Borrowed(&'a V) }, where you can #[derive(Clone)], and then you don't need to duplicate the unsafe code
There was a problem hiding this comment.
I'd still need unsafe code to extract the Box<V> and &'a V. These could be added as methods, but I think it only cleans up one method (this one).
There was a problem hiding this comment.
I still think the encapsulation would be better, but not strongly
Fixes #5561
We can add more methods as needed, right now I think it covers most use cases.