Skip to content

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

Merged
merged 1 commit into from
Nov 17, 2016

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Nov 17, 2016

Partially addresses #274 until my patch or a similar one gets accepted and we can use it.

r? @fitzgen or @upsuper

@@ -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;
Copy link
Contributor

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.

Copy link
Contributor Author

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);
}
Copy link
Member

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.

Copy link
Contributor Author

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.

@emilio
Copy link
Contributor Author

emilio commented Nov 17, 2016

@bors-servo r=fitzgen

@bors-servo
Copy link

📌 Commit 06e5f6c has been approved by fitzgen

@bors-servo
Copy link

⚡ Test exempted - status

@bors-servo bors-servo merged commit 06e5f6c into rust-lang:master Nov 17, 2016
bors-servo pushed a commit that referenced this pull request Nov 17, 2016
ir: Avoid generating out-of-range values in constants.

Partially addresses #274 until my patch or a similar one gets accepted and we can use it.

r? @fitzgen or @upsuper
@emilio emilio deleted the vartype-fixes branch November 17, 2016 23:38
luser pushed a commit to luser/rust-bindgen that referenced this pull request Mar 27, 2017
This completely disables translation of C enums to Rust enums,
restoring to the old behaviour of translating them to integer constants.
luser pushed a commit to luser/rust-bindgen that referenced this pull request Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants