Skip to content
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

Open
chrysn opened this issue Jan 27, 2023 · 3 comments
Open

Genericity over width #79

chrysn opened this issue Jan 27, 2023 · 3 comments

Comments

@chrysn
Copy link

chrysn commented Jan 27, 2023

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.

@akhilles
Copy link
Collaborator

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.

@jamesmunns
Copy link

jamesmunns commented Apr 21, 2023

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: 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.

@jamesmunns
Copy link

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)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants