From 109277892a9b336fd3fe297236ba463c81ebff36 Mon Sep 17 00:00:00 2001 From: Porter Smith Date: Fri, 1 Feb 2019 17:59:59 +0000 Subject: [PATCH] Rework the way that argument types and names are found from function 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. --- src/clang.rs | 53 +++++++++++++------ src/ir/function.rs | 45 ++++++++++------ src/ir/item.rs | 1 + tests/expectations/tests/block_return_type.rs | 16 ++++++ .../tests/func_ptr_return_type.rs | 18 +++++++ .../expectations/tests/objc_property_fnptr.rs | 28 ++++++++-- tests/headers/block_return_type.h | 4 ++ tests/headers/func_ptr_return_type.h | 1 + tests/headers/objc_property_fnptr.h | 2 - 9 files changed, 128 insertions(+), 40 deletions(-) create mode 100644 tests/expectations/tests/block_return_type.rs create mode 100644 tests/expectations/tests/func_ptr_return_type.rs create mode 100644 tests/headers/block_return_type.h create mode 100644 tests/headers/func_ptr_return_type.h diff --git a/src/clang.rs b/src/clang.rs index 8a3e3c688c..10bf054ae2 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -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> { - // 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 { unsafe { @@ -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> { + 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 { + 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 { diff --git a/src/ir/function.rs b/src/ir/function.rs index fae6e056aa..5ae7583d5e 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -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, 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( @@ -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. diff --git a/src/ir/item.rs b/src/ir/item.rs index a54cbb827d..eec64695c3 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -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, diff --git a/tests/expectations/tests/block_return_type.rs b/tests/expectations/tests/block_return_type.rs new file mode 100644 index 0000000000..7779540c93 --- /dev/null +++ b/tests/expectations/tests/block_return_type.rs @@ -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>; diff --git a/tests/expectations/tests/func_ptr_return_type.rs b/tests/expectations/tests/func_ptr_return_type.rs new file mode 100644 index 0000000000..3a2c90019b --- /dev/null +++ b/tests/expectations/tests/func_ptr_return_type.rs @@ -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, + >; +} + diff --git a/tests/expectations/tests/objc_property_fnptr.rs b/tests/expectations/tests/objc_property_fnptr.rs index 4f98f9fb0b..94ebfad640 100644 --- a/tests/expectations/tests/objc_property_fnptr.rs +++ b/tests/expectations/tests/objc_property_fnptr.rs @@ -1,7 +1,11 @@ /* 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] @@ -9,14 +13,30 @@ 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 ::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 ::std::os::raw::c_int>, ); } impl Foo for id { - unsafe fn func(self) -> ::std::option::Option ::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_( diff --git a/tests/headers/block_return_type.h b/tests/headers/block_return_type.h new file mode 100644 index 0000000000..be76cb5956 --- /dev/null +++ b/tests/headers/block_return_type.h @@ -0,0 +1,4 @@ +// bindgen-flags: --generate-block --block-extern-crate -- -fblocks +// bindgen-osx-only + +int (^func(void))(int, int); diff --git a/tests/headers/func_ptr_return_type.h b/tests/headers/func_ptr_return_type.h new file mode 100644 index 0000000000..d529edc770 --- /dev/null +++ b/tests/headers/func_ptr_return_type.h @@ -0,0 +1 @@ +int (*func(void))(int, int); diff --git a/tests/headers/objc_property_fnptr.h b/tests/headers/objc_property_fnptr.h index 8312ba4ae7..bac0c779a9 100644 --- a/tests/headers/objc_property_fnptr.h +++ b/tests/headers/objc_property_fnptr.h @@ -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