Skip to content

Commit df043bf

Browse files
author
bors-servo
authored
Auto merge of #383 - emilio:virtual-base, r=fitzgen
Tidy up and test virtual inheritance handling. Done while investigating #380. r? @fitzgen
2 parents c77e76c + 12fd256 commit df043bf

File tree

4 files changed

+159
-28
lines changed

4 files changed

+159
-28
lines changed

‎libbindgen/src/codegen/mod.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod helpers;
44
use aster;
55

66
use ir::annotations::FieldAccessorKind;
7-
use ir::comp::{CompInfo, CompKind, Field, Method, MethodKind};
7+
use ir::comp::{Base, CompInfo, CompKind, Field, Method, MethodKind};
88
use ir::context::{BindgenContext, ItemId};
99
use ir::derive::{CanDeriveCopy, CanDeriveDebug};
1010
use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue};
@@ -556,13 +556,13 @@ struct Vtable<'a> {
556556
#[allow(dead_code)]
557557
methods: &'a [Method],
558558
#[allow(dead_code)]
559-
base_classes: &'a [ItemId],
559+
base_classes: &'a [Base],
560560
}
561561

562562
impl<'a> Vtable<'a> {
563563
fn new(item_id: ItemId,
564564
methods: &'a [Method],
565-
base_classes: &'a [ItemId])
565+
base_classes: &'a [Base])
566566
-> Self {
567567
Vtable {
568568
item_id: item_id,
@@ -835,14 +835,18 @@ impl CodeGenerator for CompInfo {
835835
}
836836

837837
for (i, base) in self.base_members().iter().enumerate() {
838-
let base_ty = ctx.resolve_type(*base);
838+
// Virtual bases are already taken into account by the vtable
839+
// pointer.
840+
//
841+
// FIXME(emilio): Is this always right?
842+
if base.is_virtual() {
843+
continue;
844+
}
845+
846+
let base_ty = ctx.resolve_type(base.ty);
839847
// NB: We won't include unsized types in our base chain because they
840848
// would contribute to our size given the dummy field we insert for
841849
// unsized types.
842-
//
843-
// NB: Canonical type is here because it could be inheriting from a
844-
// typedef, for example, and the lack of `unwrap()` is because we
845-
// can inherit from a template parameter, yes.
846850
if base_ty.is_unsized(ctx) {
847851
continue;
848852
}
@@ -854,7 +858,7 @@ impl CodeGenerator for CompInfo {
854858
}
855859
}
856860

857-
let inner = base.to_rust_ty(ctx);
861+
let inner = base.ty.to_rust_ty(ctx);
858862
let field_name = if i == 0 {
859863
"_base".into()
860864
} else {

‎libbindgen/src/ir/comp.rs

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,40 @@ impl<'a> CanDeriveCopy<'a> for Field {
174174
}
175175
}
176176

177+
178+
/// The kind of inheritance a base class is using.
179+
#[derive(Clone, Debug, PartialEq, Eq)]
180+
pub enum BaseKind {
181+
/// Normal inheritance, like:
182+
///
183+
/// ```cpp
184+
/// class A : public B {};
185+
/// ```
186+
Normal,
187+
/// Virtual inheritance, like:
188+
///
189+
/// ```cpp
190+
/// class A: public virtual B {};
191+
/// ```
192+
Virtual,
193+
}
194+
195+
/// A base class.
196+
#[derive(Clone, Debug)]
197+
pub struct Base {
198+
/// The type of this base class.
199+
pub ty: ItemId,
200+
/// The kind of inheritance we're doing.
201+
pub kind: BaseKind,
202+
}
203+
204+
impl Base {
205+
/// Whether this base class is inheriting virtually.
206+
pub fn is_virtual(&self) -> bool {
207+
self.kind == BaseKind::Virtual
208+
}
209+
}
210+
177211
/// A compound type.
178212
///
179213
/// Either a struct or union, a compound type is built up from the combination
@@ -199,7 +233,7 @@ pub struct CompInfo {
199233
constructors: Vec<ItemId>,
200234

201235
/// Vector of classes this one inherits from.
202-
base_members: Vec<ItemId>,
236+
base_members: Vec<Base>,
203237

204238
/// The parent reference template if any.
205239
ref_template: Option<ItemId>,
@@ -287,7 +321,7 @@ impl CompInfo {
287321
pub fn is_unsized(&self, ctx: &BindgenContext) -> bool {
288322
!self.has_vtable(ctx) && self.fields.is_empty() &&
289323
self.base_members.iter().all(|base| {
290-
ctx.resolve_type(*base).canonical_type(ctx).is_unsized(ctx)
324+
ctx.resolve_type(base.ty).canonical_type(ctx).is_unsized(ctx)
291325
}) &&
292326
self.ref_template
293327
.map_or(true, |template| ctx.resolve_type(template).is_unsized(ctx))
@@ -314,12 +348,12 @@ impl CompInfo {
314348
self.ref_template.as_ref().map_or(false, |t| {
315349
ctx.resolve_type(*t).has_destructor(ctx)
316350
}) ||
317-
self.template_args
318-
.iter()
319-
.any(|t| ctx.resolve_type(*t).has_destructor(ctx)) ||
320-
self.base_members
321-
.iter()
322-
.any(|t| ctx.resolve_type(*t).has_destructor(ctx)) ||
351+
self.template_args.iter().any(|t| {
352+
ctx.resolve_type(*t).has_destructor(ctx)
353+
}) ||
354+
self.base_members.iter().any(|base| {
355+
ctx.resolve_type(base.ty).has_destructor(ctx)
356+
}) ||
323357
self.fields.iter().any(|field| {
324358
ctx.resolve_type(field.ty)
325359
.has_destructor(ctx)
@@ -394,7 +428,7 @@ impl CompInfo {
394428
pub fn has_vtable(&self, ctx: &BindgenContext) -> bool {
395429
self.has_vtable ||
396430
self.base_members().iter().any(|base| {
397-
ctx.resolve_type(*base)
431+
ctx.resolve_type(base.ty)
398432
.has_vtable(ctx)
399433
}) ||
400434
self.ref_template.map_or(false, |template| {
@@ -418,7 +452,7 @@ impl CompInfo {
418452
}
419453

420454
/// The set of types that this one inherits from.
421-
pub fn base_members(&self) -> &[ItemId] {
455+
pub fn base_members(&self) -> &[Base] {
422456
&self.base_members
423457
}
424458

@@ -590,14 +624,23 @@ impl CompInfo {
590624
ci.template_args.push(param);
591625
}
592626
CXCursor_CXXBaseSpecifier => {
593-
if !ci.has_vtable {
594-
ci.has_vtable = cur.is_virtual_base();
595-
}
627+
let is_virtual_base = cur.is_virtual_base();
628+
ci.has_vtable |= is_virtual_base;
629+
630+
let kind = if is_virtual_base {
631+
BaseKind::Virtual
632+
} else {
633+
BaseKind::Normal
634+
};
635+
596636
let type_id = Item::from_ty_or_ref(cur.cur_type(),
597637
Some(cur),
598638
None,
599639
ctx);
600-
ci.base_members.push(type_id);
640+
ci.base_members.push(Base {
641+
ty: type_id,
642+
kind: kind,
643+
});
601644
}
602645
CXCursor_Constructor |
603646
CXCursor_Destructor |
@@ -794,7 +837,7 @@ impl CompInfo {
794837
// Unfortunately, given the way we implement --match-pat, and also
795838
// that you can inherit from templated types, we need to handle
796839
// other cases here too.
797-
ctx.resolve_type(*base)
840+
ctx.resolve_type(base.ty)
798841
.canonical_type(ctx)
799842
.as_comp()
800843
.map_or(false, |ci| ci.has_vtable(ctx))
@@ -835,7 +878,7 @@ impl CanDeriveDebug for CompInfo {
835878
let can_derive_debug = {
836879
self.base_members
837880
.iter()
838-
.all(|id| id.can_derive_debug(ctx, ())) &&
881+
.all(|base| base.ty.can_derive_debug(ctx, ())) &&
839882
self.template_args
840883
.iter()
841884
.all(|id| id.can_derive_debug(ctx, ())) &&
@@ -888,7 +931,7 @@ impl<'a> CanDeriveCopy<'a> for CompInfo {
888931
.map_or(true, |t| t.can_derive_copy(ctx, ())) &&
889932
self.base_members
890933
.iter()
891-
.all(|t| t.can_derive_copy(ctx, ())) &&
934+
.all(|base| base.ty.can_derive_copy(ctx, ())) &&
892935
self.fields.iter().all(|field| field.can_derive_copy(ctx, ()))
893936
}
894937

@@ -916,8 +959,8 @@ impl TypeCollector for CompInfo {
916959
types.insert(arg);
917960
}
918961

919-
for &base in self.base_members() {
920-
types.insert(base);
962+
for base in self.base_members() {
963+
types.insert(base.ty);
921964
}
922965

923966
for field in self.fields() {

‎libbindgen/tests/expectations/tests/virtual_inheritance.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/* automatically generated by rust-bindgen */
2+
3+
4+
#![allow(non_snake_case)]
5+
6+
7+
#[repr(C)]
8+
#[derive(Debug, Copy)]
9+
pub struct A {
10+
pub foo: ::std::os::raw::c_int,
11+
}
12+
#[test]
13+
fn bindgen_test_layout_A() {
14+
assert_eq!(::std::mem::size_of::<A>() , 4usize);
15+
assert_eq!(::std::mem::align_of::<A>() , 4usize);
16+
}
17+
impl Clone for A {
18+
fn clone(&self) -> Self { *self }
19+
}
20+
#[repr(C)]
21+
pub struct B__bindgen_vtable {
22+
}
23+
#[repr(C)]
24+
#[derive(Debug, Copy)]
25+
pub struct B {
26+
pub vtable_: *const B__bindgen_vtable,
27+
pub bar: ::std::os::raw::c_int,
28+
}
29+
#[test]
30+
fn bindgen_test_layout_B() {
31+
assert_eq!(::std::mem::size_of::<B>() , 16usize);
32+
assert_eq!(::std::mem::align_of::<B>() , 8usize);
33+
}
34+
impl Clone for B {
35+
fn clone(&self) -> Self { *self }
36+
}
37+
#[repr(C)]
38+
pub struct C__bindgen_vtable {
39+
}
40+
#[repr(C)]
41+
#[derive(Debug, Copy)]
42+
pub struct C {
43+
pub vtable_: *const C__bindgen_vtable,
44+
pub baz: ::std::os::raw::c_int,
45+
}
46+
#[test]
47+
fn bindgen_test_layout_C() {
48+
assert_eq!(::std::mem::size_of::<C>() , 16usize);
49+
assert_eq!(::std::mem::align_of::<C>() , 8usize);
50+
}
51+
impl Clone for C {
52+
fn clone(&self) -> Self { *self }
53+
}
54+
#[repr(C)]
55+
#[derive(Debug, Copy)]
56+
pub struct D {
57+
pub _base: C,
58+
pub _base_1: B,
59+
pub bazz: ::std::os::raw::c_int,
60+
}
61+
#[test]
62+
fn bindgen_test_layout_D() {
63+
assert_eq!(::std::mem::size_of::<D>() , 40usize);
64+
assert_eq!(::std::mem::align_of::<D>() , 8usize);
65+
}
66+
impl Clone for D {
67+
fn clone(&self) -> Self { *self }
68+
}

‎libbindgen/tests/headers/virtual_inheritance.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
class A {
3+
int foo;
4+
};
5+
6+
class B: public virtual A {
7+
int bar;
8+
};
9+
10+
class C: public virtual A {
11+
int baz;
12+
};
13+
14+
class D: public C, public B {
15+
int bazz;
16+
};

0 commit comments

Comments
 (0)