-
Notifications
You must be signed in to change notification settings - Fork 698
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
Fix issue #1535: use correct names for function parameters in function pointer arguments. #1536
Conversation
src/ir/function.rs
Outdated
_ => panic!() | ||
} | ||
let cursor_args = cursor.args().unwrap().into_iter(); | ||
let type_args = ty.args().unwrap_or_else(Vec::new).into_iter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unwrap_or_default
?
.map(Some) | ||
.chain(std::iter::repeat(None)) | ||
) | ||
.take_while(|(cur, ty)| cur.is_some() || ty.is_some()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a note that we could use itertools::zip_longest
(https://docs.rs/itertools/0.7.8/itertools/trait.Itertools.html#method.zip_longest) if we wanted to?
This is fine for now to avoid introducing a new dependency unnecessarily.
@@ -1 +1 @@ | |||
void foo(void (*bar)()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep both functions? Having more tests doesn't really hurt :)
Thanks a lot for working on this @flowbish! |
Updated the branch, thanks for taking a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Issue #1535 showed that parameter names were incorrect in function pointers arguments.
This was caused by the incorrect cursor being passed through in determining argument types and names -- the cursor was recycled the function declaration, causing bindgen to recycle the parameter names.
This patch fixes it by using the correct cursor when constructing the argument types.
Argument types can be found in either the cursor or the type, but argument names may only be
found on the cursor. We often have access to both a type and a cursor for each argument, but
in some cases we may only have one.
Prefer using the type as the source of truth for the argument's type, but fall back to
inspecting the cursor (this happens for Objective C interfaces).
Prefer using the cursor for the argument's type, but fall back to using the parent's cursor
(this happens for function pointer return types).