From f922f68531fa44708af7779ffbb4b118d6e189ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Jan 2021 21:33:29 +0100 Subject: [PATCH] ir: Avoid generating bogus pointer arguments. Now that we track nullability in the pointer type it's easy to do it. Fixes #223 --- src/bindgen/cdecl.rs | 6 +- src/bindgen/ir/ty.rs | 78 ++++++++++++++----- src/bindgen/mangle.rs | 19 ++++- tests/expectations/simplify_option_ptr.both.c | 12 ++- .../simplify_option_ptr.both.compat.c | 12 ++- tests/expectations/simplify_option_ptr.c | 8 +- .../expectations/simplify_option_ptr.compat.c | 8 +- tests/expectations/simplify_option_ptr.cpp | 7 +- tests/expectations/simplify_option_ptr.pyx | 10 ++- tests/expectations/simplify_option_ptr.tag.c | 12 ++- .../simplify_option_ptr.tag.compat.c | 12 ++- .../expectations/simplify_option_ptr.tag.pyx | 10 ++- tests/rust/simplify_option_ptr.rs | 5 +- 13 files changed, 169 insertions(+), 30 deletions(-) diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs index b915d9d..8075f97 100644 --- a/src/bindgen/cdecl.rs +++ b/src/bindgen/cdecl.rs @@ -158,7 +158,11 @@ impl CDecl { self.declarators.push(CDeclarator::Array(len)); self.build_type(t, is_const, config); } - Type::FuncPtr(ref ret, ref args) => { + Type::FuncPtr { + ref ret, + ref args, + is_nullable: _, + } => { let args = args .iter() .map(|(ref name, ref ty)| (name.clone(), CDecl::from_type(ty, config))) diff --git a/src/bindgen/ir/ty.rs b/src/bindgen/ir/ty.rs index 8232330..a66f9f9 100644 --- a/src/bindgen/ir/ty.rs +++ b/src/bindgen/ir/ty.rs @@ -343,7 +343,11 @@ pub enum Type { Path(GenericPath), Primitive(PrimitiveType), Array(Box, ArrayLength), - FuncPtr(Box, Vec<(Option, Type)>), + FuncPtr { + ret: Box, + args: Vec<(Option, Type)>, + is_nullable: bool, + }, } impl Type { @@ -484,7 +488,11 @@ impl Type { } }; - Type::FuncPtr(Box::new(ret), args) + Type::FuncPtr { + ret: Box::new(ret), + args, + is_nullable: false, + } } syn::Type::Tuple(ref tuple) => { if tuple.elems.is_empty() { @@ -515,14 +523,22 @@ impl Type { ref ty, is_const, is_ref, - .. + is_nullable: false, } => Some(Type::Ptr { ty: ty.clone(), is_const, is_ref, is_nullable: true, }), - Type::FuncPtr(ref x, ref y) => Some(Type::FuncPtr(x.clone(), y.clone())), + Type::FuncPtr { + ref ret, + ref args, + is_nullable: false, + } => Some(Type::FuncPtr { + ret: ret.clone(), + args: args.clone(), + is_nullable: true, + }), _ => None, } } @@ -543,7 +559,6 @@ impl Type { None => Cow::Borrowed(unsimplified_generic), }; match path.name() { - // FIXME(#223): This is not quite right. "Option" => generic.make_nullable(), "NonNull" => Some(Type::Ptr { ty: Box::new(generic.into_owned()), @@ -580,7 +595,11 @@ impl Type { generic_path.replace_self_with(self_ty); } Type::Primitive(..) => {} - Type::FuncPtr(ref mut ret, ref mut args) => { + Type::FuncPtr { + ref mut ret, + ref mut args, + .. + } => { ret.replace_self_with(self_ty); for arg in args { arg.1.replace_self_with(self_ty); @@ -603,7 +622,7 @@ impl Type { Type::Array(..) => { return None; } - Type::FuncPtr(..) => { + Type::FuncPtr { .. } => { return None; } }; @@ -644,13 +663,19 @@ impl Type { Type::Array(ref ty, ref constant) => { Type::Array(Box::new(ty.specialize(mappings)), constant.clone()) } - Type::FuncPtr(ref ret, ref args) => Type::FuncPtr( - Box::new(ret.specialize(mappings)), - args.iter() + Type::FuncPtr { + ref ret, + ref args, + is_nullable, + } => Type::FuncPtr { + ret: Box::new(ret.specialize(mappings)), + args: args + .iter() .cloned() .map(|(name, ty)| (name, ty.specialize(mappings))) .collect(), - ), + is_nullable, + }, } } @@ -694,7 +719,9 @@ impl Type { Type::Array(ref ty, _) => { ty.add_dependencies_ignoring_generics(generic_params, library, out); } - Type::FuncPtr(ref ret, ref args) => { + Type::FuncPtr { + ref ret, ref args, .. + } => { ret.add_dependencies_ignoring_generics(generic_params, library, out); for (_, ref arg) in args { arg.add_dependencies_ignoring_generics(generic_params, library, out); @@ -728,7 +755,9 @@ impl Type { Type::Array(ref ty, _) => { ty.add_monomorphs(library, out); } - Type::FuncPtr(ref ret, ref args) => { + Type::FuncPtr { + ref ret, ref args, .. + } => { ret.add_monomorphs(library, out); for (_, ref arg) in args { arg.add_monomorphs(library, out); @@ -750,7 +779,11 @@ impl Type { ty.rename_for_config(config, generic_params); len.rename_for_config(config); } - Type::FuncPtr(ref mut ret, ref mut args) => { + Type::FuncPtr { + ref mut ret, + ref mut args, + .. + } => { ret.rename_for_config(config, generic_params); for (_, arg) in args { arg.rename_for_config(config, generic_params); @@ -771,7 +804,11 @@ impl Type { Type::Array(ref mut ty, _) => { ty.resolve_declaration_types(resolver); } - Type::FuncPtr(ref mut ret, ref mut args) => { + Type::FuncPtr { + ref mut ret, + ref mut args, + .. + } => { ret.resolve_declaration_types(resolver); for (_, ref mut arg) in args { arg.resolve_declaration_types(resolver); @@ -804,7 +841,11 @@ impl Type { Type::Array(ref mut ty, _) => { ty.mangle_paths(monomorphs); } - Type::FuncPtr(ref mut ret, ref mut args) => { + Type::FuncPtr { + ref mut ret, + ref mut args, + .. + } => { ret.mangle_paths(monomorphs); for (_, ref mut arg) in args { arg.mangle_paths(monomorphs); @@ -815,11 +856,12 @@ impl Type { pub fn can_cmp_order(&self) -> bool { match *self { + // FIXME: Shouldn't this look at ty.can_cmp_order() as well? Type::Ptr { is_ref, .. } => !is_ref, Type::Path(..) => true, Type::Primitive(ref p) => p.can_cmp_order(), Type::Array(..) => false, - Type::FuncPtr(..) => false, + Type::FuncPtr { .. } => false, } } @@ -829,7 +871,7 @@ impl Type { Type::Path(..) => true, Type::Primitive(ref p) => p.can_cmp_eq(), Type::Array(..) => false, - Type::FuncPtr(..) => true, + Type::FuncPtr { .. } => true, } } } diff --git a/src/bindgen/mangle.rs b/src/bindgen/mangle.rs index 0142671..909b465 100644 --- a/src/bindgen/mangle.rs +++ b/src/bindgen/mangle.rs @@ -20,6 +20,9 @@ enum Separator { ClosingAngleBracket, BeginMutPtr, BeginConstPtr, + BeginFn, + BetweenFnArg, + EndFn, } struct Mangler<'a> { @@ -93,7 +96,21 @@ impl<'a> Mangler<'a> { }); self.append_mangled_type(&**ty, last); } - Type::Array(..) | Type::FuncPtr(..) => { + Type::FuncPtr { + ref ret, ref args, .. + } => { + self.push(Separator::BeginFn); + self.append_mangled_type(&**ret, args.is_empty()); + for (i, arg) in args.iter().enumerate() { + self.push(Separator::BetweenFnArg); + let last = last && i == args.len() - 1; + self.append_mangled_type(&arg.1, last); + } + if !self.last { + self.push(Separator::EndFn); + } + } + Type::Array(..) => { unimplemented!( "Unable to mangle generic parameter {:?} for '{}'", ty, diff --git a/tests/expectations/simplify_option_ptr.both.c b/tests/expectations/simplify_option_ptr.both.c index 402e5fe..1efe0f8 100644 --- a/tests/expectations/simplify_option_ptr.both.c +++ b/tests/expectations/simplify_option_ptr.both.c @@ -5,16 +5,26 @@ typedef struct Opaque Opaque; +typedef struct Option_____Opaque Option_____Opaque; + +typedef struct Option_______c_void Option_______c_void; + typedef struct Foo { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; } Foo; typedef union Bar { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; } Bar; -void root(const struct Opaque *a, struct Opaque *b, struct Foo c, union Bar d); +void root(const struct Opaque *a, + struct Opaque *b, + struct Foo c, + union Bar d, + struct Option_____Opaque *e); diff --git a/tests/expectations/simplify_option_ptr.both.compat.c b/tests/expectations/simplify_option_ptr.both.compat.c index d03e4bc..3ad00d1 100644 --- a/tests/expectations/simplify_option_ptr.both.compat.c +++ b/tests/expectations/simplify_option_ptr.both.compat.c @@ -5,23 +5,33 @@ typedef struct Opaque Opaque; +typedef struct Option_____Opaque Option_____Opaque; + +typedef struct Option_______c_void Option_______c_void; + typedef struct Foo { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; } Foo; typedef union Bar { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; } Bar; #ifdef __cplusplus extern "C" { #endif // __cplusplus -void root(const struct Opaque *a, struct Opaque *b, struct Foo c, union Bar d); +void root(const struct Opaque *a, + struct Opaque *b, + struct Foo c, + union Bar d, + struct Option_____Opaque *e); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/simplify_option_ptr.c b/tests/expectations/simplify_option_ptr.c index 87b5ce9..42d4328 100644 --- a/tests/expectations/simplify_option_ptr.c +++ b/tests/expectations/simplify_option_ptr.c @@ -5,16 +5,22 @@ typedef struct Opaque Opaque; +typedef struct Option_____Opaque Option_____Opaque; + +typedef struct Option_______c_void Option_______c_void; + typedef struct { const Opaque *x; Opaque *y; void (*z)(void); + Option_______c_void *zz; } Foo; typedef union { const Opaque *x; Opaque *y; void (*z)(void); + Option_______c_void *zz; } Bar; -void root(const Opaque *a, Opaque *b, Foo c, Bar d); +void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option_____Opaque *e); diff --git a/tests/expectations/simplify_option_ptr.compat.c b/tests/expectations/simplify_option_ptr.compat.c index f3761ec..7c523b5 100644 --- a/tests/expectations/simplify_option_ptr.compat.c +++ b/tests/expectations/simplify_option_ptr.compat.c @@ -5,23 +5,29 @@ typedef struct Opaque Opaque; +typedef struct Option_____Opaque Option_____Opaque; + +typedef struct Option_______c_void Option_______c_void; + typedef struct { const Opaque *x; Opaque *y; void (*z)(void); + Option_______c_void *zz; } Foo; typedef union { const Opaque *x; Opaque *y; void (*z)(void); + Option_______c_void *zz; } Bar; #ifdef __cplusplus extern "C" { #endif // __cplusplus -void root(const Opaque *a, Opaque *b, Foo c, Bar d); +void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option_____Opaque *e); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/simplify_option_ptr.cpp b/tests/expectations/simplify_option_ptr.cpp index 5ff7f22..5b45381 100644 --- a/tests/expectations/simplify_option_ptr.cpp +++ b/tests/expectations/simplify_option_ptr.cpp @@ -6,20 +6,25 @@ struct Opaque; +template +struct Option; + struct Foo { const Opaque *x; Opaque *y; void (*z)(); + Option *zz; }; union Bar { const Opaque *x; Opaque *y; void (*z)(); + Option *zz; }; extern "C" { -void root(const Opaque *a, Opaque *b, Foo c, Bar d); +void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option *e); } // extern "C" diff --git a/tests/expectations/simplify_option_ptr.pyx b/tests/expectations/simplify_option_ptr.pyx index f2f6892..7496142 100644 --- a/tests/expectations/simplify_option_ptr.pyx +++ b/tests/expectations/simplify_option_ptr.pyx @@ -9,14 +9,22 @@ cdef extern from *: ctypedef struct Opaque: pass + ctypedef struct Option_____Opaque: + pass + + ctypedef struct Option_______c_void: + pass + ctypedef struct Foo: const Opaque *x; Opaque *y; void (*z)(); + Option_______c_void *zz; ctypedef union Bar: const Opaque *x; Opaque *y; void (*z)(); + Option_______c_void *zz; - void root(const Opaque *a, Opaque *b, Foo c, Bar d); + void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option_____Opaque *e); diff --git a/tests/expectations/simplify_option_ptr.tag.c b/tests/expectations/simplify_option_ptr.tag.c index 0eb33b1..ce28659 100644 --- a/tests/expectations/simplify_option_ptr.tag.c +++ b/tests/expectations/simplify_option_ptr.tag.c @@ -5,16 +5,26 @@ struct Opaque; +struct Option_____Opaque; + +struct Option_______c_void; + struct Foo { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; }; union Bar { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; }; -void root(const struct Opaque *a, struct Opaque *b, struct Foo c, union Bar d); +void root(const struct Opaque *a, + struct Opaque *b, + struct Foo c, + union Bar d, + struct Option_____Opaque *e); diff --git a/tests/expectations/simplify_option_ptr.tag.compat.c b/tests/expectations/simplify_option_ptr.tag.compat.c index 752a99d..9288244 100644 --- a/tests/expectations/simplify_option_ptr.tag.compat.c +++ b/tests/expectations/simplify_option_ptr.tag.compat.c @@ -5,23 +5,33 @@ struct Opaque; +struct Option_____Opaque; + +struct Option_______c_void; + struct Foo { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; }; union Bar { const struct Opaque *x; struct Opaque *y; void (*z)(void); + struct Option_______c_void *zz; }; #ifdef __cplusplus extern "C" { #endif // __cplusplus -void root(const struct Opaque *a, struct Opaque *b, struct Foo c, union Bar d); +void root(const struct Opaque *a, + struct Opaque *b, + struct Foo c, + union Bar d, + struct Option_____Opaque *e); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/simplify_option_ptr.tag.pyx b/tests/expectations/simplify_option_ptr.tag.pyx index 72890fb..66cb440 100644 --- a/tests/expectations/simplify_option_ptr.tag.pyx +++ b/tests/expectations/simplify_option_ptr.tag.pyx @@ -9,14 +9,22 @@ cdef extern from *: cdef struct Opaque: pass + cdef struct Option_____Opaque: + pass + + cdef struct Option_______c_void: + pass + cdef struct Foo: const Opaque *x; Opaque *y; void (*z)(); + Option_______c_void *zz; cdef union Bar: const Opaque *x; Opaque *y; void (*z)(); + Option_______c_void *zz; - void root(const Opaque *a, Opaque *b, Foo c, Bar d); + void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option_____Opaque *e); diff --git a/tests/rust/simplify_option_ptr.rs b/tests/rust/simplify_option_ptr.rs index b6e3f5b..d43be3e 100644 --- a/tests/rust/simplify_option_ptr.rs +++ b/tests/rust/simplify_option_ptr.rs @@ -6,6 +6,7 @@ struct Foo { x: Option<&Opaque>, y: Option<&mut Opaque>, z: Option ()>, + zz: *mut Option ()>, } #[repr(C)] @@ -13,6 +14,7 @@ union Bar { x: Option<&Opaque>, y: Option<&mut Opaque>, z: Option ()>, + zz: *mut Option ()>, } #[no_mangle] @@ -20,5 +22,6 @@ pub extern "C" fn root( a: Option<&Opaque>, b: Option<&mut Opaque>, c: Foo, - d: Bar + d: Bar, + e: *mut Option<*mut Opaque>, ) { }