Skip to content

clang::Cursor::semantic_parent should call clang::Cursor::fallible_semantic_parent -- not the other way around #120

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
Closed
fitzgen opened this issue Oct 24, 2016 · 3 comments · Fixed by #172
Closed

clang::Cursor::semantic_parent should call clang::Cursor::fallible_semantic_parent -- not the other way around #120

fitzgen opened this issue Oct 24, 2016 · 3 comments · Fixed by #172
Labels
C-assigned E-easy I-cleanup

Comments

@fitzgen
Copy link
Member

fitzgen commented Oct 24, 2016

Right now, clang::Cursor::fallible_semantic_parent calls clang::Cursor::semantic_parent and does some validation and error checking, and it is clang::Cursor::semantic_parent that does the ffi call to clang_getCursorSemanticParent. This means that clang::Cursor::semantic_parent isn't very safe, since the result was never validated. We should make fallible_semantic_parent do the ffi call and validation, and then have semantic_parent call fallible_semantic_parent().unwrap() so that we are properly verifying return values regardless what method we call.

I can mentor someone who 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!

@malisas
Copy link
Contributor

malisas commented Oct 26, 2016

I'll work on this :)

@fitzgen
Copy link
Member Author

fitzgen commented Oct 26, 2016

Great! Let me know if you have run into any trouble building or running tests or anything like that!

@malisas malisas mentioned this issue Oct 30, 2016
Merged
bors-servo pushed a commit that referenced this issue Nov 7, 2016
Auto merge of #172 - malisas:malisa-semantic-parent-call, r=fitzgen,e…
86c3bba 
…milio

Make clang::Cursor::fallible_semantic_parent make ffi call

This PR fixes #120 . `clang::Cursor::semantic_parent` now just calls `clang::Cursor::fallible_semantic_parent`, and the ffi call has been moved into `fallible_semantic_parent`.

This change broke a number of tests which call `is_toplevel()` (which itself calls `semantic_parent()`):
> panicked at called Option::unwrap() on a None value

So I re-wrote `is_toplevel()` to call `fallible_semantic_parent()`, which returns an `Option<Cursor>` type instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-assigned E-easy I-cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make clang::Cursor::fallible_semantic_parent make ffi call malisas/rust-bindgen
3 participants
@fitzgen @highfive @malisas