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.

Sign up for GitHub

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

Jump to bottom

clang::Cursor::num_args should return Option<u32> #132

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

clang::Cursor::num_args should return Option<u32> #132

fitzgen opened this issue Oct 25, 2016 · 2 comments
Labels
E-easy I-cleanup

Comments

@fitzgen
Copy link
Member

fitzgen commented Oct 25, 2016

Right now we just directly do an FFI call that returns -1 if the cursor's referent is not a function/method call or declaration. We should inspect the return value and return None if it is -1 or Some if it is >= 0. This would be one less foot gun where we require callers of this method to remember to check.

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

For other cursors -1 is returned.

I can mentor whoever picks up this bug.
@highfive
Copy link

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

@ghost
Copy link

ghost commented Oct 27, 2016

added PR #150

bors-servo pushed a commit that referenced this issue Oct 27, 2016
Auto merge of #150 - malfunc:master, r=emilio,fitzgen
72f435e 
fix: clang::Cursor::num_args should return Option<u32> #132

Hi, tried to fix the issue mentioned. Not sure, if changing fn args makes sense (the err message probably doesn't...). cargo test --features llvm_stable was succesful.
- changed output of fn num_args to option<u32>
- changed fn args to handle option<u32> instead of usize
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
E-easy I-cleanup
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fitzgen @KiChjang @highfive