From 6fc7c77b0fcd57edae6c6ed11392b1d84ad10697 Mon Sep 17 00:00:00 2001 From: Nick Wilcox Date: Wed, 5 Aug 2020 18:58:26 +1000 Subject: [PATCH] remove duplicated code, correct test names, change config param --- docs.md | 11 +-- src/bindgen/cdecl.rs | 65 ++++++++------ src/bindgen/config.rs | 25 +++++- src/bindgen/ir/constant.rs | 4 +- src/bindgen/ir/function.rs | 10 ++- src/bindgen/ir/global.rs | 2 +- src/bindgen/ir/ty.rs | 145 ++++++++++++++---------------- src/bindgen/mangle.rs | 4 +- tests/rust/nonnull_attribute.rs | 8 +- tests/rust/nonnull_attribute.toml | 1 + 10 files changed, 148 insertions(+), 127 deletions(-) diff --git a/docs.md b/docs.md index a659791..b873080 100644 --- a/docs.md +++ b/docs.md @@ -476,11 +476,6 @@ tab_width = 3 # default: "auto" documentation_style = "doxy" -# An optional string that, if present, will decorate all pointers that are -# required to be non null. Nullability is inferred from the Rust type: `&T`, -# `&mut T` and `NonNull` are all require a valid pointer value. -non_null_attribute = "_Nonnull" - @@ -942,6 +937,12 @@ default_features = true # default: [] features = ["cbindgen"] +[ptr] +# An optional string to decorate all pointers that are +# required to be non null. Nullability is inferred from the Rust type: `&T`, +# `&mut T` and `NonNull` all require a valid pointer value. +non_null_attribute = "_Nonnull" + ``` diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs index 9f90e8c..33c5990 100644 --- a/src/bindgen/cdecl.rs +++ b/src/bindgen/cdecl.rs @@ -14,7 +14,7 @@ use crate::bindgen::{Config, Language}; // http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf enum CDeclarator { - Ptr(bool, bool), + Ptr { is_const: bool, is_nullable: bool }, Ref, Array(String), Func(Vec<(Option, CDecl)>, bool), @@ -23,7 +23,7 @@ enum CDeclarator { impl CDeclarator { fn is_ptr(&self) -> bool { match self { - CDeclarator::Ptr(..) | CDeclarator::Ref | CDeclarator::Func(..) => true, + CDeclarator::Ptr { .. } | CDeclarator::Ref | CDeclarator::Func(..) => true, _ => false, } } @@ -114,21 +114,25 @@ impl CDecl { self.type_name = p.to_string(); } - Type::ConstPtr(ref t) => { - self.declarators.push(CDeclarator::Ptr(is_const, false)); - self.build_type(t, true); + Type::ConstPtr { + ref ty, + is_nullable, + } => { + self.declarators.push(CDeclarator::Ptr { + is_const, + is_nullable: *is_nullable, + }); + self.build_type(ty, true); } - Type::Ptr(ref t) => { - self.declarators.push(CDeclarator::Ptr(is_const, false)); - self.build_type(t, false); - } - Type::NonNullPtr(ref t) => { - self.declarators.push(CDeclarator::Ptr(is_const, true)); - self.build_type(t, false); - } - Type::ConstNonNullPtr(ref t) => { - self.declarators.push(CDeclarator::Ptr(is_const, true)); - self.build_type(t, true); + Type::Ptr { + ref ty, + is_nullable, + } => { + self.declarators.push(CDeclarator::Ptr { + is_const, + is_nullable: *is_nullable, + }); + self.build_type(ty, false); } Type::Ref(ref t) => { self.declarators.push(CDeclarator::Ref); @@ -148,7 +152,10 @@ impl CDecl { .iter() .map(|(ref name, ref ty)| (name.clone(), CDecl::from_type(ty))) .collect(); - self.declarators.push(CDeclarator::Ptr(false, false)); + self.declarators.push(CDeclarator::Ptr { + is_const: false, + is_nullable: true, + }); self.declarators.push(CDeclarator::Func(args, false)); self.build_type(ret, false); } @@ -186,17 +193,19 @@ impl CDecl { let next_is_pointer = iter_rev.peek().map_or(false, |x| x.is_ptr()); match *declarator { - CDeclarator::Ptr(ref is_const, ref is_nonnull) => { - let non_null_attribute = if *is_nonnull { - config.non_null_attribute.as_ref() + CDeclarator::Ptr { + is_const, + is_nullable, + } => { + if is_const { + out.write("*const ") } else { - None - }; - match (non_null_attribute, is_const) { - (None, true) => out.write("*const "), - (None, false) => out.write("*"), - (Some(attr), true) => write!(out, "*const {} ", attr), - (Some(attr), false) => write!(out, "* {} ", attr), + out.write("*") + } + if !is_nullable { + if let Some(attr) = &config.pointer.non_null_attribute { + write!(out, "{} ", attr); + } } } CDeclarator::Ref => { @@ -227,7 +236,7 @@ impl CDecl { #[allow(clippy::while_let_on_iterator)] while let Some(declarator) = iter.next() { match *declarator { - CDeclarator::Ptr(..) => { + CDeclarator::Ptr { .. } => { last_was_pointer = true; } CDeclarator::Ref => { diff --git a/src/bindgen/config.rs b/src/bindgen/config.rs index 05a1eb3..0eb0fbc 100644 --- a/src/bindgen/config.rs +++ b/src/bindgen/config.rs @@ -692,6 +692,24 @@ impl ParseConfig { } } +/// Settings to apply to pointers +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "snake_case")] +#[serde(deny_unknown_fields)] +#[serde(default)] +pub struct PtrConfig { + /// Optional attribute to apply to pointers that are required to not be null + pub non_null_attribute: Option, +} + +impl Default for PtrConfig { + fn default() -> Self { + Self { + non_null_attribute: None, + } + } +} + /// A collection of settings to customize the generated bindings. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "snake_case")] @@ -765,8 +783,9 @@ pub struct Config { pub documentation: bool, /// How documentation comments should be styled. pub documentation_style: DocumentationStyle, - /// Optional attribute to apply to pointers that are required to not be null - pub non_null_attribute: Option, + /// Configuration options for pointers + #[serde(rename = "ptr")] + pub pointer: PtrConfig, } impl Default for Config { @@ -802,7 +821,7 @@ impl Default for Config { defines: HashMap::new(), documentation: true, documentation_style: DocumentationStyle::Auto, - non_null_attribute: None, + pointer: PtrConfig::default(), } } } diff --git a/src/bindgen/ir/constant.rs b/src/bindgen/ir/constant.rs index c337c4b..08c32e5 100644 --- a/src/bindgen/ir/constant.rs +++ b/src/bindgen/ir/constant.rs @@ -494,7 +494,7 @@ impl Constant { debug_assert!(config.structure.associated_constants_in_body); debug_assert!(config.constant.allow_static_const); - if let Type::ConstPtr(..) = self.ty { + if let Type::ConstPtr { .. } = self.ty { out.write("static "); } else { out.write("static const "); @@ -579,7 +579,7 @@ impl Constant { out.write(if in_body { "inline " } else { "static " }); } - if let Type::ConstPtr(..) = self.ty { + if let Type::ConstPtr { .. } = self.ty { // Nothing. } else { out.write("const "); diff --git a/src/bindgen/ir/function.rs b/src/bindgen/ir/function.rs index a6bc8d2..adda7c4 100644 --- a/src/bindgen/ir/function.rs +++ b/src/bindgen/ir/function.rs @@ -305,8 +305,14 @@ fn gen_self_type(receiver: &syn::Receiver) -> Type { let self_ty = Type::Path(GenericPath::self_path()); match receiver.reference { Some(_) => match receiver.mutability { - Some(_) => Type::Ptr(Box::new(self_ty)), - None => Type::ConstPtr(Box::new(self_ty)), + Some(_) => Type::Ptr { + ty: Box::new(self_ty), + is_nullable: true, + }, + None => Type::ConstPtr { + ty: Box::new(self_ty), + is_nullable: true, + }, }, None => self_ty, } diff --git a/src/bindgen/ir/global.rs b/src/bindgen/ir/global.rs index d58f35f..b5cf027 100644 --- a/src/bindgen/ir/global.rs +++ b/src/bindgen/ir/global.rs @@ -109,7 +109,7 @@ impl Item for Static { impl Source for Static { fn write(&self, config: &Config, out: &mut SourceWriter) { out.write("extern "); - if let Type::ConstPtr(..) = self.ty { + if let Type::ConstPtr { .. } = self.ty { } else if !self.mutable { out.write("const "); } diff --git a/src/bindgen/ir/ty.rs b/src/bindgen/ir/ty.rs index 9e94815..12f4888 100644 --- a/src/bindgen/ir/ty.rs +++ b/src/bindgen/ir/ty.rs @@ -209,10 +209,8 @@ impl ArrayLength { #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum Type { - ConstPtr(Box), - Ptr(Box), - ConstNonNullPtr(Box), - NonNullPtr(Box), + ConstPtr { ty: Box, is_nullable: bool }, + Ptr { ty: Box, is_nullable: bool }, Ref(Box), MutRef(Box), Path(GenericPath), @@ -237,8 +235,14 @@ impl Type { }; match reference.mutability { - Some(_) => Type::NonNullPtr(Box::new(converted)), - None => Type::ConstNonNullPtr(Box::new(converted)), + Some(_) => Type::Ptr { + ty: Box::new(converted), + is_nullable: false, + }, + None => Type::ConstPtr { + ty: Box::new(converted), + is_nullable: false, + }, } } syn::Type::Ptr(ref pointer) => { @@ -254,8 +258,14 @@ impl Type { }; match pointer.mutability { - Some(_) => Type::Ptr(Box::new(converted)), - None => Type::ConstPtr(Box::new(converted)), + Some(_) => Type::Ptr { + ty: Box::new(converted), + is_nullable: true, + }, + None => Type::ConstPtr { + ty: Box::new(converted), + is_nullable: true, + }, } } syn::Type::Path(ref path) => { @@ -360,7 +370,7 @@ impl Type { pub fn is_primitive_or_ptr_primitive(&self) -> bool { match *self { Type::Primitive(..) => true, - Type::ConstPtr(ref x) => match x.as_ref() { + Type::ConstPtr { ref ty, .. } => match ty.as_ref() { Type::Primitive(..) => true, _ => false, }, @@ -370,10 +380,8 @@ impl Type { pub fn is_repr_ptr(&self) -> bool { match *self { - Type::Ptr(..) => true, - Type::NonNullPtr(..) => true, - Type::ConstPtr(..) => true, - Type::ConstNonNullPtr(..) => true, + Type::Ptr { .. } => true, + Type::ConstPtr { .. } => true, Type::FuncPtr(..) => true, _ => false, } @@ -381,10 +389,14 @@ impl Type { pub fn make_nullable(&self) -> Option { match self.clone() { - Type::Ptr(x) => Some(Type::Ptr(x)), - Type::NonNullPtr(x) => Some(Type::Ptr(x)), - Type::ConstPtr(x) => Some(Type::ConstPtr(x)), - Type::ConstNonNullPtr(x) => Some(Type::ConstPtr(x)), + Type::Ptr { ty, .. } => Some(Type::Ptr { + ty, + is_nullable: true, + }), + Type::ConstPtr { ty, .. } => Some(Type::ConstPtr { + ty, + is_nullable: true, + }), Type::FuncPtr(x, y) => Some(Type::FuncPtr(x, y)), _ => None, } @@ -406,7 +418,10 @@ impl Type { match path.name() { // FIXME(#223): This is not quite correct. "Option" if generic.is_repr_ptr() => generic.make_nullable(), - "NonNull" => Some(Type::NonNullPtr(Box::new(generic))), + "NonNull" => Some(Type::Ptr { + ty: Box::new(generic), + is_nullable: false, + }), "Cell" => Some(generic), _ => None, } @@ -423,10 +438,8 @@ impl Type { Type::Array(ref mut ty, ..) | Type::MutRef(ref mut ty) | Type::Ref(ref mut ty) - | Type::Ptr(ref mut ty) - | Type::NonNullPtr(ref mut ty) - | Type::ConstNonNullPtr(ref mut ty) - | Type::ConstPtr(ref mut ty) => ty.replace_self_with(self_ty), + | Type::Ptr { ref mut ty, .. } + | Type::ConstPtr { ref mut ty, .. } => ty.replace_self_with(self_ty), Type::Path(ref mut generic_path) => { generic_path.replace_self_with(self_ty); } @@ -444,10 +457,8 @@ impl Type { let mut current = self; loop { match *current { - Type::ConstPtr(ref ty) => current = ty, - Type::Ptr(ref ty) => current = ty, - Type::NonNullPtr(ref ty) => current = ty, - Type::ConstNonNullPtr(ref ty) => current = ty, + Type::ConstPtr { ref ty, .. } => current = ty, + Type::Ptr { ref ty, .. } => current = ty, Type::Ref(ref ty) => current = ty, Type::MutRef(ref ty) => current = ty, Type::Path(ref generic) => { @@ -468,12 +479,20 @@ impl Type { pub fn specialize(&self, mappings: &[(&Path, &Type)]) -> Type { match *self { - Type::ConstPtr(ref ty) => Type::ConstPtr(Box::new(ty.specialize(mappings))), - Type::Ptr(ref ty) => Type::Ptr(Box::new(ty.specialize(mappings))), - Type::NonNullPtr(ref ty) => Type::NonNullPtr(Box::new(ty.specialize(mappings))), - Type::ConstNonNullPtr(ref ty) => { - Type::ConstNonNullPtr(Box::new(ty.specialize(mappings))) - } + Type::ConstPtr { + ref ty, + is_nullable, + } => Type::ConstPtr { + ty: Box::new(ty.specialize(mappings)), + is_nullable, + }, + Type::Ptr { + ref ty, + is_nullable, + } => Type::Ptr { + ty: Box::new(ty.specialize(mappings)), + is_nullable, + }, Type::Ref(ref ty) => Type::Ref(Box::new(ty.specialize(mappings))), Type::MutRef(ref ty) => Type::MutRef(Box::new(ty.specialize(mappings))), Type::Path(ref generic_path) => { @@ -514,16 +533,10 @@ impl Type { out: &mut Dependencies, ) { match *self { - Type::ConstPtr(ref ty) => { + Type::ConstPtr { ref ty, .. } => { ty.add_dependencies_ignoring_generics(generic_params, library, out); } - Type::Ptr(ref ty) => { - ty.add_dependencies_ignoring_generics(generic_params, library, out); - } - Type::NonNullPtr(ref ty) => { - ty.add_dependencies_ignoring_generics(generic_params, library, out); - } - Type::ConstNonNullPtr(ref ty) => { + Type::Ptr { ref ty, .. } => { ty.add_dependencies_ignoring_generics(generic_params, library, out); } Type::Ref(ref ty) | Type::MutRef(ref ty) => { @@ -574,16 +587,10 @@ impl Type { pub fn add_monomorphs(&self, library: &Library, out: &mut Monomorphs) { match *self { - Type::ConstPtr(ref ty) => { + Type::ConstPtr { ref ty, .. } => { ty.add_monomorphs(library, out); } - Type::Ptr(ref ty) => { - ty.add_monomorphs(library, out); - } - Type::NonNullPtr(ref ty) => { - ty.add_monomorphs(library, out); - } - Type::ConstNonNullPtr(ref ty) => { + Type::Ptr { ref ty, .. } => { ty.add_monomorphs(library, out); } Type::Ref(ref ty) | Type::MutRef(ref ty) => { @@ -616,16 +623,10 @@ impl Type { pub fn rename_for_config(&mut self, config: &Config, generic_params: &GenericParams) { match *self { - Type::ConstPtr(ref mut ty) => { + Type::ConstPtr { ref mut ty, .. } => { ty.rename_for_config(config, generic_params); } - Type::Ptr(ref mut ty) => { - ty.rename_for_config(config, generic_params); - } - Type::NonNullPtr(ref mut ty) => { - ty.rename_for_config(config, generic_params); - } - Type::ConstNonNullPtr(ref mut ty) => { + Type::Ptr { ref mut ty, .. } => { ty.rename_for_config(config, generic_params); } Type::Ref(ref mut ty) | Type::MutRef(ref mut ty) => { @@ -650,16 +651,10 @@ impl Type { pub fn resolve_declaration_types(&mut self, resolver: &DeclarationTypeResolver) { match *self { - Type::ConstPtr(ref mut ty) => { + Type::ConstPtr { ref mut ty, .. } => { ty.resolve_declaration_types(resolver); } - Type::Ptr(ref mut ty) => { - ty.resolve_declaration_types(resolver); - } - Type::NonNullPtr(ref mut ty) => { - ty.resolve_declaration_types(resolver); - } - Type::ConstNonNullPtr(ref mut ty) => { + Type::Ptr { ref mut ty, .. } => { ty.resolve_declaration_types(resolver); } Type::Ref(ref mut ty) | Type::MutRef(ref mut ty) => { @@ -683,16 +678,10 @@ impl Type { pub fn mangle_paths(&mut self, monomorphs: &Monomorphs) { match *self { - Type::ConstPtr(ref mut ty) => { + Type::ConstPtr { ref mut ty, .. } => { ty.mangle_paths(monomorphs); } - Type::Ptr(ref mut ty) => { - ty.mangle_paths(monomorphs); - } - Type::NonNullPtr(ref mut ty) => { - ty.mangle_paths(monomorphs); - } - Type::ConstNonNullPtr(ref mut ty) => { + Type::Ptr { ref mut ty, .. } => { ty.mangle_paths(monomorphs); } Type::Ref(ref mut ty) | Type::MutRef(ref mut ty) => { @@ -728,10 +717,8 @@ impl Type { pub fn can_cmp_order(&self) -> bool { match *self { - Type::ConstPtr(..) => true, - Type::Ptr(..) => true, - Type::NonNullPtr(..) => true, - Type::ConstNonNullPtr(..) => true, + Type::ConstPtr { .. } => true, + Type::Ptr { .. } => true, Type::Ref(..) | Type::MutRef(..) => false, Type::Path(..) => true, Type::Primitive(ref p) => p.can_cmp_order(), @@ -742,10 +729,8 @@ impl Type { pub fn can_cmp_eq(&self) -> bool { match *self { - Type::ConstPtr(..) => true, - Type::Ptr(..) => true, - Type::NonNullPtr(..) => true, - Type::ConstNonNullPtr(..) => true, + Type::ConstPtr { .. } => true, + Type::Ptr { .. } => true, Type::Ref(..) | Type::MutRef(..) => false, Type::Path(..) => true, Type::Primitive(ref p) => p.can_cmp_eq(), diff --git a/src/bindgen/mangle.rs b/src/bindgen/mangle.rs index e20b03b..4f3ff39 100644 --- a/src/bindgen/mangle.rs +++ b/src/bindgen/mangle.rs @@ -58,11 +58,11 @@ impl<'a> Mangler<'a> { Type::Primitive(ref primitive) => { self.output.push_str(primitive.to_repr_rust()); } - Type::Ptr(ref ty) | Type::MutRef(ref ty) | Type::NonNullPtr(ref ty) => { + Type::Ptr { ref ty, .. } | Type::MutRef(ref ty) => { self.push(Separator::BeginMutPtr); self.append_mangled_type(&**ty, last); } - Type::ConstPtr(ref ty) | Type::Ref(ref ty) | Type::ConstNonNullPtr(ref ty) => { + Type::ConstPtr { ref ty, .. } | Type::Ref(ref ty) => { self.push(Separator::BeginConstPtr); self.append_mangled_type(&**ty, last); } diff --git a/tests/rust/nonnull_attribute.rs b/tests/rust/nonnull_attribute.rs index 46e8f4d..5089ba9 100644 --- a/tests/rust/nonnull_attribute.rs +++ b/tests/rust/nonnull_attribute.rs @@ -37,16 +37,16 @@ pub extern "C" fn mutltiple_args( } #[no_mangle] -pub extern "C" fn mut_ref_arg(arg: &Pointers) {} +pub extern "C" fn ref_arg(arg: &Pointers) {} #[no_mangle] -pub extern "C" fn ref_arg(arg: &mut Pointers) {} +pub extern "C" fn mut_ref_arg(arg: &mut Pointers) {} #[no_mangle] -pub extern "C" fn optional_mut_ref_arg(arg: Option<&Pointers>) {} +pub extern "C" fn optional_ref_arg(arg: Option<&Pointers>) {} #[no_mangle] -pub extern "C" fn optional_ref_arg(arg: Option<&mut Pointers>) {} +pub extern "C" fn optional_mut_ref_arg(arg: Option<&mut Pointers>) {} #[no_mangle] pub extern "C" fn nullable_const_ptr(arg: *const Pointers) {} diff --git a/tests/rust/nonnull_attribute.toml b/tests/rust/nonnull_attribute.toml index ca2a341..8c051d7 100644 --- a/tests/rust/nonnull_attribute.toml +++ b/tests/rust/nonnull_attribute.toml @@ -6,4 +6,5 @@ header = """ #endif """ +[ptr] non_null_attribute = "CBINDGEN_NONNULL" \ No newline at end of file