Skip to content

Commit

Permalink
Rework the way that argument types and names are found from function …
Browse files Browse the repository at this point in the history
…signatures.

This fixes the issue seen in #1500 where function
pointers as a return type have their parameters incorrectly generated.

This also fixes a broken test case, objc_property_fnptr, as its types
are now generated correctly.
  • Loading branch information
flowbish authored and emilio committed Feb 3, 2019
1 parent fa457f2 commit 1092778
Show file tree
Hide file tree
Showing 9 changed files with 128 additions and 40 deletions.
53 changes: 36 additions & 17 deletions src/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,33 +550,27 @@ impl Cursor {

/// Given that this cursor's referent is a function, return cursors to its
/// parameters.
///
/// Returns None if the cursor's referent is not a function/method call or
/// declaration.
pub fn args(&self) -> Option<Vec<Cursor>> {
// XXX: We might want to use and keep num_args
// match self.kind() {
// CXCursor_FunctionDecl |
// CXCursor_CXXMethod => {
unsafe {
let w = clang_Cursor_getNumArguments(self.x);
if w == -1 {
None
} else {
let num = w as u32;

let mut args = vec![];
for i in 0..num {
args.push(Cursor {
x: clang_Cursor_getArgument(self.x, i as c_uint),
});
self.num_args().ok().map(|num| {
(0..num).map(|i| {
Cursor {
x: unsafe { clang_Cursor_getArgument(self.x, i as c_uint) },
}
Some(args)
}
}
})
.collect()
})
}

/// Given that this cursor's referent is a function/method call or
/// declaration, return the number of arguments it takes.
///
/// Returns -1 if the cursor's referent is not a function/method call or
/// Returns Err if the cursor's referent is not a function/method call or
/// declaration.
pub fn num_args(&self) -> Result<u32, ()> {
unsafe {
Expand Down Expand Up @@ -1017,6 +1011,31 @@ impl Type {
})
}

/// Given that this type is a function prototype, return the types of its parameters.
///
/// Returns None if the type is not a function prototype.
pub fn args(&self) -> Option<Vec<Type>> {
self.num_args().ok().map(|num| {
(0..num).map(|i| {
Type {
x: unsafe { clang_getArgType(self.x, i as c_uint) },
}
})
.collect()
})
}

/// Given that this type is a function prototype, return the number of arguments it takes.
///
/// Returns Err if the type is not a function prototype.
pub fn num_args(&self) -> Result<u32, ()> {
unsafe {
let w = clang_getNumArgTypes(self.x);
if w == -1 { Err(()) } else { Ok(w as u32) }
}
}


/// Given that this type is a pointer type, return the type that it points
/// to.
pub fn pointee_type(&self) -> Option<Type> {
Expand Down
45 changes: 28 additions & 17 deletions src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,31 @@ pub fn cursor_mangling(
Some(mangling)
}

fn args_from_ty_and_cursor(
ty: &clang::Type,
cursor: &clang::Cursor,
ctx: &mut BindgenContext,
) -> Vec<(Option<String>, TypeId)> {
match (cursor.args(), ty.args()) {
(Some(cursor_args), Some(ty_args)) => {
ty_args.iter().enumerate().map(|(i, ty)| {
let name = cursor_args.get(i)
.map(|c| c.spelling())
.and_then(|name| if name.is_empty() { None } else { Some(name) });
(name, Item::from_ty_or_ref(*ty, *cursor, None, ctx))
}).collect()
}
(Some(cursor_args), None) => {
cursor_args.iter().map(|cursor| {
let name = cursor.spelling();
let name = if name.is_empty() { None } else { Some(name) };
(name, Item::from_ty_or_ref(cursor.cur_type(), *cursor, None, ctx))
}).collect()
}
_ => panic!()
}
}

impl FunctionSig {
/// Construct a new function signature.
pub fn new(
Expand Down Expand Up @@ -363,28 +388,14 @@ impl FunctionSig {
ty.declaration()
};

let mut args: Vec<_> = match kind {
let mut args = match kind {
CXCursor_FunctionDecl |
CXCursor_Constructor |
CXCursor_CXXMethod |
CXCursor_ObjCInstanceMethodDecl |
CXCursor_ObjCClassMethodDecl => {
// For CXCursor_FunctionDecl, cursor.args() is the reliable way
// to get parameter names and types.
cursor
.args()
.unwrap()
.iter()
.map(|arg| {
let arg_ty = arg.cur_type();
let name = arg.spelling();
let name =
if name.is_empty() { None } else { Some(name) };
let ty = Item::from_ty_or_ref(arg_ty, *arg, None, ctx);
(name, ty)
})
.collect()
}
args_from_ty_and_cursor(&ty, &cursor, ctx)
},
_ => {
// For non-CXCursor_FunctionDecl, visiting the cursor's children
// is the only reliable way to get parameter names.
Expand Down
1 change: 1 addition & 0 deletions src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,7 @@ impl ClangItemParser for Item {
let is_const = ty.is_const();
let kind = TypeKind::UnresolvedTypeRef(ty, location, parent_id);
let current_module = ctx.current_module();

ctx.add_item(
Item::new(
potential_id,
Expand Down
16 changes: 16 additions & 0 deletions tests/expectations/tests/block_return_type.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* automatically generated by rust-bindgen */

#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]
#![cfg(target_os = "macos")]

extern crate block;
extern "C" {
pub fn func() -> _bindgen_ty_id_4;
}
pub type _bindgen_ty_id_4 =
*const ::block::Block<(::std::os::raw::c_int, ::std::os::raw::c_int), ::std::os::raw::c_int>;
18 changes: 18 additions & 0 deletions tests/expectations/tests/func_ptr_return_type.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* automatically generated by rust-bindgen */

#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]

extern "C" {
pub fn func() -> ::std::option::Option<
unsafe extern "C" fn(
arg1: ::std::os::raw::c_int,
arg2: ::std::os::raw::c_int,
) -> ::std::os::raw::c_int,
>;
}

28 changes: 24 additions & 4 deletions tests/expectations/tests/objc_property_fnptr.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,42 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]
#![cfg(target_os = "macos")]

#[macro_use]
extern crate objc;
#[allow(non_camel_case_types)]
pub type id = *mut objc::runtime::Object;
pub trait Foo {
unsafe fn func(self) -> ::std::option::Option<unsafe extern "C" fn() -> ::std::os::raw::c_int>;
unsafe fn func(
self,
) -> ::std::option::Option<
unsafe extern "C" fn(
arg1: ::std::os::raw::c_char,
arg2: ::std::os::raw::c_short,
arg3: f32,
) -> ::std::os::raw::c_int,
>;
unsafe fn setFunc_(
self,
func: ::std::option::Option<unsafe extern "C" fn() -> ::std::os::raw::c_int>,
);
}
impl Foo for id {
unsafe fn func(self) -> ::std::option::Option<unsafe extern "C" fn() -> ::std::os::raw::c_int> {
unsafe fn func(
self,
) -> ::std::option::Option<
unsafe extern "C" fn(
arg1: ::std::os::raw::c_char,
arg2: ::std::os::raw::c_short,
arg3: f32,
) -> ::std::os::raw::c_int,
> {
msg_send!(self, func)
}
unsafe fn setFunc_(
Expand Down
4 changes: 4 additions & 0 deletions tests/headers/block_return_type.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// bindgen-flags: --generate-block --block-extern-crate -- -fblocks
// bindgen-osx-only

int (^func(void))(int, int);
1 change: 1 addition & 0 deletions tests/headers/func_ptr_return_type.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int (*func(void))(int, int);
2 changes: 0 additions & 2 deletions tests/headers/objc_property_fnptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,5 @@
// bindgen-osx-only

@interface Foo
// FIXME: We are not generating valid code for this
// but at least we should not die
@property int (*func)(char, short, float);
@end

0 comments on commit 1092778

Please sign in to comment.