From f31340159e8a30882a7a41dd7838b87a7d6fe753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 12:45:39 +0200 Subject: [PATCH 01/10] options: Allow configuring the generation of constructors. --- src/options.rs | 5 +-- tests/expectations/tests/gen-constructors.rs | 33 ++++++++++++++++++++ tests/headers/gen-constructors.hpp | 6 ++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/expectations/tests/gen-constructors.rs create mode 100644 tests/headers/gen-constructors.hpp diff --git a/src/options.rs b/src/options.rs index f1c8479acf..9f229f6676 100644 --- a/src/options.rs +++ b/src/options.rs @@ -111,8 +111,8 @@ pub fn builder_from_flags Arg::with_name("generate") .long("generate") .help("Generate a given kind of items, split by commas. \ - Valid values are \"functions\",\"types\", \"vars\" and \ - \"methods\".") + Valid values are \"functions\",\"types\", \"vars\", \ + \"methods\" and \"constructors\".") .takes_value(true), Arg::with_name("ignore-methods") .long("ignore-methods") @@ -271,6 +271,7 @@ pub fn builder_from_flags "types" => config.types = true, "vars" => config.vars = true, "methods" => config.methods = true, + "constructors" => config.constructors = true, _ => { return Err(Error::new(ErrorKind::Other, "Unknown generate item")); diff --git a/tests/expectations/tests/gen-constructors.rs b/tests/expectations/tests/gen-constructors.rs new file mode 100644 index 0000000000..72c2fc5300 --- /dev/null +++ b/tests/expectations/tests/gen-constructors.rs @@ -0,0 +1,33 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Default, Copy)] +pub struct Foo { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!(::std::mem::size_of::() , 1usize , concat ! ( + "Size of: " , stringify ! ( Foo ) )); + assert_eq! (::std::mem::align_of::() , 1usize , concat ! ( + "Alignment of " , stringify ! ( Foo ) )); +} +extern "C" { + #[link_name = "_ZN3FooC1Ei"] + pub fn Foo_Foo(this: *mut Foo, a: ::std::os::raw::c_int); +} +impl Clone for Foo { + fn clone(&self) -> Self { *self } +} +impl Foo { + #[inline] + pub unsafe fn new(a: ::std::os::raw::c_int) -> Self { + let mut __bindgen_tmp = ::std::mem::uninitialized(); + Foo_Foo(&mut __bindgen_tmp, a); + __bindgen_tmp + } +} diff --git a/tests/headers/gen-constructors.hpp b/tests/headers/gen-constructors.hpp new file mode 100644 index 0000000000..809d6ef998 --- /dev/null +++ b/tests/headers/gen-constructors.hpp @@ -0,0 +1,6 @@ +// bindgen-flags: --generate types,constructors,functions + +class Foo { + public: + Foo(int a); +}; From 8f1e5f5ad4fff230b06cdfca868266a28ada3881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 14:36:59 +0200 Subject: [PATCH 02/10] ir: Force the D1 destructor symbol even in older libclang versions. --- src/ir/function.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/ir/function.rs b/src/ir/function.rs index fd88b657d1..2c588fdd54 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -114,6 +114,7 @@ fn get_abi(cc: CXCallingConv) -> Option { pub fn cursor_mangling(ctx: &BindgenContext, cursor: &clang::Cursor) -> Option { + use clang_sys; if !ctx.options().enable_mangling { return None; } @@ -131,10 +132,37 @@ pub fn cursor_mangling(ctx: &BindgenContext, } // Try to undo backend linkage munging (prepended _, generally) + // + // TODO(emilio): This is wrong when the target system is not the host + // system. See https://github.com/servo/rust-bindgen/issues/593 if cfg!(target_os = "macos") { mangling.remove(0); } + if cursor.kind() == clang_sys::CXCursor_Destructor { + // With old (3.8-) libclang versions, and the Itanium ABI, clang returns + // the "destructor group 0" symbol, which means that it'll try to free + // memory, which definitely isn't what we want. + // + // Explicitly force the destructor group 1 symbol. + // + // See http://refspecs.linuxbase.org/cxxabi-1.83.html#mangling-special + // for the reference, and http://stackoverflow.com/a/6614369/1091587 for + // a more friendly explanation. + // + // We don't need to do this for constructors since clang seems to always + // have returned the C1 constructor. + // + // FIXME(emilio): Can a legit symbol in other ABIs end with this string? + // I don't think so, but if it can this would become a linker error + // anyway, not an invalid free at runtime. + if mangling.ends_with("D0Ev") { + let new_len = mangling.len() - 4; + mangling.truncate(new_len); + mangling.push_str("D1Ev"); + } + } + Some(mangling) } From 1714f947e062a7b90ebda71d190daccba87ab32d Mon Sep 17 00:00:00 2001 From: Nikhil Shagrithaya Date: Mon, 27 Feb 2017 16:01:15 +0530 Subject: [PATCH 03/10] Add codegen for destructors. --- src/codegen/mod.rs | 15 +++++++++++++++ src/ir/comp.rs | 21 +++++++++++++++++++-- src/ir/function.rs | 18 ++++++++++++++---- src/lib.rs | 4 ++++ tests/expectations/tests/public-dtor.rs | 10 ++++++++++ tests/expectations/tests/union_dtor.rs | 10 ++++++++++ tests/expectations/tests/virtual_dtor.rs | 10 ++++++++++ 7 files changed, 82 insertions(+), 6 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index fb6c839d84..d15232734f 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1597,6 +1597,20 @@ impl CodeGenerator for CompInfo { self); } } + + if ctx.options().codegen_config.destructor { + if let Some(destructor) = *self.destructor() { + Method::new(MethodKind::Destructor, + destructor, + false) + .codegen_method(ctx, + &mut methods, + &mut method_names, + result, + whitelisted_items, + self); + } + } } // NB: We can't use to_rust_ty here since for opaque types this tries to @@ -1701,6 +1715,7 @@ impl MethodCodegen for Method { let signature_item = ctx.resolve_item(function.signature()); let mut name = match self.kind() { MethodKind::Constructor => "new".into(), + MethodKind::Destructor => "__bindgen_destructor__".into(), _ => function.name().to_owned(), }; diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 9a6548a7ef..44e7005872 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -26,6 +26,8 @@ pub enum MethodKind { /// A constructor. We represent it as method for convenience, to avoid code /// duplication. Constructor, + /// A destructor method + Destructor, /// A static method. Static, /// A normal method. @@ -61,6 +63,11 @@ impl Method { self.kind } + /// Is this a destructor method? + pub fn is_destructor(&self) -> bool { + self.kind == MethodKind::Destructor + } + /// Is this a constructor? pub fn is_constructor(&self) -> bool { self.kind == MethodKind::Constructor @@ -250,6 +257,9 @@ pub struct CompInfo { /// The different constructors this struct or class contains. constructors: Vec, + /// The destructor of this type + destructor: Option, + /// Vector of classes this one inherits from. base_members: Vec, @@ -321,6 +331,7 @@ impl CompInfo { template_params: vec![], methods: vec![], constructors: vec![], + destructor: None, base_members: vec![], inner_types: vec![], inner_vars: vec![], @@ -434,6 +445,11 @@ impl CompInfo { &self.constructors } + /// Get this type's destructor. + pub fn destructor(&self) -> &Option { + &self.destructor + } + /// What kind of compound type is this? pub fn kind(&self) -> CompKind { self.kind @@ -657,8 +673,9 @@ impl CompInfo { CXCursor_Constructor => { ci.constructors.push(signature); } - // TODO(emilio): Bind the destructor? - CXCursor_Destructor => {} + CXCursor_Destructor => { + ci.destructor = Some(signature); + } CXCursor_CXXMethod => { let is_const = cur.method_is_const(); let method_kind = if is_static { diff --git a/src/ir/function.rs b/src/ir/function.rs index 2c588fdd54..163226e305 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -248,13 +248,14 @@ impl FunctionSig { let is_method = cursor.kind() == CXCursor_CXXMethod; let is_constructor = cursor.kind() == CXCursor_Constructor; - if (is_constructor || is_method) && + let is_destructor = cursor.kind() == CXCursor_Destructor; + if (is_constructor || is_destructor || is_method) && cursor.lexical_parent() != cursor.semantic_parent() { // Only parse constructors once. return Err(ParseError::Continue); } - if is_method || is_constructor { + if is_method || is_constructor || is_destructor { let is_const = is_method && cursor.method_is_const(); let is_virtual = is_method && cursor.method_is_virtual(); let is_static = is_method && cursor.method_is_static(); @@ -320,9 +321,9 @@ impl ClangSubItemParser for Function { -> Result, ParseError> { use clang_sys::*; match cursor.kind() { - // FIXME(emilio): Generate destructors properly. CXCursor_FunctionDecl | CXCursor_Constructor | + CXCursor_Destructor | CXCursor_CXXMethod => {} _ => return Err(ParseError::Continue), }; @@ -353,7 +354,16 @@ impl ClangSubItemParser for Function { let sig = try!(Item::from_ty(&cursor.cur_type(), cursor, None, context)); - let name = cursor.spelling(); + let name = match cursor.kind() { + CXCursor_Destructor => { + let mut name_ = cursor.spelling(); + // remove the `~` + name_.remove(0); + name_ + "_destructor" + }, + _ => cursor.spelling(), + }; + assert!(!name.is_empty(), "Empty function name?"); let mut mangled_name = cursor_mangling(context, &cursor); diff --git a/src/lib.rs b/src/lib.rs index eccf80e347..8ee1653432 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -109,6 +109,8 @@ pub struct CodegenConfig { pub methods: bool, /// Whether to generate constructors. pub constructors: bool, + /// Whether to generate a destructor. + pub destructor: bool, } impl CodegenConfig { @@ -120,6 +122,7 @@ impl CodegenConfig { vars: true, methods: true, constructors: true, + destructor: true, } } @@ -131,6 +134,7 @@ impl CodegenConfig { vars: false, methods: false, constructors: false, + destructor: false, } } } diff --git a/tests/expectations/tests/public-dtor.rs b/tests/expectations/tests/public-dtor.rs index 1accf49cb5..0412b9f4b7 100644 --- a/tests/expectations/tests/public-dtor.rs +++ b/tests/expectations/tests/public-dtor.rs @@ -16,3 +16,13 @@ fn bindgen_test_layout_cv_String() { assert_eq! (::std::mem::align_of::() , 1usize , concat ! ( "Alignment of " , stringify ! ( cv_String ) )); } +extern "C" { + #[link_name = "_ZN2cv6StringD1Ev"] + pub fn cv_String_String_destructor(this: *mut cv_String); +} +impl cv_String { + #[inline] + pub unsafe fn __bindgen_destructor__(&mut self) { + cv_String_String_destructor(&mut *self) + } +} diff --git a/tests/expectations/tests/union_dtor.rs b/tests/expectations/tests/union_dtor.rs index bfd573e021..1bcb45b672 100644 --- a/tests/expectations/tests/union_dtor.rs +++ b/tests/expectations/tests/union_dtor.rs @@ -52,3 +52,13 @@ fn bindgen_test_layout_UnionWithDtor() { "Alignment of field: " , stringify ! ( UnionWithDtor ) , "::" , stringify ! ( mBar ) )); } +extern "C" { + #[link_name = "_ZN13UnionWithDtorD1Ev"] + pub fn UnionWithDtor_UnionWithDtor_destructor(this: *mut UnionWithDtor); +} +impl UnionWithDtor { + #[inline] + pub unsafe fn __bindgen_destructor__(&mut self) { + UnionWithDtor_UnionWithDtor_destructor(&mut *self) + } +} diff --git a/tests/expectations/tests/virtual_dtor.rs b/tests/expectations/tests/virtual_dtor.rs index e5d3ace2fc..a84da69382 100644 --- a/tests/expectations/tests/virtual_dtor.rs +++ b/tests/expectations/tests/virtual_dtor.rs @@ -18,6 +18,16 @@ fn bindgen_test_layout_nsSlots() { assert_eq! (::std::mem::align_of::() , 8usize , concat ! ( "Alignment of " , stringify ! ( nsSlots ) )); } +extern "C" { + #[link_name = "_ZN7nsSlotsD1Ev"] + pub fn nsSlots_nsSlots_destructor(this: *mut nsSlots); +} impl Default for nsSlots { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } +impl nsSlots { + #[inline] + pub unsafe fn __bindgen_destructor__(&mut self) { + nsSlots_nsSlots_destructor(&mut *self) + } +} From 5cf424ca484bdc3c7ab0c7e396fef25485bf4c83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 13:11:35 +0200 Subject: [PATCH 04/10] Refactor a bit and do not generate virtual destructors. --- src/codegen/mod.rs | 13 ++++++++---- src/ir/comp.rs | 21 ++++++++++++-------- src/ir/function.rs | 25 ++++++++++++++---------- tests/expectations/tests/virtual_dtor.rs | 10 ---------- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index d15232734f..78b2e02ad9 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1599,10 +1599,14 @@ impl CodeGenerator for CompInfo { } if ctx.options().codegen_config.destructor { - if let Some(destructor) = *self.destructor() { - Method::new(MethodKind::Destructor, - destructor, - false) + if let Some((is_virtual, destructor)) = self.destructor() { + let kind = if is_virtual { + MethodKind::VirtualDestructor + } else { + MethodKind::Destructor + }; + + Method::new(kind, destructor, false) .codegen_method(ctx, &mut methods, &mut method_names, @@ -1707,6 +1711,7 @@ impl MethodCodegen for Method { if self.is_virtual() { return; // FIXME } + // First of all, output the actual function. let function_item = ctx.resolve_item(self.signature()); function_item.codegen(ctx, result, whitelisted_items, &()); diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 44e7005872..2c7b6de239 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -26,8 +26,10 @@ pub enum MethodKind { /// A constructor. We represent it as method for convenience, to avoid code /// duplication. Constructor, - /// A destructor method + /// A destructor. Destructor, + /// A virtual destructor. + VirtualDestructor, /// A static method. Static, /// A normal method. @@ -65,7 +67,8 @@ impl Method { /// Is this a destructor method? pub fn is_destructor(&self) -> bool { - self.kind == MethodKind::Destructor + self.kind == MethodKind::Destructor || + self.kind == MethodKind::VirtualDestructor } /// Is this a constructor? @@ -75,7 +78,8 @@ impl Method { /// Is this a virtual method? pub fn is_virtual(&self) -> bool { - self.kind == MethodKind::Virtual + self.kind == MethodKind::Virtual || + self.kind == MethodKind::VirtualDestructor } /// Is this a static method? @@ -257,8 +261,9 @@ pub struct CompInfo { /// The different constructors this struct or class contains. constructors: Vec, - /// The destructor of this type - destructor: Option, + /// The destructor of this type. The bool represents whether this destructor + /// is virtual. + destructor: Option<(bool, ItemId)>, /// Vector of classes this one inherits from. base_members: Vec, @@ -446,8 +451,8 @@ impl CompInfo { } /// Get this type's destructor. - pub fn destructor(&self) -> &Option { - &self.destructor + pub fn destructor(&self) -> Option<(bool, ItemId)> { + self.destructor } /// What kind of compound type is this? @@ -674,7 +679,7 @@ impl CompInfo { ci.constructors.push(signature); } CXCursor_Destructor => { - ci.destructor = Some(signature); + ci.destructor = Some((is_virtual, signature)); } CXCursor_CXXMethod => { let is_const = cur.method_is_const(); diff --git a/src/ir/function.rs b/src/ir/function.rs index 163226e305..f80a573615 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -354,18 +354,23 @@ impl ClangSubItemParser for Function { let sig = try!(Item::from_ty(&cursor.cur_type(), cursor, None, context)); - let name = match cursor.kind() { - CXCursor_Destructor => { - let mut name_ = cursor.spelling(); - // remove the `~` - name_.remove(0); - name_ + "_destructor" - }, - _ => cursor.spelling(), - }; - + let mut name = cursor.spelling(); assert!(!name.is_empty(), "Empty function name?"); + if cursor.kind() == CXCursor_Destructor { + // Remove the leading `~`. The alternative to this is special-casing + // code-generation for destructor functions, which seems less than + // ideal. + if name.starts_with('~') { + name.remove(0); + } + + // Add a suffix to avoid colliding with constructors. This would be + // technically fine (since we handle duplicated functions/methods), + // but seems easy enough to handle it here. + name.push_str("_destructor"); + } + let mut mangled_name = cursor_mangling(context, &cursor); if mangled_name.as_ref() == Some(&name) { mangled_name = None; diff --git a/tests/expectations/tests/virtual_dtor.rs b/tests/expectations/tests/virtual_dtor.rs index a84da69382..e5d3ace2fc 100644 --- a/tests/expectations/tests/virtual_dtor.rs +++ b/tests/expectations/tests/virtual_dtor.rs @@ -18,16 +18,6 @@ fn bindgen_test_layout_nsSlots() { assert_eq! (::std::mem::align_of::() , 8usize , concat ! ( "Alignment of " , stringify ! ( nsSlots ) )); } -extern "C" { - #[link_name = "_ZN7nsSlotsD1Ev"] - pub fn nsSlots_nsSlots_destructor(this: *mut nsSlots); -} impl Default for nsSlots { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } -impl nsSlots { - #[inline] - pub unsafe fn __bindgen_destructor__(&mut self) { - nsSlots_nsSlots_destructor(&mut *self) - } -} From fcbd61ce793c7505b1a956555dd816aa87ac0141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 13:15:24 +0200 Subject: [PATCH 05/10] options: Allow configuring destructors via CLI. --- src/codegen/mod.rs | 2 +- src/lib.rs | 8 ++--- src/options.rs | 3 +- tests/expectations/tests/gen-destructors.rs | 33 +++++++++++++++++++++ tests/headers/gen-destructors.hpp | 7 +++++ 5 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 tests/expectations/tests/gen-destructors.rs create mode 100644 tests/headers/gen-destructors.hpp diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 78b2e02ad9..6d9af09b2b 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1598,7 +1598,7 @@ impl CodeGenerator for CompInfo { } } - if ctx.options().codegen_config.destructor { + if ctx.options().codegen_config.destructors { if let Some((is_virtual, destructor)) = self.destructor() { let kind = if is_virtual { MethodKind::VirtualDestructor diff --git a/src/lib.rs b/src/lib.rs index 8ee1653432..e6a66dfc42 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -109,8 +109,8 @@ pub struct CodegenConfig { pub methods: bool, /// Whether to generate constructors. pub constructors: bool, - /// Whether to generate a destructor. - pub destructor: bool, + /// Whether to generate destructors. + pub destructors: bool, } impl CodegenConfig { @@ -122,7 +122,7 @@ impl CodegenConfig { vars: true, methods: true, constructors: true, - destructor: true, + destructors: true, } } @@ -134,7 +134,7 @@ impl CodegenConfig { vars: false, methods: false, constructors: false, - destructor: false, + destructors: false, } } } diff --git a/src/options.rs b/src/options.rs index 9f229f6676..6d06ad3b2f 100644 --- a/src/options.rs +++ b/src/options.rs @@ -112,7 +112,7 @@ pub fn builder_from_flags .long("generate") .help("Generate a given kind of items, split by commas. \ Valid values are \"functions\",\"types\", \"vars\", \ - \"methods\" and \"constructors\".") + \"methods\", \"constructors\" and \"destructors\".") .takes_value(true), Arg::with_name("ignore-methods") .long("ignore-methods") @@ -272,6 +272,7 @@ pub fn builder_from_flags "vars" => config.vars = true, "methods" => config.methods = true, "constructors" => config.constructors = true, + "destructors" => config.destructors = true, _ => { return Err(Error::new(ErrorKind::Other, "Unknown generate item")); diff --git a/tests/expectations/tests/gen-destructors.rs b/tests/expectations/tests/gen-destructors.rs new file mode 100644 index 0000000000..14a51d1401 --- /dev/null +++ b/tests/expectations/tests/gen-destructors.rs @@ -0,0 +1,33 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Default)] +pub struct Foo { + pub bar: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!(::std::mem::size_of::() , 4usize , concat ! ( + "Size of: " , stringify ! ( Foo ) )); + assert_eq! (::std::mem::align_of::() , 4usize , concat ! ( + "Alignment of " , stringify ! ( Foo ) )); + assert_eq! (unsafe { + & ( * ( 0 as * const Foo ) ) . bar as * const _ as usize } , + 0usize , concat ! ( + "Alignment of field: " , stringify ! ( Foo ) , "::" , + stringify ! ( bar ) )); +} +extern "C" { + #[link_name = "_ZN3FooD1Ev"] + pub fn Foo_Foo_destructor(this: *mut Foo); +} +impl Foo { + #[inline] + pub unsafe fn __bindgen_destructor__(&mut self) { + Foo_Foo_destructor(&mut *self) + } +} diff --git a/tests/headers/gen-destructors.hpp b/tests/headers/gen-destructors.hpp new file mode 100644 index 0000000000..719eb248a9 --- /dev/null +++ b/tests/headers/gen-destructors.hpp @@ -0,0 +1,7 @@ +// bindgen-flags: --generate types,destructors,functions + +class Foo { + int bar; + public: + ~Foo(); +}; From ebcd36954fbdc090a5007196b6658c20b8a048ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 13:22:01 +0200 Subject: [PATCH 06/10] codegen: Rename the destructor methods to "destruct". --- src/codegen/mod.rs | 2 +- tests/expectations/tests/gen-destructors.rs | 4 +--- tests/expectations/tests/public-dtor.rs | 2 +- tests/expectations/tests/union_dtor.rs | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 6d9af09b2b..72c03c3746 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1720,7 +1720,7 @@ impl MethodCodegen for Method { let signature_item = ctx.resolve_item(function.signature()); let mut name = match self.kind() { MethodKind::Constructor => "new".into(), - MethodKind::Destructor => "__bindgen_destructor__".into(), + MethodKind::Destructor => "destruct".into(), _ => function.name().to_owned(), }; diff --git a/tests/expectations/tests/gen-destructors.rs b/tests/expectations/tests/gen-destructors.rs index 14a51d1401..4bbc839391 100644 --- a/tests/expectations/tests/gen-destructors.rs +++ b/tests/expectations/tests/gen-destructors.rs @@ -27,7 +27,5 @@ extern "C" { } impl Foo { #[inline] - pub unsafe fn __bindgen_destructor__(&mut self) { - Foo_Foo_destructor(&mut *self) - } + pub unsafe fn destruct(&mut self) { Foo_Foo_destructor(&mut *self) } } diff --git a/tests/expectations/tests/public-dtor.rs b/tests/expectations/tests/public-dtor.rs index 0412b9f4b7..519cdf6734 100644 --- a/tests/expectations/tests/public-dtor.rs +++ b/tests/expectations/tests/public-dtor.rs @@ -22,7 +22,7 @@ extern "C" { } impl cv_String { #[inline] - pub unsafe fn __bindgen_destructor__(&mut self) { + pub unsafe fn destruct(&mut self) { cv_String_String_destructor(&mut *self) } } diff --git a/tests/expectations/tests/union_dtor.rs b/tests/expectations/tests/union_dtor.rs index 1bcb45b672..af9a8c041b 100644 --- a/tests/expectations/tests/union_dtor.rs +++ b/tests/expectations/tests/union_dtor.rs @@ -58,7 +58,7 @@ extern "C" { } impl UnionWithDtor { #[inline] - pub unsafe fn __bindgen_destructor__(&mut self) { + pub unsafe fn destruct(&mut self) { UnionWithDtor_UnionWithDtor_destructor(&mut *self) } } From 7aed4bab534c39c3c218d2fe54d6f86be07aac52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 13:33:21 +0200 Subject: [PATCH 07/10] codegen: Add integration tests for destructors. --- bindgen-integration/cpp/Test.cc | 11 ++++++++++- bindgen-integration/cpp/Test.h | 8 ++++++++ bindgen-integration/src/lib.rs | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/bindgen-integration/cpp/Test.cc b/bindgen-integration/cpp/Test.cc index 1d9624069c..7b0ec4addd 100644 --- a/bindgen-integration/cpp/Test.cc +++ b/bindgen-integration/cpp/Test.cc @@ -21,6 +21,15 @@ Test::Test(double foo) , m_double(foo) {} +AutoRestoreBool::AutoRestoreBool(bool* ptr) + : m_ptr(ptr) + , m_value(*ptr) +{} + +AutoRestoreBool::~AutoRestoreBool() { + *m_ptr = m_value; +} + namespace bitfields { bool @@ -47,4 +56,4 @@ Third::assert(int first, bool second, ItemKind third) kind == third; } -} +} // namespace bitfields diff --git a/bindgen-integration/cpp/Test.h b/bindgen-integration/cpp/Test.h index 310478bbf4..01c7aea1c7 100644 --- a/bindgen-integration/cpp/Test.h +++ b/bindgen-integration/cpp/Test.h @@ -64,3 +64,11 @@ struct Third { }; } // namespace bitfields + +struct AutoRestoreBool { + bool* m_ptr; + bool m_value; + + AutoRestoreBool(bool*); + ~AutoRestoreBool(); +}; diff --git a/bindgen-integration/src/lib.rs b/bindgen-integration/src/lib.rs index 8d7eb75379..be3c84518b 100755 --- a/bindgen-integration/src/lib.rs +++ b/bindgen-integration/src/lib.rs @@ -101,3 +101,21 @@ fn test_bitfields_third() { bindings::bitfields::ItemKind::ITEM_KIND_TRES) }); } + +impl Drop for bindings::AutoRestoreBool { + fn drop(&mut self) { + unsafe { bindings::AutoRestoreBool::destruct(self) } + } +} + +#[test] +fn test_destructors() { + let mut v = true; + + { + let auto_restore = unsafe { bindings::AutoRestoreBool::new(&mut v) }; + v = false; + } + + assert!(v, "Should've been restored when going out of scope"); +} From a4e606bff0d3a04d24b6109ac94aae5b7d93e102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 3 Apr 2017 13:43:57 +0200 Subject: [PATCH 08/10] codegen: Simplify method implementations. --- src/codegen/mod.rs | 6 +----- tests/expectations/tests/bitfield-method-same-name.rs | 6 +++--- tests/expectations/tests/class.rs | 6 +++--- tests/expectations/tests/class_with_typedef.rs | 8 ++++---- tests/expectations/tests/gen-destructors.rs | 2 +- tests/expectations/tests/issue-410.rs | 2 +- tests/expectations/tests/method-mangling.rs | 4 +--- tests/expectations/tests/namespace.rs | 2 +- tests/expectations/tests/public-dtor.rs | 4 +--- tests/expectations/tests/union_dtor.rs | 2 +- 10 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 72c03c3746..5586c146d3 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1821,11 +1821,7 @@ impl MethodCodegen for Method { exprs[0] = quote_expr!(ctx.ext_cx(), &mut __bindgen_tmp); } else if !self.is_static() { assert!(!exprs.is_empty()); - exprs[0] = if self.is_const() { - quote_expr!(ctx.ext_cx(), &*self) - } else { - quote_expr!(ctx.ext_cx(), &mut *self) - }; + exprs[0] = quote_expr!(ctx.ext_cx(), self); }; let call = aster::expr::ExprBuilder::new() diff --git a/tests/expectations/tests/bitfield-method-same-name.rs b/tests/expectations/tests/bitfield-method-same-name.rs index 9222f74d11..e180153b79 100644 --- a/tests/expectations/tests/bitfield-method-same-name.rs +++ b/tests/expectations/tests/bitfield-method-same-name.rs @@ -54,14 +54,14 @@ impl Foo { } #[inline] pub unsafe fn type_(&mut self) -> ::std::os::raw::c_schar { - Foo_type(&mut *self) + Foo_type(self) } #[inline] pub unsafe fn set_type_(&mut self, c: ::std::os::raw::c_schar) { - Foo_set_type_(&mut *self, c) + Foo_set_type_(self, c) } #[inline] pub unsafe fn set_type(&mut self, c: ::std::os::raw::c_schar) { - Foo_set_type(&mut *self, c) + Foo_set_type(self, c) } } diff --git a/tests/expectations/tests/class.rs b/tests/expectations/tests/class.rs index 3ed5edd243..edb2697b52 100644 --- a/tests/expectations/tests/class.rs +++ b/tests/expectations/tests/class.rs @@ -283,14 +283,14 @@ impl Clone for RealAbstractionWithTonsOfMethods { } impl RealAbstractionWithTonsOfMethods { #[inline] - pub unsafe fn bar(&self) { RealAbstractionWithTonsOfMethods_bar(&*self) } + pub unsafe fn bar(&self) { RealAbstractionWithTonsOfMethods_bar(self) } #[inline] pub unsafe fn bar1(&mut self) { - RealAbstractionWithTonsOfMethods_bar1(&mut *self) + RealAbstractionWithTonsOfMethods_bar1(self) } #[inline] pub unsafe fn bar2(&mut self, foo: ::std::os::raw::c_int) { - RealAbstractionWithTonsOfMethods_bar2(&mut *self, foo) + RealAbstractionWithTonsOfMethods_bar2(self, foo) } #[inline] pub unsafe fn sta() { RealAbstractionWithTonsOfMethods_sta() } diff --git a/tests/expectations/tests/class_with_typedef.rs b/tests/expectations/tests/class_with_typedef.rs index 03822233ff..e962992dc9 100644 --- a/tests/expectations/tests/class_with_typedef.rs +++ b/tests/expectations/tests/class_with_typedef.rs @@ -70,18 +70,18 @@ impl Default for C { } impl C { #[inline] - pub unsafe fn method(&mut self, c: C_MyInt) { C_method(&mut *self, c) } + pub unsafe fn method(&mut self, c: C_MyInt) { C_method(self, c) } #[inline] pub unsafe fn methodRef(&mut self, c: *mut C_MyInt) { - C_methodRef(&mut *self, c) + C_methodRef(self, c) } #[inline] pub unsafe fn complexMethodRef(&mut self, c: *mut C_Lookup) { - C_complexMethodRef(&mut *self, c) + C_complexMethodRef(self, c) } #[inline] pub unsafe fn anotherMethod(&mut self, c: AnotherInt) { - C_anotherMethod(&mut *self, c) + C_anotherMethod(self, c) } } #[repr(C)] diff --git a/tests/expectations/tests/gen-destructors.rs b/tests/expectations/tests/gen-destructors.rs index 4bbc839391..7ec13b03d6 100644 --- a/tests/expectations/tests/gen-destructors.rs +++ b/tests/expectations/tests/gen-destructors.rs @@ -27,5 +27,5 @@ extern "C" { } impl Foo { #[inline] - pub unsafe fn destruct(&mut self) { Foo_Foo_destructor(&mut *self) } + pub unsafe fn destruct(&mut self) { Foo_Foo_destructor(self) } } diff --git a/tests/expectations/tests/issue-410.rs b/tests/expectations/tests/issue-410.rs index 1f624fec7b..8833ef4e51 100644 --- a/tests/expectations/tests/issue-410.rs +++ b/tests/expectations/tests/issue-410.rs @@ -34,7 +34,7 @@ pub mod root { impl Value { #[inline] pub unsafe fn a(&mut self, arg1: root::JSWhyMagic) { - Value_a(&mut *self, arg1) + Value_a(self, arg1) } } } diff --git a/tests/expectations/tests/method-mangling.rs b/tests/expectations/tests/method-mangling.rs index 23f1228018..360f0ea7de 100644 --- a/tests/expectations/tests/method-mangling.rs +++ b/tests/expectations/tests/method-mangling.rs @@ -25,7 +25,5 @@ impl Clone for Foo { } impl Foo { #[inline] - pub unsafe fn type_(&mut self) -> ::std::os::raw::c_int { - Foo_type(&mut *self) - } + pub unsafe fn type_(&mut self) -> ::std::os::raw::c_int { Foo_type(self) } } diff --git a/tests/expectations/tests/namespace.rs b/tests/expectations/tests/namespace.rs index 21b5a58b7a..8e64fa2237 100644 --- a/tests/expectations/tests/namespace.rs +++ b/tests/expectations/tests/namespace.rs @@ -58,7 +58,7 @@ pub mod root { #[inline] pub unsafe fn lets_hope_this_works(&mut self) -> ::std::os::raw::c_int { - A_lets_hope_this_works(&mut *self) + A_lets_hope_this_works(self) } } } diff --git a/tests/expectations/tests/public-dtor.rs b/tests/expectations/tests/public-dtor.rs index 519cdf6734..d24e863e4a 100644 --- a/tests/expectations/tests/public-dtor.rs +++ b/tests/expectations/tests/public-dtor.rs @@ -22,7 +22,5 @@ extern "C" { } impl cv_String { #[inline] - pub unsafe fn destruct(&mut self) { - cv_String_String_destructor(&mut *self) - } + pub unsafe fn destruct(&mut self) { cv_String_String_destructor(self) } } diff --git a/tests/expectations/tests/union_dtor.rs b/tests/expectations/tests/union_dtor.rs index af9a8c041b..61fb0380f7 100644 --- a/tests/expectations/tests/union_dtor.rs +++ b/tests/expectations/tests/union_dtor.rs @@ -59,6 +59,6 @@ extern "C" { impl UnionWithDtor { #[inline] pub unsafe fn destruct(&mut self) { - UnionWithDtor_UnionWithDtor_destructor(&mut *self) + UnionWithDtor_UnionWithDtor_destructor(self) } } From 0dc7bcdbbad5d4e99310c213d261aed2c53c4863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 4 Apr 2017 01:04:46 +0200 Subject: [PATCH 09/10] tests: Add negative tests for constructor/destructor generation behavior. --- .../tests/gen-constructors-neg.rs | 21 +++++++++++++++++ .../expectations/tests/gen-destructors-neg.rs | 23 +++++++++++++++++++ tests/headers/gen-constructors-neg.hpp | 6 +++++ tests/headers/gen-destructors-neg.hpp | 9 ++++++++ 4 files changed, 59 insertions(+) create mode 100644 tests/expectations/tests/gen-constructors-neg.rs create mode 100644 tests/expectations/tests/gen-destructors-neg.rs create mode 100644 tests/headers/gen-constructors-neg.hpp create mode 100644 tests/headers/gen-destructors-neg.hpp diff --git a/tests/expectations/tests/gen-constructors-neg.rs b/tests/expectations/tests/gen-constructors-neg.rs new file mode 100644 index 0000000000..fbeb3d5e52 --- /dev/null +++ b/tests/expectations/tests/gen-constructors-neg.rs @@ -0,0 +1,21 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Default, Copy)] +pub struct Foo { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!(::std::mem::size_of::() , 1usize , concat ! ( + "Size of: " , stringify ! ( Foo ) )); + assert_eq! (::std::mem::align_of::() , 1usize , concat ! ( + "Alignment of " , stringify ! ( Foo ) )); +} +impl Clone for Foo { + fn clone(&self) -> Self { *self } +} diff --git a/tests/expectations/tests/gen-destructors-neg.rs b/tests/expectations/tests/gen-destructors-neg.rs new file mode 100644 index 0000000000..4aaec83a39 --- /dev/null +++ b/tests/expectations/tests/gen-destructors-neg.rs @@ -0,0 +1,23 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Default)] +pub struct Foo { + pub bar: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!(::std::mem::size_of::() , 4usize , concat ! ( + "Size of: " , stringify ! ( Foo ) )); + assert_eq! (::std::mem::align_of::() , 4usize , concat ! ( + "Alignment of " , stringify ! ( Foo ) )); + assert_eq! (unsafe { + & ( * ( 0 as * const Foo ) ) . bar as * const _ as usize } , + 0usize , concat ! ( + "Alignment of field: " , stringify ! ( Foo ) , "::" , + stringify ! ( bar ) )); +} diff --git a/tests/headers/gen-constructors-neg.hpp b/tests/headers/gen-constructors-neg.hpp new file mode 100644 index 0000000000..2dd491c4fd --- /dev/null +++ b/tests/headers/gen-constructors-neg.hpp @@ -0,0 +1,6 @@ +// bindgen-flags: --generate types,functions + +class Foo { + public: + Foo(int a); +}; diff --git a/tests/headers/gen-destructors-neg.hpp b/tests/headers/gen-destructors-neg.hpp new file mode 100644 index 0000000000..5ede3ba357 --- /dev/null +++ b/tests/headers/gen-destructors-neg.hpp @@ -0,0 +1,9 @@ +// bindgen-flags: --generate types,functions +// +// NB: This is intended to _not_ generate destructors. + +class Foo { + int bar; + public: + ~Foo(); +}; From 1f53966ee6b872b7443b335a84d9cf1b57394f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 4 Apr 2017 01:10:45 +0200 Subject: [PATCH 10/10] ir: Add a note about cpp_demangle. --- src/ir/function.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ir/function.rs b/src/ir/function.rs index f80a573615..0809b3c28e 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -156,6 +156,9 @@ pub fn cursor_mangling(ctx: &BindgenContext, // FIXME(emilio): Can a legit symbol in other ABIs end with this string? // I don't think so, but if it can this would become a linker error // anyway, not an invalid free at runtime. + // + // TODO(emilio, #611): Use cpp_demangle if this becomes nastier with + // time. if mangling.ends_with("D0Ev") { let new_len = mangling.len() - 4; mangling.truncate(new_len);