From 414442638a913bf5d924f9b16b2ae417cde5840b Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 10 Mar 2017 10:46:50 -0800 Subject: [PATCH 1/2] Allow anonymous template types We have various assertions that the only way that some template parameter related methods will return `None` is if the template definition is marked opaque. These assertions fail in the presence of test cases with unnamed template types, because we never save an IR item for the template type, and subsequently think that the template definition has no template parameters. The assertions are in fact sound and correct, so it doesn't make sense to remove them. Instead it is more correct to save IR items for the anonymous template types and simply let the template usage analysis prevent them from getting codegen'd. Fixes #574 --- src/ir/comp.rs | 8 ---- src/ir/item.rs | 16 +++++++- src/ir/ty.rs | 14 +++---- .../tests/anonymous-template-types.rs | 33 ++++++++++++++++ .../bad-namespace-parenthood-inheritance.rs | 10 ++--- tests/expectations/tests/issue-358.rs | 5 +-- .../tests/issue-544-stylo-creduce.rs | 5 +-- ...ate-params-causing-layout-test-failures.rs | 2 +- .../issue-574-assertion-failure-in-codegen.rs | 38 +++++++++++++++++++ tests/headers/anonymous-template-types.hpp | 24 ++++++++++++ ...issue-574-assertion-failure-in-codegen.hpp | 6 +++ 11 files changed, 129 insertions(+), 32 deletions(-) create mode 100644 tests/expectations/tests/anonymous-template-types.rs create mode 100644 tests/expectations/tests/issue-574-assertion-failure-in-codegen.rs create mode 100644 tests/headers/anonymous-template-types.hpp create mode 100644 tests/headers/issue-574-assertion-failure-in-codegen.hpp diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 814204c268..9a6548a7ef 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -597,14 +597,6 @@ impl CompInfo { ci.packed = true; } CXCursor_TemplateTypeParameter => { - // Yes! You can arrive here with an empty template parameter - // name! Awesome, isn't it? - // - // see tests/headers/empty_template_param_name.hpp - if cur.spelling().is_empty() { - return CXChildVisit_Continue; - } - let param = Item::named_type(None, cur, ctx) .expect("Item::named_type should't fail when pointing \ at a TemplateTypeParameter"); diff --git a/src/ir/item.rs b/src/ir/item.rs index 93df9c7730..89422e874d 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -19,6 +19,7 @@ use std::collections::BTreeSet; use std::fmt::Write; use std::io; use std::iter; +use regex; /// A trait to get the canonical name from an item. /// @@ -1313,8 +1314,19 @@ impl ClangItemParser for Item { fn is_template_with_spelling(refd: &clang::Cursor, spelling: &str) -> bool { - refd.kind() == clang_sys::CXCursor_TemplateTypeParameter && - refd.spelling() == spelling + lazy_static! { + static ref ANON_TYPE_PARAM_RE: regex::Regex = + regex::Regex::new(r"^type\-parameter\-\d+\-\d+$").unwrap(); + } + + if refd.kind() != clang_sys::CXCursor_TemplateTypeParameter { + return false; + } + + let refd_spelling = refd.spelling(); + refd_spelling == spelling || + // Allow for anonymous template parameters. + (refd_spelling.is_empty() && ANON_TYPE_PARAM_RE.is_match(spelling.as_ref())) } let definition = if is_template_with_spelling(&location, diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 6c8e736355..5492569680 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -292,8 +292,12 @@ impl Type { /// Creates a new named type, with name `name`. pub fn named(name: String) -> Self { - assert!(!name.is_empty()); - Self::new(Some(name), None, TypeKind::Named, false) + let name = if name.is_empty() { + None + } else { + Some(name) + }; + Self::new(name, None, TypeKind::Named, false) } /// Is this a floating point type? @@ -1127,12 +1131,6 @@ impl Type { ctx); } CXCursor_TemplateTypeParameter => { - // See the comment in src/ir/comp.rs - // about the same situation. - if cur.spelling().is_empty() { - return CXChildVisit_Continue; - } - let param = Item::named_type(None, cur, diff --git a/tests/expectations/tests/anonymous-template-types.rs b/tests/expectations/tests/anonymous-template-types.rs new file mode 100644 index 0000000000..bb4be1055a --- /dev/null +++ b/tests/expectations/tests/anonymous-template-types.rs @@ -0,0 +1,33 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Foo { + pub t_member: T, +} +impl Default for Foo { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Bar { + pub member: ::std::os::raw::c_schar, +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Quux { + pub v_member: V, +} +impl Default for Quux { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Lobo { + pub also_member: ::std::os::raw::c_schar, +} +pub type AliasWithAnonType = ::std::os::raw::c_schar; diff --git a/tests/expectations/tests/bad-namespace-parenthood-inheritance.rs b/tests/expectations/tests/bad-namespace-parenthood-inheritance.rs index 4074dd02eb..9ff4af1f06 100644 --- a/tests/expectations/tests/bad-namespace-parenthood-inheritance.rs +++ b/tests/expectations/tests/bad-namespace-parenthood-inheritance.rs @@ -5,15 +5,15 @@ #[repr(C)] -#[derive(Debug, Default, Copy, Clone)] +#[derive(Debug, Copy, Clone)] pub struct std_char_traits { pub _address: u8, } +impl Default for std_char_traits { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} #[repr(C)] -#[derive(Debug, Default, Copy)] +#[derive(Debug, Default, Copy, Clone)] pub struct __gnu_cxx_char_traits { pub _address: u8, } -impl Clone for __gnu_cxx_char_traits { - fn clone(&self) -> Self { *self } -} diff --git a/tests/expectations/tests/issue-358.rs b/tests/expectations/tests/issue-358.rs index e574bd017e..d01ae8007a 100644 --- a/tests/expectations/tests/issue-358.rs +++ b/tests/expectations/tests/issue-358.rs @@ -13,13 +13,10 @@ impl Default for JS_PersistentRooted { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } #[repr(C)] -#[derive(Debug, Copy)] +#[derive(Debug, Copy, Clone)] pub struct a { pub b: *mut a, } -impl Clone for a { - fn clone(&self) -> Self { *self } -} impl Default for a { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } diff --git a/tests/expectations/tests/issue-544-stylo-creduce.rs b/tests/expectations/tests/issue-544-stylo-creduce.rs index 88cc0d87d0..fb543cba63 100644 --- a/tests/expectations/tests/issue-544-stylo-creduce.rs +++ b/tests/expectations/tests/issue-544-stylo-creduce.rs @@ -5,10 +5,7 @@ #[repr(C)] -#[derive(Debug, Default, Copy)] +#[derive(Debug, Default, Copy, Clone)] pub struct a { pub _address: u8, } -impl Clone for a { - fn clone(&self) -> Self { *self } -} diff --git a/tests/expectations/tests/issue-569-non-type-template-params-causing-layout-test-failures.rs b/tests/expectations/tests/issue-569-non-type-template-params-causing-layout-test-failures.rs index 6cf2ba16c6..c4ce513902 100644 --- a/tests/expectations/tests/issue-569-non-type-template-params-causing-layout-test-failures.rs +++ b/tests/expectations/tests/issue-569-non-type-template-params-causing-layout-test-failures.rs @@ -32,7 +32,7 @@ impl Default for JS_AutoIdVector { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } #[test] -fn __bindgen_test_layout_JS_Base_instantiation_16() { +fn __bindgen_test_layout_JS_Base_instantiation_20() { assert_eq!(::std::mem::size_of::() , 1usize , concat ! ( "Size of template specialization: " , stringify ! ( JS_Base ) )); diff --git a/tests/expectations/tests/issue-574-assertion-failure-in-codegen.rs b/tests/expectations/tests/issue-574-assertion-failure-in-codegen.rs new file mode 100644 index 0000000000..084cdb62c9 --- /dev/null +++ b/tests/expectations/tests/issue-574-assertion-failure-in-codegen.rs @@ -0,0 +1,38 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct a { + pub _address: u8, +} +#[repr(C)] +#[derive(Debug, Copy)] +pub struct _bindgen_ty_1 { + pub ar: a, +} +#[test] +fn bindgen_test_layout__bindgen_ty_1() { + assert_eq!(::std::mem::size_of::<_bindgen_ty_1>() , 1usize , concat ! ( + "Size of: " , stringify ! ( _bindgen_ty_1 ) )); + assert_eq! (::std::mem::align_of::<_bindgen_ty_1>() , 1usize , concat ! ( + "Alignment of " , stringify ! ( _bindgen_ty_1 ) )); + assert_eq! (unsafe { + & ( * ( 0 as * const _bindgen_ty_1 ) ) . ar as * const _ as + usize } , 0usize , concat ! ( + "Alignment of field: " , stringify ! ( _bindgen_ty_1 ) , "::" + , stringify ! ( ar ) )); +} +impl Clone for _bindgen_ty_1 { + fn clone(&self) -> Self { *self } +} +impl Default for _bindgen_ty_1 { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +extern "C" { + #[link_name = "AutoIdVector"] + pub static mut AutoIdVector: _bindgen_ty_1; +} diff --git a/tests/headers/anonymous-template-types.hpp b/tests/headers/anonymous-template-types.hpp new file mode 100644 index 0000000000..9ada71a908 --- /dev/null +++ b/tests/headers/anonymous-template-types.hpp @@ -0,0 +1,24 @@ +// bindgen-flags: -- -std=c++14 + +template +struct Foo { + T t_member; +}; + +template +struct Bar { + char member; +}; + +template +struct Quux { + V v_member; +}; + +template +struct Lobo { + char also_member; +}; + +template +using AliasWithAnonType = char; diff --git a/tests/headers/issue-574-assertion-failure-in-codegen.hpp b/tests/headers/issue-574-assertion-failure-in-codegen.hpp new file mode 100644 index 0000000000..b563b4efca --- /dev/null +++ b/tests/headers/issue-574-assertion-failure-in-codegen.hpp @@ -0,0 +1,6 @@ +// bindgen-flags: -- -std=c++14 + +template class a {}; +class { + a ar; +} AutoIdVector; From b7f7850568c9d8f70af8d382d29c00252648e9cc Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 10 Mar 2017 13:53:26 -0800 Subject: [PATCH 2/2] Properly align function arguments Not sure why rustfmt hasn't caught this before... --- src/ir/context.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ir/context.rs b/src/ir/context.rs index 6795a864e7..1dc4f4a4c1 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -1032,10 +1032,10 @@ impl<'ctx> BindgenContext<'ctx> { } return self.instantiate_template(with_id, - id, - parent_id, - ty, - location) + id, + parent_id, + ty, + location) .or_else(|| Some(id)); }