Skip to content

Commit

Permalink
Auto merge of #823 - fitzgen:issue-820-unused-template-param-in-alias…
Browse files Browse the repository at this point in the history
…, r=emilio

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

r? @emilio
  • Loading branch information
bors-servo committed Jul 20, 2017
2 parents 6053d99 + 20253fc commit 781385f
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 8 deletions.
20 changes: 15 additions & 5 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -1442,7 +1450,7 @@ impl CanDeriveDebug for CompInfo {
layout: Option<Layout>)
-> 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,
Expand Down Expand Up @@ -1498,9 +1506,11 @@ 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);
Expand Down Expand Up @@ -1528,7 +1538,7 @@ impl<'a> CanDeriveCopy<'a> for CompInfo {
(item, layout): (&Item, Option<Layout>))
-> 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
Expand Down
13 changes: 11 additions & 2 deletions src/ir/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,14 +361,23 @@ impl CanDeriveDebug for TemplateInstantiation {
layout: Option<Layout>)
-> 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
Expand Down
4 changes: 4 additions & 0 deletions src/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
}
}
Expand Down
4 changes: 4 additions & 0 deletions tests/expectations/tests/issue-372.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub mod root {
ai = 11,
}
#[repr(C)]
#[derive(Copy)]
pub struct F {
pub w: [u64; 33usize],
}
Expand All @@ -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() } }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ 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,
}
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,
}
Expand All @@ -28,6 +30,9 @@ fn bindgen_test_layout_JS_AutoIdVector() {
assert_eq! (::std::mem::align_of::<JS_AutoIdVector>() , 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() } }
}
Expand Down
9 changes: 8 additions & 1 deletion tests/expectations/tests/issue-573-layout-test-failures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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() } }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
4 changes: 4 additions & 0 deletions tests/expectations/tests/non-type-params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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() } }
}
Expand Down
4 changes: 4 additions & 0 deletions tests/expectations/tests/size_t_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@


#[repr(C)]
#[derive(Debug, Copy)]
pub struct C {
pub arr: [u32; 3usize],
}
Expand All @@ -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() } }
}
Expand Down
5 changes: 5 additions & 0 deletions tests/headers/issue-820-unused-template-param-in-alias.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
template<typename E, int N>
class Foo {
typedef Foo<E, N> self_type;
E mBar;
};

0 comments on commit 781385f

Please sign in to comment.