scroll_derive: support reference #101
Conversation
| @@ -128,6 +128,14 @@ fn impl_pwrite_field(ident: &proc_macro2::TokenStream, ty: &syn::Type) -> proc_m | |||
| _ => panic!("Pwrite derive with bad array constexpr"), | |||
| }, | |||
| syn::Type::Group(group) => impl_pwrite_field(ident, &group.elem), | |||
| syn::Type::Reference(reference) => match *reference.elem { | |||
| syn::Type::Slice(_) => quote! { | |||
| dst.gwrite_with(self.#ident, offset, ())? | |||
There was a problem hiding this comment.
refer
Lines 713 to 716 in 3cffc5a
There was a problem hiding this comment.
😅 haha yes i forgot pwrite for &[u8] has no ctx, though that is a bit quirky/assymetric, as it's pread has a usize ctx (e.g., how many things to read). r
| name: b"name", | ||
| }; | ||
| let mut bytes = vec![0; 32]; | ||
| assert_eq!(bytes.pwrite_with(&data, 0, LE).unwrap(), 7); |
There was a problem hiding this comment.
i'm surprised you can pass a &data reference to pwrite_with here, instead of just data
There was a problem hiding this comment.
the derivation of Pwrite will also generates a Pwrite impl for &Data. that's not my credit :)
| }, | ||
| p => quote! { #p } | ||
| syn::GenericParam::Lifetime(_) => None, |
There was a problem hiding this comment.
not clear to me why this branch is None here
There was a problem hiding this comment.
actually 'the heart of the fix' is here.
the prevoius code doesn't process lifetime properly, which will generates tokens like
impl<'b> ... for ...
where
... : ... ,
'b
^^ invalid syntax
| syn::GenericParam::Lifetime(_) => None, | ||
| p => Some(quote! { #p }), | ||
| }).collect(); | ||
| if !gi.is_empty() { |
There was a problem hiding this comment.
i think this is the heart of the fix, thank you for looking into this and doing it!
There was a problem hiding this comment.
no, this one doesn't matter here. a dangling where clause will be ignored by rust. I did more processing here just to make the generated token clearer.
|
Somewhat related, and wondering out loud, I hope there aren't similar bugs in the Cread derive code, as I think it is probably large copy pasted from the Pread derive code? anyway, great work and thanks for this fix, do you want a minor patch release for scroll_derive? if it's urgent i can release it now |
i can investigate more and propose a PR later.
nope, it's okay. thanks though. |
Closes #83