Skip to content

Commit

Permalink
Auto merge of #1199 - emilio:pure-virtual, r=pepyakin
Browse files Browse the repository at this point in the history
Don't generate symbols for pure virtual functions.

Fixes #1197.
  • Loading branch information
bors-servo authored Dec 29, 2017
2 parents 39d0778 + f4d2d6a commit 04591f0
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 35 deletions.
4 changes: 4 additions & 0 deletions bindgen-integration/cpp/Test.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class Test {
static const int* countdown();
};

class ITest {
virtual void foo() = 0;
};

namespace testing {

typedef Test TypeAlias;
Expand Down
7 changes: 6 additions & 1 deletion src/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,11 +606,16 @@ impl Cursor {
unsafe { clang_CXXMethod_isConst(self.x) != 0 }
}

/// Is this cursor's referent a member function that is declared `const`?
/// Is this cursor's referent a member function that is virtual?
pub fn method_is_virtual(&self) -> bool {
unsafe { clang_CXXMethod_isVirtual(self.x) != 0 }
}

/// Is this cursor's referent a member function that is pure virtual?
pub fn method_is_pure_virtual(&self) -> bool {
unsafe { clang_CXXMethod_isPureVirtual(self.x) != 0 }
}

/// Is this cursor's referent a struct or class with virtual members?
pub fn is_virtual_base(&self) -> bool {
unsafe { clang_isVirtualBase(self.x) != 0 }
Expand Down
24 changes: 14 additions & 10 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use ir::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault,
CanDerivePartialEq, CanDeriveEq, CanDerive};
use ir::dot;
use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue};
use ir::function::{Abi, Function, FunctionSig, Linkage};
use ir::function::{Abi, Function, FunctionKind, FunctionSig, Linkage};
use ir::int::IntKind;
use ir::item::{IsOpaque, Item, ItemCanonicalName, ItemCanonicalPath};
use ir::item_kind::ItemKind;
Expand Down Expand Up @@ -1878,13 +1878,8 @@ impl CodeGenerator for CompInfo {
}

