Open chrysn opened 1 year ago
I'd love to make update
generic over Width
, but unfortunately this is blocked on const fn
support in traits. Currently, we can either have a generic implementation of update
or a const
implementation, but not both.
For what it's worth, we're adding CRC support to postcard, and it required quite a bit of macros (and paste
) to make happen.
I managed to get a more generic approach working with some non-ideal hacks, see: https://github.com/huntc/postcard/pull/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.