Skip to content

Commit 589f476

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 types 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 c462b63 commit 589f476

File tree

4 files changed

+162
-85
lines changed

4 files changed

+162
-85
lines changed

‎src/codegen/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,8 +1738,10 @@ impl CodeGenerator for CompInfo {
17381738
if item.can_derive_default(ctx) {
17391739
derives.push("Default");
17401740
} else {
1741-
needs_default_impl =
1742-
ctx.options().derive_default && !self.is_forward_declaration();
1741+
needs_default_impl = ctx.options().derive_default &&
1742+
!self.is_forward_declaration() &&
1743+
ctx.lookup_can_derive_default(item.id()) ==
1744+
CanDerive::Manually;
17431745
}
17441746

17451747
let all_template_params = item.all_template_params(ctx);

‎src/ir/analysis/derive.rs

Lines changed: 135 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,10 @@ impl<'ctx> CannotDerive<'ctx> {
143143
" cannot derive {} for blacklisted type",
144144
self.derive_trait
145145
);
146-
return CanDerive::No;
146+
match self.derive_trait {
147+
DeriveTrait::Default => return CanDerive::Manually,
148+
_ => return CanDerive::No,
149+
}
147150
}
148151

149152
if self.derive_trait.not_by_name(self.ctx, &item) {
@@ -156,15 +159,26 @@ impl<'ctx> CannotDerive<'ctx> {
156159

157160
trace!("ty: {:?}", ty);
158161
if item.is_opaque(self.ctx, &()) {
159-
if !self.derive_trait.can_derive_union() &&
160-
ty.is_union() &&
162+
if ty.is_union() &&
161163
self.ctx.options().rust_features().untagged_union
162164
{
163-
trace!(
164-
" cannot derive {} for Rust unions",
165-
self.derive_trait
166-
);
167-
return CanDerive::No;
165+
match self.derive_trait.can_derive_union() {
166+
CanDerive::Manually => {
167+
trace!(
168+
" cannot derive {} for Rust unions, but it can be implemented",
169+
self.derive_trait
170+
);
171+
return CanDerive::Manually;
172+
}
173+
CanDerive::No => {
174+
trace!(
175+
" cannot derive {} for Rust unions",
176+
self.derive_trait
177+
);
178+
return CanDerive::No;
179+
}
180+
_ => (),
181+
};
168182
}
169183

170184
let layout_can_derive =
@@ -179,7 +193,13 @@ impl<'ctx> CannotDerive<'ctx> {
179193
self.derive_trait
180194
);
181195
}
182-
_ => {
196+
CanDerive::Manually => {
197+
trace!(
198+
" we cannot derive {} for the layout, but it can be implemented",
199+
self.derive_trait
200+
);
201+
}
202+
CanDerive::No => {
183203
trace!(
184204
" we cannot derive {} for the layout",
185205
self.derive_trait
@@ -223,15 +243,27 @@ impl<'ctx> CannotDerive<'ctx> {
223243
TypeKind::Array(t, len) => {
224244
let inner_type =
225245
self.can_derive.get(&t.into()).cloned().unwrap_or_default();
226-
if inner_type != CanDerive::Yes {
227-
trace!(
228-
" arrays of T for which we cannot derive {} \
229-
also cannot derive {}",
230-
self.derive_trait,
231-
self.derive_trait
232-
);
233-
return CanDerive::No;
234-
}
246+
match inner_type {
247+
CanDerive::Manually => {
248+
trace!(
249+
" arrays of T for which we can only manually implement {} \
250+
also can only manually implement {}",
251+
self.derive_trait,
252+
self.derive_trait
253+
);
254+
return CanDerive::Manually;
255+
}
256+
CanDerive::No => {
257+
trace!(
258+
" arrays of T for which we cannot derive {} \
259+
also cannot derive {}",
260+
self.derive_trait,
261+
self.derive_trait
262+
);
263+
return CanDerive::No;
264+
}
265+
_ => (),
266+
};
235267

236268
if len == 0 && !self.derive_trait.can_derive_incomplete_array()
237269
{
@@ -308,62 +340,90 @@ impl<'ctx> CannotDerive<'ctx> {
308340
}
309341

310342
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 => {
343+
match self.derive_trait.can_derive_union() {
344+
CanDerive::Yes => {
345+
if self.ctx.options().rust_features().untagged_union &&
346+
// https://github.com/rust-lang/rust/issues/36640
347+
(!info.self_template_params(self.ctx).is_empty() ||
348+
!item.all_template_params(self.ctx).is_empty())
349+
{
339350
trace!(
340-
" union layout can trivially derive {}",
341-
self.derive_trait
351+
" cannot derive {} for Rust union because issue 36640", self.derive_trait
342352
);
353+
return CanDerive::No;
343354
}
344-
_ => {
345-
trace!(
346-
" union layout cannot derive {}",
347-
self.derive_trait
348-
);
355+
// fall through to be same as non-union handling
356+
}
357+
trait_can_derive => {
358+
if self.ctx.options().rust_features().untagged_union
359+
{
360+
match trait_can_derive {
361+
CanDerive::Manually => {
362+
trace!(
363+
" cannot derive {} for Rust union, but it can be implemented",
364+
self.derive_trait
365+
);
366+
}
367+
_ => {
368+
trace!(
369+
" cannot derive {} for Rust unions",
370+
self.derive_trait
371+
);
372+
}
373+
}
374+
return trait_can_derive;
349375
}
350-
};
351-
return layout_can_derive;
376+
377+
let layout_can_derive = ty.layout(self.ctx).map_or(
378+
CanDerive::Yes,
379+
|l| {
380+
l.opaque().array_size_within_derive_limit(
381+
self.ctx,
382+
)
383+
},
384+
);
385+
match layout_can_derive {
386+
CanDerive::Yes => {
387+
trace!(
388+
" union layout can trivially derive {}",
389+
self.derive_trait
390+
);
391+
}
392+
_ => {
393+
trace!(
394+
" union layout cannot derive {}",
395+
self.derive_trait
396+
);
397+
}
398+
};
399+
return layout_can_derive;
400+
}
352401
}
353402
}
354403

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;
404+
match (
405+
self.derive_trait.can_derive_compound_with_vtable(),
406+
item.has_vtable(self.ctx),
407+
) {
408+
(CanDerive::Manually, true) => {
409+
trace!(
410+
" cannot derive {} for comp with vtable, but it can be implemented",
411+
self.derive_trait
412+
);
413+
return CanDerive::Manually;
414+
}
415+
(CanDerive::No, true) => {
416+
trace!(
417+
" cannot derive {} for comp with vtable",
418+
self.derive_trait
419+
);
420+
return CanDerive::No;
421+
}
422+
_ => (),
363423
}
364424

365425
let pred = self.derive_trait.consider_edge_comp();
366-
return self.constrain_join(item, pred);
426+
self.constrain_join(item, pred)
367427
}
368428

369429
TypeKind::ResolvedTypeRef(..) |
@@ -476,10 +536,11 @@ impl DeriveTrait {
476536
}
477537
}
478538

479-
fn can_derive_union(&self) -> bool {
539+
fn can_derive_union(&self) -> CanDerive {
480540
match self {
481-
DeriveTrait::Copy => true,
482-
_ => false,
541+
DeriveTrait::Copy => CanDerive::Yes,
542+
DeriveTrait::Default => CanDerive::Manually,
543+
_ => CanDerive::No,
483544
}
484545
}
485546

@@ -490,10 +551,10 @@ impl DeriveTrait {
490551
}
491552
}
492553

493-
fn can_derive_compound_with_vtable(&self) -> bool {
554+
fn can_derive_compound_with_vtable(&self) -> CanDerive {
494555
match self {
495-
DeriveTrait::Default => false,
496-
_ => true,
556+
DeriveTrait::Default => CanDerive::Manually,
557+
_ => CanDerive::Yes,
497558
}
498559
}
499560

@@ -549,8 +610,8 @@ impl DeriveTrait {
549610
fn can_derive_pointer(&self) -> CanDerive {
550611
match self {
551612
DeriveTrait::Default => {
552-
trace!(" pointer cannot derive Default");
553-
CanDerive::No
613+
trace!(" pointer cannot derive Default, but it may be implemented");
614+
CanDerive::Manually
554615
}
555616
_ => {
556617
trace!(" pointer can derive {}", self);
@@ -570,8 +631,8 @@ impl DeriveTrait {
570631
(DeriveTrait::Default, TypeKind::ObjCInterface(..)) |
571632
(DeriveTrait::Default, TypeKind::ObjCId) |
572633
(DeriveTrait::Default, TypeKind::ObjCSel) => {
573-
trace!(" types that always cannot derive Default");
574-
CanDerive::No
634+
trace!(" types that always cannot derive Default, but may implement it manually");
635+
CanDerive::Manually
575636
}
576637
(DeriveTrait::Default, TypeKind::UnresolvedTypeRef(..)) => {
577638
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
///
@@ -2445,16 +2447,16 @@ If you encounter an error missing from this list, please file an issue or a PR!"
24452447
assert!(self.cannot_derive_default.is_none());
24462448
if self.options.derive_default {
24472449
self.cannot_derive_default =
2448-
Some(as_cannot_derive_set(analyze::<CannotDerive>((
2449-
self,
2450-
DeriveTrait::Default,
2451-
))));
2450+
Some(analyze::<CannotDerive>((self, DeriveTrait::Default)));
24522451
}
24532452
}
24542453

24552454
/// Look up whether the item with `id` can
24562455
/// derive default or not.
2457-
pub fn lookup_can_derive_default<Id: Into<ItemId>>(&self, id: Id) -> bool {
2456+
pub fn lookup_can_derive_default<Id: Into<ItemId>>(
2457+
&self,
2458+
id: Id,
2459+
) -> CanDerive {
24582460
let id = id.into();
24592461
assert!(
24602462
self.in_codegen_phase(),
@@ -2463,7 +2465,12 @@ If you encounter an error missing from this list, please file an issue or a PR!"
24632465

24642466
// Look up the computed value for whether the item with `id` can
24652467
// derive default or not.
2466-
!self.cannot_derive_default.as_ref().unwrap().contains(&id)
2468+
self.cannot_derive_default
2469+
.as_ref()
2470+
.unwrap()
2471+
.get(&id)
2472+
.cloned()
2473+
.unwrap_or(CanDerive::Yes)
24672474
}
24682475

24692476
/// 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)