Skip to content

clang::Type::ret_type should return Option<Type> #141

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 25, 2016 · 2 comments
Closed

clang::Type::ret_type should return Option<Type> #141

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

Comments

@fitzgen
Copy link
Member

fitzgen commented Oct 25, 2016

ret_type does an FFI call to clang_getReturnType, which returns an invalid type if the input type is not a function type. We should check for the invalid type result, and return None, and otherwise return Some.

This will save callers from needing to remember to do this check, which is one less foot gun if they forget.

I can mentor whoever would like to work on this.
@highfive
Copy link

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

@jeanphilippeD
Copy link
Contributor

Hello,
I'd like to try an address this issue. I should have a PR ready soon.
I assumed you meant 'clang_getResultType'.

I'm not completely sure about the kind of error handling we want for one of the caller, but other similar PR have used expect, so I'll default to that for now.
Thanks,
JP

jeanphilippeD added a commit to jeanphilippeD/rust-bindgen that referenced this issue Oct 29, 2016
@jeanphilippeD
Return Option<Type> in clang::Type::ret_type fix rust-lang#141
fb8cdd3
jeanphilippeD added a commit to jeanphilippeD/rust-bindgen that referenced this issue Oct 29, 2016
@jeanphilippeD
Return Option<Type> in clang::Type::ret_type fix rust-lang#141
d98f19f
@jeanphilippeD jeanphilippeD mentioned this issue Oct 29, 2016
Merged
jeanphilippeD added a commit to jeanphilippeD/rust-bindgen that referenced this issue Oct 29, 2016
@jeanphilippeD
Return Option<Type> in clang::Type::ret_type fix rust-lang#141
a7d5f5d 
Also reduce the scope for unsafe code
jeanphilippeD added a commit to jeanphilippeD/rust-bindgen that referenced this issue Oct 29, 2016
@jeanphilippeD
Return Option<Type> in clang::Type::ret_type fix rust-lang#141
17494de 
Also reduce the scope for unsafe code
bors-servo pushed a commit that referenced this issue Oct 30, 2016
Auto merge of #165 - jeanphilippeD:issue141, r=emilio
604965a 
Return Option<Type> in clang::Type::ret_type fix #141

Fixes #141
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

No branches or pull requests
4 participants
@fitzgen @emilio @jeanphilippeD @highfive