Skip to content

Commit 1111301

Browse files
fletcherwfletcherw
committed
Track what can be Impl'ed for derive analysis
For analyze::<CannotDerive>(), differentiate between traits which we can derive, traits we cannot derive but can Impl, and traits which we can neither derive nor Impl. These states are, respectively, CanDerive::Yes, CanDerive::Manually, and CanDerive::No. Convert BindgenContext's cannot_derive_default from a HashSet<ItemId> to a HashMap<ItemId, CanDerive>. Change codegen to only generate a Default Impl for ItemIds which are CanDerive::Manually (which we lookup in the cannot_derive_default set). By doing this, we can differentiate between ItemIds which don't support deriving Default, but still need an Impl, and ItemIds for which we do not want a derived or Impl'ed Default (namely types which are blacklisted using the forthcoming "--no-default" flag).
1 parent 6ff3efd commit 1111301

File tree

4 files changed

+118
-79
lines changed

4 files changed

+118
-79
lines changed

‎src/codegen/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,8 +1790,10 @@ impl CodeGenerator for CompInfo {
17901790
if item.can_derive_default(ctx) {
17911791
derives.push("Default");
17921792
} else {
1793-
needs_default_impl =
1794-
ctx.options().derive_default && !self.is_forward_declaration();
1793+
needs_default_impl = ctx.options().derive_default &&
1794+
!self.is_forward_declaration() &&
1795+
ctx.lookup_can_derive_default(item.id()) ==
1796+
CanDerive::Manually;
17951797
}
17961798

17971799
let all_template_params = item.all_template_params(ctx);

‎src/ir/analysis/derive.rs

Lines changed: 91 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,10 @@ impl<'ctx> CannotDerive<'ctx> {
140140
fn constrain_type(&mut self, item: &Item, ty: &Type) -> CanDerive {
141141
if !self.ctx.whitelisted_items().contains(&item.id()) {
142142
trace!(
143-
" cannot derive {} for blacklisted type",
143+
" cannot derive {} for blacklisted type, but it can be implemented",
144144
self.derive_trait
145145
);
146-
return CanDerive::No;
146+
return CanDerive::Manually;
147147
}
148148

149149
if self.derive_trait.not_by_name(self.ctx, &item) {
@@ -156,15 +156,18 @@ impl<'ctx> CannotDerive<'ctx> {
156156

157157
trace!("ty: {:?}", ty);
158158
if item.is_opaque(self.ctx, &()) {
159-
if !self.derive_trait.can_derive_union() &&
160-
ty.is_union() &&
159+
if ty.is_union() &&
161160
self.ctx.options().rust_features().untagged_union
162161
{
163-
trace!(
164-
" cannot derive {} for Rust unions",
165-
self.derive_trait
166-
);
167-
return CanDerive::No;
162+
let can_derive = self.derive_trait.can_derive_union();
163+
if can_derive != CanDerive::Yes {
164+
trace!(
165+
" cannot derive {} for Rust unions ({:?})",
166+
self.derive_trait,
167+
can_derive,
168+
);
169+
return can_derive;
170+
}
168171
}
169172

170173
let layout_can_derive =
@@ -179,7 +182,13 @@ impl<'ctx> CannotDerive<'ctx> {
179182
self.derive_trait
180183
);
181184
}
182-
_ => {
185+
CanDerive::Manually => {
186+
trace!(
187+
" we cannot derive {} for the layout, but it can be implemented",
188+
self.derive_trait
189+
);
190+
}
191+
CanDerive::No => {
183192
trace!(
184193
" we cannot derive {} for the layout",
185194
self.derive_trait
@@ -226,11 +235,12 @@ impl<'ctx> CannotDerive<'ctx> {
226235
if inner_type != CanDerive::Yes {
227236
trace!(
228237
" arrays of T for which we cannot derive {} \
229-
also cannot derive {}",
238+
also cannot derive {}, ({:?})",
230239
self.derive_trait,
231-
self.derive_trait
240+
self.derive_trait,
241+
inner_type
232242
);
233-
return CanDerive::No;
243+
return inner_type;
234244
}
235245

236246
if len == 0 && !self.derive_trait.can_derive_incomplete_array()
@@ -308,62 +318,74 @@ impl<'ctx> CannotDerive<'ctx> {
308318
}
309319

310320
if info.kind() == CompKind::Union {
311-
if self.derive_trait.can_derive_union() {
312-
if self.ctx.options().rust_features().untagged_union &&
313-
// https://github.com/rust-lang/rust/issues/36640
314-
(!info.self_template_params(self.ctx).is_empty() ||
315-
!item.all_template_params(self.ctx).is_empty())
316-
{
317-
trace!(
318-
" cannot derive {} for Rust union because issue 36640", self.derive_trait
319-
);
320-
return CanDerive::No;
321-
}
322-
// fall through to be same as non-union handling
323-
} else {
324-
if self.ctx.options().rust_features().untagged_union {
325-
trace!(
326-
" cannot derive {} for Rust unions",
327-
self.derive_trait
328-
);
329-
return CanDerive::No;
330-
}
331-
332-
let layout_can_derive =
333-
ty.layout(self.ctx).map_or(CanDerive::Yes, |l| {
334-
l.opaque()
335-
.array_size_within_derive_limit(self.ctx)
336-
});
337-
match layout_can_derive {
338-
CanDerive::Yes => {
321+
match self.derive_trait.can_derive_union() {
322+
CanDerive::Yes => {
323+
if self.ctx.options().rust_features().untagged_union &&
324+
// https://github.com/rust-lang/rust/issues/36640
325+
(!info.self_template_params(self.ctx).is_empty() ||
326+
!item.all_template_params(self.ctx).is_empty())
327+
{
339328
trace!(
340-
" union layout can trivially derive {}",
341-
self.derive_trait
329+
" cannot derive {} for Rust union because issue 36640", self.derive_trait
342330
);
331+
return CanDerive::No;
343332
}
344-
_ => {
333+
// fall through to be same as non-union handling
334+
}
335+
trait_can_derive => {
336+
if self.ctx.options().rust_features().untagged_union
337+
{
345338
trace!(
346-
" union layout cannot derive {}",
347-
self.derive_trait
348-
);
339+
" cannot derive {} for Rust unions ({:?})",
340+
self.derive_trait,
341+
trait_can_derive,
342+
);
343+
return trait_can_derive;
349344
}
350-
};
351-
return layout_can_derive;
345+
346+
let layout_can_derive = ty.layout(self.ctx).map_or(
347+
CanDerive::Yes,
348+
|l| {
349+
l.opaque().array_size_within_derive_limit(
350+
self.ctx,
351+
)
352+
},
353+
);
354+
match layout_can_derive {
355+
CanDerive::Yes => {
356+
trace!(
357+
" union layout can trivially derive {}",
358+
self.derive_trait
359+
);
360+
}
361+
_ => {
362+
trace!(
363+
" union layout cannot derive {}, ({:?})",
364+
self.derive_trait,
365+
layout_can_derive,
366+
);
367+
}
368+
};
369+
return layout_can_derive;
370+
}
352371
}
353372
}
354373

355-
if !self.derive_trait.can_derive_compound_with_vtable() &&
356-
item.has_vtable(self.ctx)
357-
{
358-
trace!(
359-
" cannot derive {} for comp with vtable",
360-
self.derive_trait
361-
);
362-
return CanDerive::No;
374+
if item.has_vtable(self.ctx) {
375+
let can_derive =
376+
self.derive_trait.can_derive_compound_with_vtable();
377+
if can_derive != CanDerive::Yes {
378+
trace!(
379+
" cannot derive {} for comp with vtable ({:?})",
380+
self.derive_trait,
381+
can_derive,
382+
);
383+
return can_derive;
384+
}
363385
}
364386

365387
let pred = self.derive_trait.consider_edge_comp();
366-
return self.constrain_join(item, pred);
388+
self.constrain_join(item, pred)
367389
}
368390

369391
TypeKind::ResolvedTypeRef(..) |
@@ -476,10 +498,11 @@ impl DeriveTrait {
476498
}
477499
}
478500

479-
fn can_derive_union(&self) -> bool {
501+
fn can_derive_union(&self) -> CanDerive {
480502
match self {
481-
DeriveTrait::Copy => true,
482-
_ => false,
503+
DeriveTrait::Copy => CanDerive::Yes,
504+
DeriveTrait::Default => CanDerive::Manually,
505+
_ => CanDerive::No,
483506
}
484507
}
485508

@@ -490,10 +513,10 @@ impl DeriveTrait {
490513
}
491514
}
492515

493-
fn can_derive_compound_with_vtable(&self) -> bool {
516+
fn can_derive_compound_with_vtable(&self) -> CanDerive {
494517
match self {
495-
DeriveTrait::Default => false,
496-
_ => true,
518+
DeriveTrait::Default => CanDerive::Manually,
519+
_ => CanDerive::Yes,
497520
}
498521
}
499522

@@ -549,8 +572,8 @@ impl DeriveTrait {
549572
fn can_derive_pointer(&self) -> CanDerive {
550573
match self {
551574
DeriveTrait::Default => {
552-
trace!(" pointer cannot derive Default");
553-
CanDerive::No
575+
trace!(" pointer cannot derive Default, but it may be implemented");
576+
CanDerive::Manually
554577
}
555578
_ => {
556579
trace!(" pointer can derive {}", self);
@@ -570,8 +593,8 @@ impl DeriveTrait {
570593
(DeriveTrait::Default, TypeKind::ObjCInterface(..)) |
571594
(DeriveTrait::Default, TypeKind::ObjCId) |
572595
(DeriveTrait::Default, TypeKind::ObjCSel) => {
573-
trace!(" types that always cannot derive Default");
574-
CanDerive::No
596+
trace!(" types that always cannot derive Default, but may implement it manually");
597+
CanDerive::Manually
575598
}
576599
(DeriveTrait::Default, TypeKind::UnresolvedTypeRef(..)) => {
577600
unreachable!(

‎src/ir/context.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ where
223223
T: Copy + Into<ItemId>,
224224
{
225225
fn can_derive_default(&self, ctx: &BindgenContext) -> bool {
226-
ctx.options().derive_default && ctx.lookup_can_derive_default(*self)
226+
ctx.options().derive_default &&
227+
ctx.lookup_can_derive_default(*self) == CanDerive::Yes
227228
}
228229
}
229230

@@ -401,11 +402,12 @@ pub struct BindgenContext {
401402
/// and is always `None` before that and `Some` after.
402403
cannot_derive_debug: Option<HashSet<ItemId>>,
403404

404-
/// The set of (`ItemId`s of) types that can't derive default.
405+
/// Map from type `ItemId`s to why they can't derive hash. If an id is not
406+
/// in this map, we can assume that its value would be `CanDerive::Yes`.
405407
///
406408
/// This is populated when we enter codegen by `compute_cannot_derive_default`
407409
/// and is always `None` before that and `Some` after.
408-
cannot_derive_default: Option<HashSet<ItemId>>,
410+
cannot_derive_default: Option<HashMap<ItemId, CanDerive>>,
409411

410412
/// The set of (`ItemId`s of) types that can't derive copy.
411413
///
@@ -2462,16 +2464,16 @@ If you encounter an error missing from this list, please file an issue or a PR!"
24622464
assert!(self.cannot_derive_default.is_none());
24632465
if self.options.derive_default {
24642466
self.cannot_derive_default =
2465-
Some(as_cannot_derive_set(analyze::<CannotDerive>((
2466-
self,
2467-
DeriveTrait::Default,
2468-
))));
2467+
Some(analyze::<CannotDerive>((self, DeriveTrait::Default)));
24692468
}
24702469
}
24712470

24722471
/// Look up whether the item with `id` can
24732472
/// derive default or not.
2474-
pub fn lookup_can_derive_default<Id: Into<ItemId>>(&self, id: Id) -> bool {
2473+
pub fn lookup_can_derive_default<Id: Into<ItemId>>(
2474+
&self,
2475+
id: Id,
2476+
) -> CanDerive {
24752477
let id = id.into();
24762478
assert!(
24772479
self.in_codegen_phase(),
@@ -2480,7 +2482,12 @@ If you encounter an error missing from this list, please file an issue or a PR!"
24802482

24812483
// Look up the computed value for whether the item with `id` can
24822484
// derive default or not.
2483-
!self.cannot_derive_default.as_ref().unwrap().contains(&id)
2485+
self.cannot_derive_default
2486+
.as_ref()
2487+
.unwrap()
2488+
.get(&id)
2489+
.cloned()
2490+
.unwrap_or(CanDerive::Yes)
24842491
}
24852492

24862493
/// Compute whether we can derive copy.

‎tests/expectations/tests/layout_array.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,13 @@ impl Default for rte_mempool_ops_table {
284284
unsafe { ::std::mem::zeroed() }
285285
}
286286
}
287+
impl ::std::cmp::PartialEq for rte_mempool_ops_table {
288+
fn eq(&self, other: &rte_mempool_ops_table) -> bool {
289+
self.sl == other.sl &&
290+
self.num_ops == other.num_ops &&
291+
self.ops == other.ops
292+
}
293+
}
287294
/// Structure to hold malloc heap
288295
#[repr(C)]
289296
#[repr(align(64))]

0 commit comments

Comments
 (0)