-
Notifications
You must be signed in to change notification settings - Fork 748
ir: Avoid generating out-of-range values in constants. #276
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
Conversation
@@ -9,6 +9,9 @@ pub const bar: _bindgen_ty_1 = _bindgen_ty_1::bar; | |||
#[repr(u32)] | |||
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] | |||
pub enum _bindgen_ty_1 { foo = 4, bar = 8, } | |||
pub type EasyToOverflow = ::std::os::raw::c_ulonglong; | |||
pub const k: EasyToOverflow = 2147483648; | |||
pub const k_expr: EasyToOverflow = 0; |
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.
This doesn't seem to be desirable. Build would succeed, but existence of this may lead to weird bug. This should either be removed, or be linked.
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.
We're getting zero from clang in this case unfortunately, so there's little I can do :(
if val.is_none() || !kind.signedness_matches(val.unwrap()) { | ||
let tu = ctx.translation_unit(); | ||
val = get_integer_literal_from_cursor(&cursor, tu); | ||
} |
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 slightly preferred all this with the slightly more concise method chaining, but either way works.
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.
Yeah, it was a bit trickier to write it that way, because we may be overriding a Some(value out of range)
with None
, so I preferred to be more explicit.
@bors-servo r=fitzgen |
📌 Commit 06e5f6c has been approved by |
⚡ Test exempted - status |
This completely disables translation of C enums to Rust enums, restoring to the old behaviour of translating them to integer constants.
Introduce -no-rust-enums (fixes rust-lang#276)
Partially addresses #274 until my patch or a similar one gets accepted and we can use it.
r? @fitzgen or @upsuper