Skip to content

Commit ce7e69b

Browse files
author
bors-servo
authored
Auto merge of #1241 - emilio:fwd-decl-no-fun, r=fitzgen
codegen: Make forward declarations go through the more generic path. Instead of special-casing. This allows to use the normal flags to control what can be or not derived for them. Arguably deriving Copy / Clone is kind of busted for those, but changing this by default broke tests (RefPtr<ForwardDeclaredType> stopped working for example). So I think this is a good compromise. Fixes #1238
2 parents 6adb002 + 34b9216 commit ce7e69b

19 files changed

+96
-80
lines changed

‎src/codegen/mod.rs

+16-32
Original file line numberDiff line numberDiff line change
@@ -1425,23 +1425,6 @@ impl CodeGenerator for CompInfo {
14251425
let layout = ty.layout(ctx);
14261426
let mut packed = self.is_packed(ctx, &layout);
14271427

1428-
// generate tuple struct if struct or union is a forward declaration,
1429-
// skip for now if template parameters are needed.
1430-
//
1431-
// NB: We generate a proper struct to avoid struct/function name
1432-
// collisions.
1433-
if self.is_forward_declaration() && used_template_params.is_none() {
1434-
let struct_name = item.canonical_name(ctx);
1435-
let struct_name = ctx.rust_ident_raw(struct_name);
1436-
let tuple_struct = quote! {
1437-
#[repr(C)]
1438-
#[derive(Debug, Copy, Clone)]
1439-
pub struct #struct_name { _unused: [u8; 0] }
1440-
};
1441-
result.push(tuple_struct);
1442-
return;
1443-
}
1444-
14451428
let canonical_name = item.canonical_name(ctx);
14461429
let canonical_ident = ctx.rust_ident(&canonical_name);
14471430

@@ -1497,14 +1480,6 @@ impl CodeGenerator for CompInfo {
14971480
}
14981481
}
14991482

1500-
let is_union = self.kind() == CompKind::Union;
1501-
if is_union {
1502-
result.saw_union();
1503-
if !self.can_be_rust_union(ctx) {
1504-
result.saw_bindgen_union();
1505-
}
1506-
}
1507-
15081483
let mut methods = vec![];
15091484
if !is_opaque {
15101485
let codegen_depth = item.codegen_depth(ctx);
@@ -1529,8 +1504,14 @@ impl CodeGenerator for CompInfo {
15291504
}
15301505
}
15311506

