-
Notifications
You must be signed in to change notification settings - Fork 37
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) doesn't work with fields containing an array of structs #54
Comments
Here's a simple example project that demonstrates the error: https://github.com/luser/scroll-derive-struct-array . I hit this in practice trying to convert my minidump crate to use scroll. The |
Hmmm yea this 100% needs to be fixed. I wrote most of the derive code quickly for my needs (simple pods at first) and it isn’t super robust, and definitely needs some more love. Even error messages aren’t great. Is requiring the type to be Default reasonable and we can just call default in the array perhaps another approach ? And then fill it up. Or your method should work too I think. L But yea I’ll merge anything fixing this :) |
I think this can easily be fixed by simply calling default and requiring anyone to also derive Copy and Default on their types, which kind of sucks? E.g., in your use case, https://github.com/luser/rust-minidump/blob/db4a828873d68f513261d285d274feb9febc9733/minidump-common/src/format.rs#L95 I don't know if you're in control of those annotations? If so, it should probably be fine? However, I would have to test that patch with goblin and other shit, because adding those new required bounds could break a bunch of stuff if someone hadn't implemented them. Consequently, I like your suggestion better, but I don't see an easy way to generate a finite rust array in syn, inline, off the top of my head (I haven't done proc macro stuff in a while). If anyone has any ideas, or PR which generates the inline array without Default (or unsafe like uninitialized) I'd be pretty happy, as this is definitely a deficiency :) |
I actually got myself unstuck on this by switching to |
I think the best way to fix this is to use |
I am running into that and considering first a TryFromCtx impl for &'a [T; N] or [T; N] ? then you don't need any macro magic because you are just reading a classical type right? |
The code that handles array fields initializes its temporary array to
[0; #size as usize]
, which doesn't work for an array of structs, like:You get:
The additional error about
Copy
is because the[x; N]
array initializer syntax will copy thex
value.I suppose the most straightforward way to fix both of these would be to change that code to just generate the array inline, like:
[ src.gread_with::<#whatever>(offset, ctx)?, src.gread_with::<#whatever>(offset, ctx)? ... ]
The text was updated successfully, but these errors were encountered: