-
Notifications
You must be signed in to change notification settings - Fork 7
generate code using fqt for bool #134
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
base: main
Are you sure you want to change the base?
Conversation
525c7f7 to
e94ab4b
Compare
e94ab4b to
0fa4f36
Compare
0fa4f36 to
cf14e42
Compare
daxpedda
left a comment
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.
Missing a changelog entry, otherwise looks good.
I'm yet again surprised what we have to account for in a proc-macro ...
| #[repr(C)] | ||
| enum Test { | ||
| A = isize::MAX - 2, | ||
| A = (u32::MAX - 2) as isize, |
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.
Why this change?
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.
Because while we can use isize as a type, we are only allowed to use i32 values in future versions of rust with repr(C), I could probably simplify this by just using the value of i32::MAX directly
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 think we should stick with isize until we get there, we also use isize internally, so we should test for that.
(on a side note: I'm doubtful this change will actually happen because it would break too much)
| #[repr(C)] | ||
| enum Test { | ||
| A = -0x8000_0000_0000_0000_isize, | ||
| A = (-0x8000_0000_i32) as isize, |
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.
Same here.
closes #133