1507+
let is_union = self.kind() == CompKind::Union;
15321508
let layout = item.kind().expect_type().layout(ctx);
1533-
if is_union && !is_opaque {
1509+
if is_union && !is_opaque && !self.is_forward_declaration() {
1510+
result.saw_union();
1511+
if !self.can_be_rust_union(ctx) {
1512+
result.saw_bindgen_union();
1513+
}
1514+
15341515
let layout = layout.expect("Unable to get layout information?");
15351516
let ty = helpers::blob(layout);
15361517

@@ -1594,7 +1575,11 @@ impl CodeGenerator for CompInfo {
15941575
//
15951576
// NOTE: This check is conveniently here to avoid the dummy fields we
15961577
// may add for unused template parameters.
1597-
if item.is_zero_sized(ctx) {
1578+
if self.is_forward_declaration() {
1579+
fields.push(quote! {
1580+
_unused: [u8; 0],
1581+
});
1582+
} else if item.is_zero_sized(ctx) {
15981583
let has_address = if is_opaque {
15991584
// Generate the address field if it's an opaque type and
16001585
// couldn't determine the layout of the blob.
@@ -1664,12 +1649,11 @@ impl CodeGenerator for CompInfo {
16641649
if item.can_derive_default(ctx) {
16651650
derives.push("Default");
16661651
} else {
1667-
needs_default_impl = ctx.options().derive_default;
1652+
needs_default_impl =
1653+
ctx.options().derive_default && !self.is_forward_declaration();
16681654
}
16691655

1670-
if item.can_derive_copy(ctx) && !item.annotations().disallow_copy() &&
1671-
ctx.options().derive_copy
1672-
{
1656+
if item.can_derive_copy(ctx) && !item.annotations().disallow_copy() {
16731657
derives.push("Copy");
16741658

16751659
if ctx.options().rust_features().builtin_clone_impls() ||
@@ -1762,7 +1746,7 @@ impl CodeGenerator for CompInfo {
17621746
}
17631747
}
17641748

1765-
if ctx.options().layout_tests {
1749+
if ctx.options().layout_tests && !self.is_forward_declaration() {
17661750
if let Some(layout) = layout {
17671751
let fn_name =
17681752
format!("bindgen_test_layout_{}", canonical_ident.as_str());

‎src/ir/analysis/derive_copy.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,10 @@ impl<'ctx> MonotoneFramework for CannotDeriveCopy<'ctx> {
235235

236236
if info.kind() == CompKind::Union {
237237
if !self.ctx.options().rust_features().untagged_union() {
238-
// NOTE: If there's no template parameters we can derive copy
239-
// unconditionally, since arrays are magical for rustc, and
240-
// __BindgenUnionField always implements copy.
238+
// NOTE: If there's no template parameters we can derive
239+
// copy unconditionally, since arrays are magical for
240+
// rustc, and __BindgenUnionField always implements
241+
// copy.
241242
trace!(
242243
" comp can always derive debug if it's a Union and no template parameters"
243244
);

‎src/ir/analysis/derive_default.rs

+5
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ impl<'ctx> MonotoneFramework for CannotDeriveDefault<'ctx> {
265265
"The early ty.is_opaque check should have handled this case"
266266
);
267267

268+
if info.is_forward_declaration() {
269+
trace!(" cannot derive Default for forward decls");
270+
return self.insert(id);
271+
}
272+
268273
if info.kind() == CompKind::Union {
269274
if self.ctx.options().rust_features().untagged_union() {
270275
trace!(" cannot derive Default for Rust unions");

‎src/ir/analysis/derive_hash.rs

+5
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,11 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {
251251
"The early ty.is_opaque check should have handled this case"
252252
);
253253

254+
if info.is_forward_declaration() {
255+
trace!(" cannot derive Hash for forward decls");
256+
return self.insert(id);
257+
}
258+
254259
if info.kind() == CompKind::Union {
255260
if self.ctx.options().rust_features().untagged_union() {
256261
trace!(" cannot derive Hash for Rust unions");

‎src/ir/analysis/derive_partialeq_or_partialord.rs

+5
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,11 @@ impl<'ctx> CannotDerivePartialEqOrPartialOrd<'ctx> {
236236
"The early ty.is_opaque check should have handled this case"
237237
);
238238

239+
if info.is_forward_declaration() {
240+
trace!(" cannot derive for forward decls");
241+
return CanDerive::No;
242+
}
243+
239244
if info.kind() == CompKind::Union {
240245
if self.ctx.options().rust_features().untagged_union() {
241246
trace!(

‎src/ir/comp.rs

+14-7
Original file line numberDiff line numberDiff line change
@@ -1543,13 +1543,20 @@ impl CompInfo {
15431543
/// 1. Current RustTarget allows for `untagged_union`
15441544
/// 2. Each field can derive `Copy`
15451545
pub fn can_be_rust_union(&self, ctx: &BindgenContext) -> bool {
1546-
ctx.options().rust_features().untagged_union() &&
1547-
self.fields().iter().all(|f| match *f {
1548-
Field::DataMember(ref field_data) => {
1549-
field_data.ty().can_derive_copy(ctx)
1550-
}
1551-
Field::Bitfields(_) => true,
1552-
})
1546+
if !ctx.options().rust_features().untagged_union() {
1547+
return false;
1548+
}
1549+
1550+
if self.is_forward_declaration() {
1551+
return false;
1552+
}
1553+
1554+
self.fields().iter().all(|f| match *f {
1555+
Field::DataMember(ref field_data) => {
1556+
field_data.ty().can_derive_copy(ctx)
1557+
}
1558+
Field::Bitfields(_) => true,
1559+
})
15531560
}
15541561
}
15551562

‎src/ir/context.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,7 @@ where
222222
T: Copy + Into<ItemId>
223223
{
224224
fn can_derive_default(&self, ctx: &BindgenContext) -> bool {
225-
ctx.options().derive_default &&
226-
ctx.lookup_can_derive_default(*self)
225+
ctx.options().derive_default && ctx.lookup_can_derive_default(*self)
227226
}
228227
}
229228

@@ -232,7 +231,7 @@ where
232231
T: Copy + Into<ItemId>
233232
{
234233
fn can_derive_copy(&self, ctx: &BindgenContext) -> bool {
235-
ctx.lookup_can_derive_copy(*self)
234+
ctx.options().derive_copy && ctx.lookup_can_derive_copy(*self)
236235
}
237236
}
238237

@@ -2452,6 +2451,7 @@ impl BindgenContext {
24522451
// Look up the computed value for whether the item with `id` can
24532452
// derive `Copy` or not.
24542453
let id = id.into();
2454+
24552455
!self.lookup_has_type_param_in_array(id) &&
24562456
!self.cannot_derive_copy.as_ref().unwrap().contains(&id)
24572457
}

‎tests/expectations/tests/anon_union.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ pub enum TErrorResult_UnionState {
1717
HasMessage = 0,
1818
}
1919
#[repr(C)]
20-
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
20+
#[derive(Debug, Copy, Clone)]
2121
pub struct TErrorResult_Message {
22-
pub _address: u8,
22+
_unused: [u8; 0],
2323
}
2424
#[repr(C)]
25-
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
25+
#[derive(Debug, Copy, Clone)]
2626
pub struct TErrorResult_DOMExceptionInfo {
27-
pub _address: u8,
27+
_unused: [u8; 0],
2828
}
2929
#[repr(C)]
3030
pub union TErrorResult__bindgen_ty_1 {

‎tests/expectations/tests/anon_union_1_0.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,14 @@ pub enum TErrorResult_UnionState {
6161
HasMessage = 0,
6262
}
6363
#[repr(C)]
64-
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
64+
#[derive(Debug, Copy, Clone)]
6565
pub struct TErrorResult_Message {
66-
pub _address: u8,
66+
_unused: [u8; 0],
6767
}
6868
#[repr(C)]
69-
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
69+
#[derive(Debug, Copy, Clone)]
7070
pub struct TErrorResult_DOMExceptionInfo {
71-
pub _address: u8,
71+
_unused: [u8; 0],
7272
}
7373
#[repr(C)]
7474
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]

‎tests/expectations/tests/forward-declaration-autoptr.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
#[repr(C)]
86
#[derive(Debug, Copy, Clone)]
97
pub struct Foo {

‎tests/expectations/tests/forward_declared_complex_types.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
#[repr(C)]
86
#[derive(Debug, Default, Copy, Clone)]
97
pub struct Foo_empty {
@@ -60,7 +58,7 @@ extern "C" {
6058
pub fn baz_struct(f: *mut Foo);
6159
}
6260
#[repr(C)]
63-
#[derive(Debug, Copy, Clone)]
61+
#[derive(Copy, Clone)]
6462
pub struct Union {
6563
_unused: [u8; 0],
6664
}

‎tests/expectations/tests/forward_declared_complex_types_1_0.rs

+18-5
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
#[repr(C)]
86
#[derive(Debug, Default, Copy)]
97
pub struct Foo_empty {
@@ -28,10 +26,15 @@ impl Clone for Foo_empty {
2826
}
2927
}
3028
#[repr(C)]
31-
#[derive(Debug, Copy, Clone)]
29+
#[derive(Debug, Copy)]
3230
pub struct Foo {
3331
_unused: [u8; 0],
3432
}
33+
impl Clone for Foo {
34+
fn clone(&self) -> Self {
35+
*self
36+
}
37+
}
3538
#[repr(C)]
3639
#[derive(Debug, Copy)]
3740
pub struct Bar {
@@ -70,19 +73,29 @@ extern "C" {
7073
pub fn baz_struct(f: *mut Foo);
7174
}
7275
#[repr(C)]
73-
#[derive(Debug, Copy, Clone)]
76+
#[derive(Copy)]
7477
pub struct Union {
7578
_unused: [u8; 0],
7679
}
80+
impl Clone for Union {
81+
fn clone(&self) -> Self {
82+
*self
83+
}
84+
}
7785
extern "C" {
7886
#[link_name = "\u{1}_Z9baz_unionP5Union"]
7987
pub fn baz_union(u: *mut Union);
8088
}
8189
#[repr(C)]
82-
#[derive(Debug, Copy, Clone)]
90+
#[derive(Debug, Copy)]
8391
pub struct Quux {
8492
_unused: [u8; 0],
8593
}
94+
impl Clone for Quux {
95+
fn clone(&self) -> Self {
96+
*self
97+
}
98+
}
8699
extern "C" {
87100
#[link_name = "\u{1}_Z9baz_classP4Quux"]
88101
pub fn baz_class(q: *mut Quux);

‎tests/expectations/tests/issue-1238-fwd-no-copy.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/* automatically generated by rust-bindgen */
2+
3+
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
4+
5+
#[repr(C)]
6+
#[derive(Debug)]
7+
pub struct MyType {
8+
_unused: [u8; 0],
9+
}
10+
pub type MyTypeT = MyType;

‎tests/expectations/tests/issue-654-struct-fn-collision.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
#[repr(C)]
86
#[derive(Debug, Copy, Clone)]
97
pub struct foo {

‎tests/expectations/tests/issue-801-opaque-sloppiness.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
#[repr(C)]
86
#[derive(Debug, Copy, Clone)]
97
pub struct A {

‎tests/expectations/tests/layout_array.rs

+4-9
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
pub const RTE_CACHE_LINE_SIZE: ::std::os::raw::c_uint = 64;
86
pub const RTE_MEMPOOL_OPS_NAMESIZE: ::std::os::raw::c_uint = 32;
97
pub const RTE_MEMPOOL_MAX_OPS_IDX: ::std::os::raw::c_uint = 16;
@@ -21,9 +19,8 @@ pub struct rte_mempool {
2119
/// it will most likely point to a different type of data structure, and
2220
/// will be transparent to the application programmer.
2321
/// This function should set mp->pool_data.
24-
pub type rte_mempool_alloc_t = ::std::option::Option<
25-
unsafe extern "C" fn(mp: *mut rte_mempool) -> ::std::os::raw::c_int,
26-
>;
22+
pub type rte_mempool_alloc_t =
23+
::std::option::Option<unsafe extern "C" fn(mp: *mut rte_mempool) -> ::std::os::raw::c_int>;
2724
/// Free the opaque private data pointed to by mp->pool_data pointer.
2825
pub type rte_mempool_free_t = ::std::option::Option<unsafe extern "C" fn(mp: *mut rte_mempool)>;
2926
/// Enqueue an object into the external pool.
@@ -43,10 +40,8 @@ pub type rte_mempool_dequeue_t = ::std::option::Option<
4340
) -> ::std::os::raw::c_int,
4441
>;
4542
/// Return the number of available objects in the external pool.
46-
pub type rte_mempool_get_count = ::std::option::Option<
47-
unsafe extern "C" fn(mp: *const rte_mempool)
48-
-> ::std::os::raw::c_uint,
49-
>;
43+
pub type rte_mempool_get_count =
44+
::std::option::Option<unsafe extern "C" fn(mp: *const rte_mempool) -> ::std::os::raw::c_uint>;
5045
/// Structure defining mempool operations structure
5146
#[repr(C)]
5247
#[derive(Copy, Clone)]

‎tests/expectations/tests/same_struct_name_in_different_namespaces.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
/* automatically generated by rust-bindgen */
22

3-
43
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
54

6-
75
#[repr(C)]
86
#[derive(Debug, Copy, Clone)]
97
pub struct JS_Zone {

0 commit comments

Comments
 (0)