Skip to content

clang::Cursor::args should return an Option<Vec<Cursor>> #130

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

Closed
fitzgen opened this issue Oct 25, 2016 · 3 comments · Fixed by #207
Closed

clang::Cursor::args should return an Option<Vec<Cursor>> #130

fitzgen opened this issue Oct 25, 2016 · 3 comments · Fixed by #207

Comments

@fitzgen
Copy link
Member

fitzgen commented Oct 25, 2016

Right now it always returns a Vec<Cursor> but it makes FFI calls to clang_Cursor_getArgument which are only valid if the cursor's referent is a function or method declarations and calls:

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

The argument cursor can be determined for calls as well as for declarations of functions or methods. For other cursors and for invalid indices, an invalid cursor is returned.

We should check whether the cursor referent is a valid kind, and if it is not, we should return None. This will be one less foot gun.

I can mentor whoever would like to pick up this bug.

@highfive
Copy link

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

@Natim
Copy link

Natim commented Nov 4, 2016

I will try to give it a try with @glasserc

Natim pushed a commit to Natim/rust-bindgen that referenced this issue Nov 4, 2016
@fitzgen
Copy link
Member Author

fitzgen commented Nov 4, 2016

Great!

Natim pushed a commit to Natim/rust-bindgen that referenced this issue Nov 17, 2016
bors-servo pushed a commit that referenced this issue Nov 18, 2016
clang::Cursor::args should return an Option<Vec<Cursor>>

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

Successfully merging a pull request may close this issue.

3 participants