Skip to content
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

clang::Cursor::enum_value_signed should return Option<i64> #127

Closed
fitzgen opened this issue Oct 25, 2016 · 4 comments
Closed

clang::Cursor::enum_value_signed should return Option<i64> #127

fitzgen opened this issue Oct 25, 2016 · 4 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Oct 25, 2016

Right now it always returns i64, but the FFI call it makes says

http://clang.llvm.org/doxygen/group__CINDEX__TYPES.html#ga6b8585818420e7512feb4c9d209b4f4d

Retrieve the integer value of an enum constant declaration as a signed long long.

If the cursor does not reference an enum constant declaration, LLONG_MIN is returned. Since this is also potentially a valid constant value, the kind of the cursor must be verified before calling this function.

We shouldn't force callers to check the kind of the cursor, we should do the check ourselves in this function and return an option. This would result in one less foot gun.

I can mentor anyone who'd like to pick up this bug.

@highfive
Copy link

Please make a comment here if you intend to work on this issue. Thank you!

@jcdyer
Copy link

jcdyer commented Nov 6, 2016

I'll take on this one.

@jcdyer
Copy link

jcdyer commented Nov 6, 2016

I'm assuming enum_value_signed is the same thing as enum_val_signed.

jcdyer pushed a commit to jcdyer/rust-bindgen that referenced this issue Nov 6, 2016
Also reorganize calling function to avoid duplicate checking of cursor
type.

Fixes rust-lang#127
@jcdyer
Copy link

jcdyer commented Nov 6, 2016

I took a stab at reorganizing the caller to make more fluent use of the Option, which should make things more straightforward when #128 lands.

jcdyer pushed a commit to jcdyer/rust-bindgen that referenced this issue Nov 6, 2016
Also reorganize calling function to avoid duplicate checking of cursor
type.

Fixes rust-lang#127
bors-servo pushed a commit that referenced this issue Nov 6, 2016
Wrap enum_val_signed in an Option.

Also reorganize calling code to avoid duplicate checking of cursor type.

Fixes #127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants