-
Notifications
You must be signed in to change notification settings - Fork 48
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
Genericity over width #79
Comments
I'd love to make |
For what it's worth, we're adding CRC support to postcard, and it required quite a bit of macros (and I managed to get a more generic approach working with some non-ideal hacks, see: huntc/postcard#1, though I'm likely just going to stick to the paste macros. I wanted to share in case an aspect of this was useful to y'all for implementing generics in the future, or if I could help with any of the impl. |
Extracted, it might be possible to have "best of both worlds" with layering a trait over the Digest type, where the impls are inherent, but there is a (sealed) trait that lets you be generic over it: pub trait Digestif {
type Pod: PartialEq;
fn update(&mut self, data: &[u8]);
fn finalize(self) -> Self::Pod;
fn bytes_to_pod(bytes: &[u8]) -> Result<Self::Pod, ()>;
}
macro_rules! impl_digestif {
($( $int:ty ),*) => {
$(
impl<'a> Digestif for Digest<'a, $int> {
type Pod = $int;
#[inline(always)]
fn update(&mut self, data: &[u8]) {
self.update(data);
}
#[inline(always)]
fn finalize(self) -> Self::Pod {
self.finalize()
}
#[inline]
fn bytes_to_pod(bytes: &[u8]) -> Result<Self::Pod, ()> {
let arr = bytes.try_into().map_err(drop)?;
Ok(<$int>::from_le_bytes(arr))
}
}
)*
};
}
impl_digestif!(u8, u16, u32, u64, u128); and can be used downstream something like this: pub fn from_bytes_width<'a, T, C>(s: &'a [u8], digest: C) -> Result<T>
where
T: Deserialize<'a>,
C: Digestif + 'a,
{
let flav = CrcModifier::new(Slice::new(s), digest);
let mut deserializer = Deserializer::from_flavor(flav);
let r = T::deserialize(&mut deserializer)?;
let _ = deserializer.finalize()?;
Ok(r)
} |
Writing components that maintain generic CRCs is difficult using this crate because the Width trait is not used to its full extent. While the generic component can demand that a W: Width be specified, many places still require a concrete type because a W: Width is insufficient.
The two places I ran into this was the lack of an
.update()
on a digest (it's only implemented for the 5 concrete widths) and the.digest()
method.I think that it'd be possible to move to generic implementations and leverage the Width trait; code that currently sits in the concrete implementations would move into Width. The change would almost be compatible: As Width is not sealed, the trait would need to gain non-provided methods. (It's a change that's very unlikely to break any code, as just implementing Width doesn't let a user do anything with it so far, but it's technically still breaking).
As a side effect, the built documentation would become much more usable -- for example, the
crc::Crc
struct would only have one impl block instead of 5.The text was updated successfully, but these errors were encountered: