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

Referencing fields in #[brw] is impossible due to value/ref differences between BinRead and BinWrite #179

Open
DCNick3 opened this issue Dec 12, 2022 · 1 comment
Labels
confusing-api Oops! Confusion occurred

Comments

@DCNick3
Copy link
Contributor

DCNick3 commented Dec 12, 2022

It's tricky (impossible?) to use assert attribute in brw key. For example this code:

#[derive(BinRead, BinWrite)]
#[brw(assert(unused == 0u8))]
struct Hello {
  unused: u8
}

Will give you an error Can't compare &u8 with u8, probably because unused is a &u8. But if you try to dereference it with * you get Type u8 cannot be dereferenced. Really confusing.

Ultimately this is due to br giving you fields by-value and bw - by-reference. So, even when you want to enforce the invariant for both reading and writing, you would need to write two implementations in most cases.

Maybe it's worth making them both provide fields by-reference? 🤔 Not sure what other impact it may have.

It seems there was a mention of brw usability for assert being questionable in #3, but there doesn't seem to be much discussion...

@csnover csnover changed the title brw(assert) is tricky Referencing fields in #[brw] is impossible due to value/ref differences between BinRead and BinWrite Sep 22, 2023
@csnover csnover added the confusing-api Oops! Confusion occurred label Sep 22, 2023
@zodiia
Copy link

zodiia commented Apr 4, 2024

Hi, I ran into the same issue when trying to use args (and args_raw). If I use the field, it will tell me I'm using a reference when it's expecting a value, and if I dereference it, it just tells me I can't dereference a value. It only happens if I try to use args with the brw macro.

For those wondering that might come by, here is a possible fix (for args at least, but I guess it would be the same kind of fix for assert):

// Before
#[brw(args(value))] // won't compile

// After
#[br(args(value))]
#[bw(args(*value))]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confusing-api Oops! Confusion occurred
Projects
None yet
Development

No branches or pull requests

3 participants