From e42597173ba2aa1b88a0c35993a80299b5b833b5 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 18 Jul 2017 10:42:39 -0700 Subject: [PATCH] Implement `IsOpaque` for `CompInfo` This allows us to properly detect structs that should be treated as opaque due to their non-type template paramaters, which in turn lets us correctly codegen template aliases to such things. Fixes #820 --- src/ir/comp.rs | 21 ++++++++++++++----- src/ir/template.rs | 13 ++++++++++-- src/ir/ty.rs | 4 ++++ tests/expectations/tests/issue-372.rs | 4 ++++ ...ate-params-causing-layout-test-failures.rs | 5 +++++ .../tests/issue-573-layout-test-failures.rs | 9 +++++++- ...ssue-820-unused-template-param-in-alias.rs | 7 +++++++ tests/expectations/tests/non-type-params.rs | 4 ++++ tests/expectations/tests/size_t_template.rs | 4 ++++ ...sue-820-unused-template-param-in-alias.hpp | 5 +++++ 10 files changed, 68 insertions(+), 8 deletions(-) create mode 100644 tests/expectations/tests/issue-820-unused-template-param-in-alias.rs create mode 100644 tests/headers/issue-820-unused-template-param-in-alias.hpp diff --git a/src/ir/comp.rs b/src/ir/comp.rs index bc2d675d7e..2334bb8322 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -1422,6 +1422,14 @@ impl DotAttributes for CompInfo { } } +impl IsOpaque for CompInfo { + type Extra = (); + + fn is_opaque(&self, _: &BindgenContext, _: &()) -> bool { + self.has_non_type_template_params + } +} + impl TemplateParameters for CompInfo { fn self_template_params(&self, _ctx: &BindgenContext) @@ -1442,7 +1450,7 @@ impl CanDeriveDebug for CompInfo { layout: Option) -> bool { if self.has_non_type_template_params() { - return layout.map_or(false, |l| l.opaque().can_derive_debug(ctx, ())); + return layout.map_or(true, |l| l.opaque().can_derive_debug(ctx, ())); } // We can reach here recursively via template parameters of a member, @@ -1498,9 +1506,12 @@ impl CanDeriveDefault for CompInfo { return false; } - return layout.unwrap_or_else(Layout::zero) - .opaque() - .can_derive_default(ctx, ()); + return layout.map_or(true, |l| l.opaque().can_derive_default(ctx, ())); + + } + + if self.has_non_type_template_params { + return layout.map_or(true, |l| l.opaque().can_derive_default(ctx, ())); } self.detect_derive_default_cycle.set(true); @@ -1528,7 +1539,7 @@ impl<'a> CanDeriveCopy<'a> for CompInfo { (item, layout): (&Item, Option)) -> bool { if self.has_non_type_template_params() { - return layout.map_or(false, |l| l.opaque().can_derive_copy(ctx, ())); + return layout.map_or(true, |l| l.opaque().can_derive_copy(ctx, ())); } // NOTE: Take into account that while unions in C and C++ are copied by diff --git a/src/ir/template.rs b/src/ir/template.rs index fcba40e17d..a541442777 100644 --- a/src/ir/template.rs +++ b/src/ir/template.rs @@ -361,14 +361,23 @@ impl CanDeriveDebug for TemplateInstantiation { layout: Option) -> bool { self.args.iter().all(|arg| arg.can_derive_debug(ctx, ())) && - ctx.resolve_type(self.definition) + self.definition + .into_resolver() + .through_type_refs() + .through_type_aliases() + .resolve(ctx) + .as_type() + .expect("Instantiation of a non-type?") .as_comp() .and_then(|c| { // For non-type template parameters, we generate an opaque // blob, and in this case the instantiation has a better // idea of the layout than the definition does. if c.has_non_type_template_params() { - let opaque = layout.unwrap_or(Layout::zero()).opaque(); + let opaque = layout + .or_else(|| ctx.resolve_type(self.definition).layout(ctx)) + .unwrap_or(Layout::zero()) + .opaque(); Some(opaque.can_derive_debug(ctx, ())) } else { None diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 5ab7cf082f..82fae6f5e7 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -358,6 +358,7 @@ impl IsOpaque for Type { match self.kind { TypeKind::Opaque => true, TypeKind::TemplateInstantiation(ref inst) => inst.is_opaque(ctx, item), + TypeKind::Comp(ref comp) => comp.is_opaque(ctx, &()), _ => false, } } @@ -562,6 +563,9 @@ impl CanDeriveDebug for Type { TypeKind::TemplateInstantiation(ref inst) => { inst.can_derive_debug(ctx, self.layout(ctx)) } + TypeKind::Opaque => { + self.layout.map_or(true, |l| l.opaque().can_derive_debug(ctx, ())) + } _ => true, } } diff --git a/tests/expectations/tests/issue-372.rs b/tests/expectations/tests/issue-372.rs index 687ae25f75..0e715893db 100644 --- a/tests/expectations/tests/issue-372.rs +++ b/tests/expectations/tests/issue-372.rs @@ -83,6 +83,7 @@ pub mod root { ai = 11, } #[repr(C)] + #[derive(Copy)] pub struct F { pub w: [u64; 33usize], } @@ -98,6 +99,9 @@ pub mod root { "Alignment of field: " , stringify ! ( F ) , "::" , stringify ! ( w ) )); } + impl Clone for F { + fn clone(&self) -> Self { *self } + } impl Default for F { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } 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 1a783f7d49..da345825d0 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 @@ -11,6 +11,7 @@ pub const ENUM_VARIANT_2: _bindgen_ty_1 = _bindgen_ty_1::ENUM_VARIANT_2; pub enum _bindgen_ty_1 { ENUM_VARIANT_1 = 0, ENUM_VARIANT_2 = 1, } pub type JS_Alias = u8; #[repr(C)] +#[derive(Debug, Copy, Clone)] pub struct JS_Base { pub f: JS_Alias, } @@ -18,6 +19,7 @@ impl Default for JS_Base { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } #[repr(C)] +#[derive(Debug, Copy)] pub struct JS_AutoIdVector { pub _base: JS_Base, } @@ -28,6 +30,9 @@ fn bindgen_test_layout_JS_AutoIdVector() { assert_eq! (::std::mem::align_of::() , 1usize , concat ! ( "Alignment of " , stringify ! ( JS_AutoIdVector ) )); } +impl Clone for JS_AutoIdVector { + fn clone(&self) -> Self { *self } +} impl Default for JS_AutoIdVector { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } diff --git a/tests/expectations/tests/issue-573-layout-test-failures.rs b/tests/expectations/tests/issue-573-layout-test-failures.rs index 7d87aabec0..64b7540be0 100644 --- a/tests/expectations/tests/issue-573-layout-test-failures.rs +++ b/tests/expectations/tests/issue-573-layout-test-failures.rs @@ -5,11 +5,15 @@ #[repr(C)] -#[derive(Default)] +#[derive(Debug, Copy, Clone)] pub struct Outer { pub i: u8, } +impl Default for Outer { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} #[repr(C)] +#[derive(Debug, Copy)] pub struct AutoIdVector { pub ar: Outer, } @@ -25,6 +29,9 @@ fn bindgen_test_layout_AutoIdVector() { "Alignment of field: " , stringify ! ( AutoIdVector ) , "::" , stringify ! ( ar ) )); } +impl Clone for AutoIdVector { + fn clone(&self) -> Self { *self } +} impl Default for AutoIdVector { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } diff --git a/tests/expectations/tests/issue-820-unused-template-param-in-alias.rs b/tests/expectations/tests/issue-820-unused-template-param-in-alias.rs new file mode 100644 index 0000000000..23ef513f4a --- /dev/null +++ b/tests/expectations/tests/issue-820-unused-template-param-in-alias.rs @@ -0,0 +1,7 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] + + +pub type Foo_self_type = u8; diff --git a/tests/expectations/tests/non-type-params.rs b/tests/expectations/tests/non-type-params.rs index 34864993ea..7ebd1c41aa 100644 --- a/tests/expectations/tests/non-type-params.rs +++ b/tests/expectations/tests/non-type-params.rs @@ -7,6 +7,7 @@ pub type Array16 = u8; pub type ArrayInt4 = [u32; 4usize]; #[repr(C)] +#[derive(Debug, Copy)] pub struct UsesArray { pub array_char_16: [u8; 16usize], pub array_bool_8: [u8; 8usize], @@ -34,6 +35,9 @@ fn bindgen_test_layout_UsesArray() { "Alignment of field: " , stringify ! ( UsesArray ) , "::" , stringify ! ( array_int_4 ) )); } +impl Clone for UsesArray { + fn clone(&self) -> Self { *self } +} impl Default for UsesArray { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } diff --git a/tests/expectations/tests/size_t_template.rs b/tests/expectations/tests/size_t_template.rs index 47e75edb6e..f61ed34b8c 100644 --- a/tests/expectations/tests/size_t_template.rs +++ b/tests/expectations/tests/size_t_template.rs @@ -5,6 +5,7 @@ #[repr(C)] +#[derive(Debug, Copy)] pub struct C { pub arr: [u32; 3usize], } @@ -20,6 +21,9 @@ fn bindgen_test_layout_C() { "Alignment of field: " , stringify ! ( C ) , "::" , stringify ! ( arr ) )); } +impl Clone for C { + fn clone(&self) -> Self { *self } +} impl Default for C { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } diff --git a/tests/headers/issue-820-unused-template-param-in-alias.hpp b/tests/headers/issue-820-unused-template-param-in-alias.hpp new file mode 100644 index 0000000000..ca5d8b9653 --- /dev/null +++ b/tests/headers/issue-820-unused-template-param-in-alias.hpp @@ -0,0 +1,5 @@ +template +class Foo { + typedef Foo self_type; + E mBar; +};