-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Merge I128/Ixx and U128/Uxx variants #323
Conversation
017784b
to
330cd0a
Compare
Looks reasonable, thanks! I think we should add a QEMU test for the larger bitfield range. |
Range is exclusive. To match the MSB 32 for `range.end` has to be allowed.
#[clippy(warn::or_fun_call)]
I've added a qemu test and as it showed, some more places had to be changed to support 128 bitfields. I've also added ascii hint support for bitfields :) |
@@ -1608,6 +1621,31 @@ mod tests { | |||
); | |||
} | |||
|
|||
#[test] | |||
fn bitfields_u128() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a bit surprised about this test because expected that {0=119..124:b}
is encoded as 2 bytes instead of as 16 bytes. After checking locally, this test also works if I remove the last 14 bytes. It would be nice to make decode_and_expect
check that the input data was fully consumed to ensure we are not passing extra bytes that will be ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. This looks good!
bors r+ |
Build failed: |
CI should be fixed now |
Build succeeded: |
Closes #314
Also lift 32 bit range for bitfields introduced in #13
Could be split into another PR, if concerns are raised.