if ctx.options().codegen_config.destructors {
if let Some((is_virtual, destructor)) = self.destructor() {
let kind = if is_virtual {
MethodKind::VirtualDestructor
} else {
MethodKind::Destructor
};

if let Some((kind, destructor)) = self.destructor() {
debug_assert!(kind.is_destructor());
Method::new(kind, destructor, false).codegen_method(
ctx,
&mut methods,
Expand Down Expand Up @@ -1990,9 +1985,9 @@ impl MethodCodegen for Method {
match self.kind() {
MethodKind::Constructor => cc.constructors,
MethodKind::Destructor => cc.destructors,
MethodKind::VirtualDestructor => cc.destructors,
MethodKind::VirtualDestructor { .. } => cc.destructors,
MethodKind::Static | MethodKind::Normal |
MethodKind::Virtual => cc.methods,
MethodKind::Virtual { .. } => cc.methods,
}
});

Expand Down Expand Up @@ -3174,6 +3169,15 @@ impl CodeGenerator for Function {
Linkage::External => {}
}

// Pure virtual methods have no actual symbol, so we can't generate
// something meaningful for them.
match self.kind() {
FunctionKind::Method(ref method_kind) if method_kind.is_pure_virtual() => {
return;
}
_ => {},
}

// Similar to static member variables in a class template, we can't
// generate bindings to template functions, because the set of
// instantiations is open ended and we have no way of knowing which
Expand Down
71 changes: 52 additions & 19 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,40 @@ pub enum MethodKind {
/// A destructor.
Destructor,
/// A virtual destructor.
VirtualDestructor,
VirtualDestructor {
/// Whether it's pure virtual.
pure_virtual: bool,
},
/// A static method.
Static,
/// A normal method.
Normal,
/// A virtual method.
Virtual,
Virtual {
/// Whether it's pure virtual.
pure_virtual: bool,
},
}


impl MethodKind {
/// Is this a destructor method?
pub fn is_destructor(&self) -> bool {
match *self {
MethodKind::Destructor |
MethodKind::VirtualDestructor { .. } => true,
_ => false,
}
}

/// Is this a pure virtual method?
pub fn is_pure_virtual(&self) -> bool {
match *self {
MethodKind::Virtual { pure_virtual } |
MethodKind::VirtualDestructor { pure_virtual } => pure_virtual,
_ => false,
}
}
}

/// A struct representing a C++ method, either static, normal, or virtual.
Expand Down Expand Up @@ -73,21 +100,18 @@ impl Method {
self.kind
}

/// Is this a destructor method?
pub fn is_destructor(&self) -> bool {
self.kind == MethodKind::Destructor ||
self.kind == MethodKind::VirtualDestructor
}

/// Is this a constructor?
pub fn is_constructor(&self) -> bool {
self.kind == MethodKind::Constructor
}

/// Is this a virtual method?
pub fn is_virtual(&self) -> bool {
self.kind == MethodKind::Virtual ||
self.kind == MethodKind::VirtualDestructor
match self.kind {
MethodKind::Virtual { .. } |
MethodKind::VirtualDestructor { .. } => true,
_ => false,
}
}

/// Is this a static method?
Expand Down Expand Up @@ -960,7 +984,7 @@ pub struct CompInfo {

/// The destructor of this type. The bool represents whether this destructor
/// is virtual.
destructor: Option<(bool, FunctionId)>,
destructor: Option<(MethodKind, FunctionId)>,

/// Vector of classes this one inherits from.
base_members: Vec<Base>,
Expand Down Expand Up @@ -1104,7 +1128,7 @@ impl CompInfo {
}

/// Get this type's destructor.
pub fn destructor(&self) -> Option<(bool, FunctionId)> {
pub fn destructor(&self) -> Option<(MethodKind, FunctionId)> {
self.destructor
}

Expand Down Expand Up @@ -1355,14 +1379,23 @@ impl CompInfo {
ci.constructors.push(signature);
}
CXCursor_Destructor => {
ci.destructor = Some((is_virtual, signature));
let kind = if is_virtual {
MethodKind::VirtualDestructor {
pure_virtual: cur.method_is_pure_virtual(),
}
} else {
MethodKind::Destructor
};
ci.destructor = Some((kind, signature));
}
CXCursor_CXXMethod => {
let is_const = cur.method_is_const();
let method_kind = if is_static {
MethodKind::Static
} else if is_virtual {
MethodKind::Virtual
MethodKind::Virtual {
pure_virtual: cur.method_is_pure_virtual(),
}
} else {
MethodKind::Normal
};
Expand Down Expand Up @@ -1658,11 +1691,11 @@ impl Trace for CompInfo {
}

for method in self.methods() {
if method.is_destructor() {
tracer.visit_kind(method.signature.into(), EdgeKind::Destructor);
} else {
tracer.visit_kind(method.signature.into(), EdgeKind::Method);
}
tracer.visit_kind(method.signature.into(), EdgeKind::Method);
}

if let Some((_kind, signature)) = self.destructor() {
tracer.visit_kind(signature.into(), EdgeKind::Destructor);
}

for ctor in self.constructors() {
Expand Down
13 changes: 11 additions & 2 deletions src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,26 @@ pub enum FunctionKind {

impl FunctionKind {
fn from_cursor(cursor: &clang::Cursor) -> Option<FunctionKind> {
// FIXME(emilio): Deduplicate logic with `ir::comp`.
Some(match cursor.kind() {
clang_sys::CXCursor_FunctionDecl => FunctionKind::Function,
clang_sys::CXCursor_Constructor => FunctionKind::Method(
MethodKind::Constructor,
),
clang_sys::CXCursor_Destructor => FunctionKind::Method(
MethodKind::Destructor,
if cursor.method_is_virtual() {
MethodKind::VirtualDestructor {
pure_virtual: cursor.method_is_pure_virtual(),
}
} else {
MethodKind::Destructor
}
),
clang_sys::CXCursor_CXXMethod => {
if cursor.method_is_virtual() {
FunctionKind::Method(MethodKind::Virtual)
FunctionKind::Method(MethodKind::Virtual {
pure_virtual: cursor.method_is_pure_virtual(),
})
} else if cursor.method_is_static() {
FunctionKind::Method(MethodKind::Static)
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,12 +944,12 @@ impl Item {
cc.constructors
}
FunctionKind::Method(MethodKind::Destructor) |
FunctionKind::Method(MethodKind::VirtualDestructor) => {
FunctionKind::Method(MethodKind::VirtualDestructor { .. }) => {
cc.destructors
}
FunctionKind::Method(MethodKind::Static) |
FunctionKind::Method(MethodKind::Normal) |
FunctionKind::Method(MethodKind::Virtual) => cc.methods,
FunctionKind::Method(MethodKind::Virtual { .. }) => cc.methods,
}
}
}
Expand Down
29 changes: 29 additions & 0 deletions tests/expectations/tests/issue-1197-pure-virtual-stuff.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* automatically generated by rust-bindgen */

#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

#[repr(C)]
pub struct Foo__bindgen_vtable(::std::os::raw::c_void);
#[repr(C)]
#[derive(Debug)]
pub struct Foo {
pub vtable_: *const Foo__bindgen_vtable,
}
#[test]
fn bindgen_test_layout_Foo() {
assert_eq!(
::std::mem::size_of::<Foo>(),
8usize,
concat!("Size of: ", stringify!(Foo))
);
assert_eq!(
::std::mem::align_of::<Foo>(),
8usize,
concat!("Alignment of ", stringify!(Foo))
);
}
impl Default for Foo {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
6 changes: 6 additions & 0 deletions tests/headers/issue-1197-pure-virtual-stuff.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class Foo
{
public:
virtual void Bar() = 0;
virtual ~Foo() = 0;
};
2 changes: 1 addition & 1 deletion tests/headers/ref_argument_array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
#define NSID_LENGTH 10
class nsID {
public:
virtual void ToProvidedString(char (&aDest)[NSID_LENGTH]) = 0;
virtual void ToProvidedString(char (&aDest)[NSID_LENGTH]);
};

0 comments on commit 04591f0

Please sign in to comment.