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

derive(Pread) in structs with elements that don't use scroll::Endian as ctx #55

Open
kitlith opened this issue Sep 10, 2018 · 4 comments

Comments

@kitlith
Copy link

kitlith commented Sep 10, 2018

Writing an implementation for TryFromCtx<'_, T> where T != scroll::Endian (including T == ()) doesn't work if you want to use it in a struct where you're using #[derive(Pread)]. This can be worked around for cases where you didn't care about the ctx anyway, but it doesn't work in cases where you actually wanted to use another ctx type -- but I'm not sure how you'd be able to tell.

@m4b
Copy link
Owner

m4b commented Sep 10, 2018

I massively regret being convinced to change the default to (). I’m not sure how much it will break to change it back tho...

@m4b
Copy link
Owner

m4b commented Sep 10, 2018

Oh I think I see; you just want it to use unit in it’s derive. Yea; this is getting awkward because the derive traits use endian ctx (as they should) for default but the default ctx when elided is unit.

One hack here is to add an annotation to the derive to tell it to use a particular ctx (which I could see would definitely have usecases outside of forcing it to use Unit)

@kitlith
Copy link
Author

kitlith commented Sep 10, 2018

From my perspective... unit and any other CTX should work together, but I don't know how practical that'd be to detect. EDIT: Maybe you could do a impl From<Endian> for ()?

If you can do a sort of `#[derive(TryFromCtx)] then that would make most sense personally, but alternatively a #[scroll(ctx=CtxType)] as you suggest should work.

@willglynn
Copy link
Contributor

willglynn commented Oct 23, 2018

I also have this problem but I've been working it from a different angle. I now have a branch of scroll_derive which is generic over context types, such that e.g. TryFromCtx<C> for Struct is available for any context C which also has all TryFromCtx<C> for Member. No extra attributes needed; if some context can process all the fields of a struct, scroll_derive will make it process the struct too.

Given the definition in examples/main.rs:

#[derive(Debug, PartialEq, Pread, Pwrite, IOread, IOwrite, SizeWith)]
#[repr(C)]
struct Data {
    id: u32,
    timestamp: f64,
    arr: [u16; 2],
}

My branch generates:

impl <'a, C> ::scroll::ctx::TryFromCtx<'a, C> for Data
where Data: 'a,
 C: Copy,
 f64: ::scroll::ctx::TryFromCtx<'a, C, Error = ::scroll::Error>,
 u16: ::scroll::ctx::TryFromCtx<'a, C, Error = ::scroll::Error>,
 u32: ::scroll::ctx::TryFromCtx<'a, C, Error = ::scroll::Error>
{
    type Error = ::scroll::Error;

    #[inline]
    fn try_from_ctx(src: &'a [u8], ctx: C) -> ::scroll::export::result::Result<(Self, usize), Self::Error> {
        use ::scroll::Pread;
        let offset = &mut 0;
        let data =
            Data{id: src.gread_with::<u32>(offset, ctx)?,
                 timestamp: src.gread_with::<f64>(offset, ctx)?,
                 arr:
                     {
                         let mut __tmp: [u16; 2] = [0; 2u64 as usize];
                         src.gread_inout_with(offset, &mut __tmp, ctx)?;
                         __tmp
                     },
                   };
        Ok((data, *offset))
    }
}

impl <'a, C> ::scroll::ctx::TryIntoCtx<C> for Data
where C: Copy,
 f64: ::scroll::ctx::TryIntoCtx<C, Error = ::scroll::Error>,
 u16: ::scroll::ctx::TryIntoCtx<C, Error = ::scroll::Error>,
 u32: ::scroll::ctx::TryIntoCtx<C, Error = ::scroll::Error>
{
    type Error = ::scroll::Error;

    #[inline]
    fn try_into_ctx(&self, dst: &mut [u8], ctx: C) -> ::scroll::export::result::Result<usize, Self::Error> {
        use ::scroll::Pwrite;
        let offset = &mut 0;
        dst.gwrite_with(&self.id, offset, ctx)?;
        dst.gwrite_with(&self.timestamp, offset, ctx)?;
        for i in 0..self.arr.len() {
            dst.gwrite_with(&self.arr[i], offset, ctx)?;
        };
        Ok(*offset)
    }
}

// FromCtx<C> and IntoCtx<C> proceed in the same fashion

I couldn't figure out how to make this work for impl TryIntoCtx for &'a Data though, so this depends on changing fn try_into_ctx(self, …) into fn try_into_ctx(&self, …). I raised this as a possibility in #46 but there's discussion to have over there before I open any PRs.

Edit: links to these scroll + scroll_derive branches

@m4b m4b transferred this issue from m4b/scroll_derive Sep 6, 2019
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