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
codegen: signed bitfield handling #2046
Conversation
Signed-off-by: chenhengqi <chenhengqi@outlook.com>
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, this looks on the right track. This changes a bunch of test expectations though, so those need to be updated, let me know if you need help with that.
#offset, | ||
#width | ||
); | ||
if (#is_signed) { |
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.
No braces, please.
Instead of having this branch in the generated code, we should instead change the generated code depending on whether the signed field is signed.
@@ -1632,6 +1632,7 @@ impl<'a> FieldCodegen<'a> for Bitfield { | |||
} | |||
}; | |||
|
|||
let is_signed = bitfield_ty.as_integer().unwrap().is_signed(); |
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.
C++ allows you to have enum bitfields, does that still work with this patch?
|
||
#[inline] | ||
pub fn signed_val(val: u64, bits: usize) -> i64 { | ||
if bits == 64 { |
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.
Instead of having this here, let's have it as a static function in the type, to minimize the chances of naming conflicts (so put it inside the impl
block, and call it like Self::signed_val(..)
instead.
Thanks. Will get back to here when I resolve all the problems. |
☔ The latest upstream changes (presumably #2284) made this pull request unmergeable. Please resolve the merge conflicts. |
This PR aims to fix #1160
Now we have the correct behavior:
Signed-off-by: Hengqi Chen chenhengqi@outlook.com