Skip to content

Fix errors when a generic type comes up during bitsize macro processing#77

Merged
hecatia-elegua merged 2 commits into
hecatia-elegua:mainfrom
kitlith:generic_struct
Aug 25, 2023
Merged

Fix errors when a generic type comes up during bitsize macro processing#77
hecatia-elegua merged 2 commits into
hecatia-elegua:mainfrom
kitlith:generic_struct

Conversation

@kitlith
Copy link
Copy Markdown
Contributor

@kitlith kitlith commented Aug 21, 2023

Fixes #76, supersedes #64.

Adding angle brackets around a type forces it into type context, which lets us avoid having to thread things through syn's type hierarchy to try to get it to generate a turbofish. (This is kind of like a turbofish, but pointing the opposite direction.)

Trying to derive bitsize on structs that are themselves generic still fails, though I have another branch where I'm working on that: https://github.com/kitlith/bilge/tree/generic_derive

I figure I'll throw it up as a draft PR if/when this one is merged.

Still TODO: automatic implementation of bitsize on generic struct.
@hecatia-elegua
Copy link
Copy Markdown
Owner

hecatia-elegua commented Aug 21, 2023

Thank you :)
I don't mind the test failing for now, but could clear it like this:

#[cfg(not(feature = "nightly"))]
mod generic {
    use super::*;
    //...
}

Also thanks for the generic derive exploration.
Looking at it, I'm not sure how generics are supposed to work in bilge...
field: T would mean it could be e.g. u8 or u16, so you can't specify the bitsize directly, in which case we would kinda need to have a way to specify a calculated bitsize and have to possibly change a lot more.
We could add just the PhantomData part?

@kitlith
Copy link
Copy Markdown
Contributor Author

kitlith commented Aug 21, 2023

I just need to impl const on nightly, shouldn't be too difficult to pull off.

with generic const exprs we could provide a way to elide the user provided size and just use the computed one everywhere, but for now, I was planning to just get structs similar to phantom data functional and allow for post monomorphization errors for size mismatches.

@kitlith
Copy link
Copy Markdown
Contributor Author

kitlith commented Aug 21, 2023

For context: my example use case here is for a wrapper struct for (de)serializing the least significant bits of a fixed point number

use az::{WrappingAs, WrappingCast, WrappingCastFrom};
use bilge::prelude::*;
use fixed::traits::Fixed;
use std::marker::PhantomData;
use std::ops::BitAnd;

#[derive(Copy, Clone, Debug)]
pub struct Fix<Repr, N>(pub N, PhantomData<Repr>);

impl<Repr: Number, N> Bitsized for Fix<Repr, N> {
    type ArbitraryInt = Repr;
    const BITS: usize = Repr::BITS;
    const MAX: Self::ArbitraryInt = Self::ArbitraryInt::MAX;
}

// NOTE: i'm mostly working on stable. on nightly you'd need to impl const these
impl<Repr: Number, N: Fixed> From<Repr> for Fix<Repr, N>
where
    Repr::UnderlyingType: WrappingCast<N::Bits>,
{
    fn from(repr: Repr) -> Self {
        let shift: u32 = N::Bits::BITS - Repr::BITS as u32;
        let val: N::Bits = repr.value().wrapping_as();
        // sign extend
        let val = val << shift >> shift;
        Fix(N::from_bits(val), PhantomData)
    }
}

// TODO: should really also do impls for the primitives: u8 .. u128
// for now i'm using workarounds of UInt<u8, 8>, etc
impl<N: Fixed, T: Copy + Clone + BitAnd<Output = T>, const BITS: usize> From<Fix<UInt<T, BITS>, N>> for UInt<T, BITS>
where
    UInt<T, BITS>: Number<UnderlyingType = T>,
    N::Bits: WrappingCast<<UInt<T, BITS> as Number>::UnderlyingType>,
{
    fn from(val: Fix<UInt<T, BITS>, N>) -> UInt<T, BITS> {
        let raw_value: <UInt<T, BITS> as Number>::UnderlyingType = val.0.to_bits().wrapping_as();
        // it'll be nice to have signed arbitrary integers whenever that happens -- i'm not actually checking if it's in-range here.
        UInt::<T, BITS>::new(raw_value & UInt::<T, BITS>::MASK)
    }
}

i.e.: it's a single uX in disguise and with type conversion glue. Might be something to put in a derive macro at some point, but right now i'm okay with manual impls.

Comment thread tests/struct.rs
}
);

// do we want to be able to derive automatically on structs where the user has specified phantomdata?
Copy link
Copy Markdown
Owner

@hecatia-elegua hecatia-elegua Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be done by removing the PhantomData field in bitsize_internal and adding it back at the end. We should do that, in the other PR though.

@hecatia-elegua hecatia-elegua merged commit 41843c8 into hecatia-elegua:main Aug 25, 2023
@kitlith kitlith deleted the generic_struct branch August 28, 2023 08:06
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

Successfully merging this pull request may close these issues.

bitsize macro doesn't handle generic type that implements Bitsized correctly without manually including a turbofish

2 participants