Skip to content

Commit cb10597

Browse files
author
bors-servo
authored
Auto merge of #950 - fitzgen:no-derive-copy-for-blacklisted-types, r=emilio
Derive + blacklisted types * [X] document blacklisting + derive interaction * [X] derive `Copy` * [X] derive `Debug` * [X] `--impl-debug` * [x] derive `Default` * [x] derive `Hash` * [x] derive `PartialEq`
2 parents 1e49750 + 74e2aea commit cb10597

29 files changed

+425
-98
lines changed

‎book/src/blacklisting.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ appear in the bindings at
88
all, [make it opaque](./opaque.html) instead of
99
blacklisting it.)
1010

11+
Blacklisted types are pessimistically assumed not to be able to `derive` any
12+
traits, which can transitively affect other types' ability to `derive` traits or
13+
not.
14+
1115
### Library
1216

1317
* [`bindgen::Builder::hide_type`](https://docs.rs/bindgen/0.23.1/bindgen/struct.Builder.html#method.hide_type)

‎src/codegen/derive_debug.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ pub fn gen_debug_impl(
5353
impl X {
5454
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
5555
write!(f, $format_string $tokens)
56-
}
56+
}
5757
});
5858

5959
match impl_.unwrap().node {
@@ -127,6 +127,12 @@ impl<'a> ImplDebug<'a> for Item {
127127
) -> Option<(String, Vec<TokenTree>)> {
128128
let name_ident = ctx.rust_ident_raw(name);
129129

130+
// We don't know if blacklisted items `impl Debug` or not, so we can't
131+
// add them to the format string we're building up.
132+
if !ctx.whitelisted_items().contains(&self.id()) {
133+
return None;
134+
}
135+
130136
let ty = match self.as_type() {
131137
Some(ty) => ty,
132138
None => {
@@ -168,7 +174,7 @@ impl<'a> ImplDebug<'a> for Item {
168174
} else {
169175
debug_print(ctx, name, name_ident)
170176
}
171-
}
177+
}
172178

173179
// The generic is not required to implement Debug, so we can not debug print that type
174180
TypeKind::TypeParam => {

‎src/ir/analysis/derive_copy.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ impl<'ctx, 'gen> CannotDeriveCopy<'ctx, 'gen> {
8888

8989
ConstrainResult::Changed
9090
}
91+
92+
/// A type is not `Copy` if we've determined it is not copy, or if it is
93+
/// blacklisted.
94+
fn is_not_copy(&self, id: ItemId) -> bool {
95+
self.cannot_derive_copy.contains(&id) ||
96+
!self.ctx.whitelisted_items().contains(&id)
97+
}
9198
}
9299

93100
impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
@@ -118,6 +125,15 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
118125
return ConstrainResult::Same;
119126
}
120127

128+
// If an item is reachable from the whitelisted items set, but isn't
129+
// itself whitelisted, then it must be blacklisted. We assume that
130+
// blacklisted items are not `Copy`, since they are presumably
131+
// blacklisted because they are too complicated for us to understand.
132+
if !self.ctx.whitelisted_items().contains(&id) {
133+
trace!(" blacklisted items are assumed not to be Copy");
134+
return ConstrainResult::Same;
135+
}
136+
121137
let item = self.ctx.resolve_item(id);
122138
let ty = match item.as_type() {
123139
Some(ty) => ty,
@@ -163,7 +179,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
163179
}
164180

165181
TypeKind::Array(t, len) => {
166-
let cant_derive_copy = self.cannot_derive_copy.contains(&t);
182+
let cant_derive_copy = self.is_not_copy(t);
167183
if cant_derive_copy {
168184
trace!(
169185
" arrays of T for which we cannot derive Copy \
@@ -184,7 +200,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
184200
TypeKind::ResolvedTypeRef(t) |
185201
TypeKind::TemplateAlias(t, _) |
186202
TypeKind::Alias(t) => {
187-
let cant_derive_copy = self.cannot_derive_copy.contains(&t);
203+
let cant_derive_copy = self.is_not_copy(t);
188204
if cant_derive_copy {
189205
trace!(
190206
" arrays of T for which we cannot derive Copy \
@@ -237,7 +253,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
237253

238254
let bases_cannot_derive =
239255
info.base_members().iter().any(|base| {
240-
self.cannot_derive_copy.contains(&base.ty)
256+
self.is_not_copy(base.ty)
241257
});
242258
if bases_cannot_derive {
243259
trace!(
@@ -250,11 +266,11 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
250266
let fields_cannot_derive =
251267
info.fields().iter().any(|f| match *f {
252268
Field::DataMember(ref data) => {
253-
self.cannot_derive_copy.contains(&data.ty())
269+
self.is_not_copy(data.ty())
254270
}
255271
Field::Bitfields(ref bfu) => {
256272
bfu.bitfields().iter().any(|b| {
257-
self.cannot_derive_copy.contains(&b.ty())
273+
self.is_not_copy(b.ty())
258274
})
259275
}
260276
});
@@ -270,7 +286,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
270286
TypeKind::TemplateInstantiation(ref template) => {
271287
let args_cannot_derive =
272288
template.template_arguments().iter().any(|arg| {
273-
self.cannot_derive_copy.contains(&arg)
289+
self.is_not_copy(*arg)
274290
});
275291
if args_cannot_derive {
276292
trace!(
@@ -284,8 +300,8 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
284300
!template.template_definition().is_opaque(self.ctx, &()),
285301
"The early ty.is_opaque check should have handled this case"
286302
);
287-
let def_cannot_derive = self.cannot_derive_copy.contains(
288-
&template.template_definition(),
303+
let def_cannot_derive = self.is_not_copy(
304+
template.template_definition(),
289305
);
290306
if def_cannot_derive {
291307
trace!(

‎src/ir/analysis/derive_debug.rs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,13 @@ impl<'ctx, 'gen> CannotDeriveDebug<'ctx, 'gen> {
9090

9191
ConstrainResult::Changed
9292
}
93+
94+
/// A type is not `Debug` if we've determined it is not debug, or if it is
95+
/// blacklisted.
96+
fn is_not_debug(&self, id: ItemId) -> bool {
97+
self.cannot_derive_debug.contains(&id) ||
98+
!self.ctx.whitelisted_items().contains(&id)
99+
}
93100
}
94101

95102
impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
@@ -120,6 +127,15 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
120127
return ConstrainResult::Same;
121128
}
122129

130+
// If an item is reachable from the whitelisted items set, but isn't
131+
// itself whitelisted, then it must be blacklisted. We assume that
132+
// blacklisted items are not `Copy`, since they are presumably
133+
// blacklisted because they are too complicated for us to understand.
134+
if !self.ctx.whitelisted_items().contains(&id) {
135+
trace!(" blacklisted items are assumed not to be Debug");
136+
return ConstrainResult::Same;
137+
}
138+
123139
let item = self.ctx.resolve_item(id);
124140
let ty = match item.as_type() {
125141
Some(ty) => ty,
@@ -129,11 +145,13 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
129145
}
130146
};
131147

132-
if ty.is_opaque(self.ctx, item) {
148+
if item.is_opaque(self.ctx, &()) {
133149
let layout_can_derive = ty.layout(self.ctx).map_or(true, |l| {
134150
l.opaque().can_trivially_derive_debug()
135151
});
136-
return if layout_can_derive {
152+
return if layout_can_derive &&
153+
!(ty.is_union() &&
154+
self.ctx.options().rust_features().untagged_union()) {
137155
trace!(" we can trivially derive Debug for the layout");
138156
ConstrainResult::Same
139157
} else {
@@ -176,7 +194,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
176194
}
177195

178196
TypeKind::Array(t, len) => {
179-
if self.cannot_derive_debug.contains(&t) {
197+
if self.is_not_debug(t) {
180198
trace!(
181199
" arrays of T for which we cannot derive Debug \
182200
also cannot derive Debug"
@@ -196,7 +214,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
196214
TypeKind::ResolvedTypeRef(t) |
197215
TypeKind::TemplateAlias(t, _) |
198216
TypeKind::Alias(t) => {
199-
if self.cannot_derive_debug.contains(&t) {
217+
if self.is_not_debug(t) {
200218
trace!(
201219
" aliases and type refs to T which cannot derive \
202220
Debug also cannot derive Debug"
@@ -237,7 +255,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
237255

238256
let bases_cannot_derive =
239257
info.base_members().iter().any(|base| {
240-
self.cannot_derive_debug.contains(&base.ty)
258+
self.is_not_debug(base.ty)
241259
});
242260
if bases_cannot_derive {
243261
trace!(
@@ -250,11 +268,11 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
250268
let fields_cannot_derive =
251269
info.fields().iter().any(|f| match *f {
252270
Field::DataMember(ref data) => {
253-
self.cannot_derive_debug.contains(&data.ty())
271+
self.is_not_debug(data.ty())
254272
}
255273
Field::Bitfields(ref bfu) => {
256274
bfu.bitfields().iter().any(|b| {
257-
self.cannot_derive_debug.contains(&b.ty())
275+
self.is_not_debug(b.ty())
258276
})
259277
}
260278
});
@@ -287,7 +305,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
287305
TypeKind::TemplateInstantiation(ref template) => {
288306
let args_cannot_derive =
289307
template.template_arguments().iter().any(|arg| {
290-
self.cannot_derive_debug.contains(&arg)
308+
self.is_not_debug(*arg)
291309
});
292310
if args_cannot_derive {
293311
trace!(
@@ -301,8 +319,8 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
301319
!template.template_definition().is_opaque(self.ctx, &()),
302320
"The early ty.is_opaque check should have handled this case"
303321
);
304-
let def_cannot_derive = self.cannot_derive_debug.contains(
305-
&template.template_definition(),
322+
let def_cannot_derive = self.is_not_debug(
323+
template.template_definition(),
306324
);
307325
if def_cannot_derive {
308326
trace!(

‎src/ir/analysis/derive_default.rs

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ impl<'ctx, 'gen> CannotDeriveDefault<'ctx, 'gen> {
8787

8888
ConstrainResult::Changed
8989
}
90+
91+
fn is_not_default(&self, id: ItemId) -> bool {
92+
self.cannot_derive_default.contains(&id) ||
93+
!self.ctx.whitelisted_items().contains(&id)
94+
}
9095
}
9196

9297
impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
@@ -153,6 +158,11 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
153158
return ConstrainResult::Same;
154159
}
155160

161+
if !self.ctx.whitelisted_items().contains(&id) {
162+
trace!(" blacklisted items pessimistically cannot derive Default");
163+
return ConstrainResult::Same;
164+
}
165+
156166
let item = self.ctx.resolve_item(id);
157167
let ty = match item.as_type() {
158168
Some(ty) => ty,
@@ -166,7 +176,9 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
166176
let layout_can_derive = ty.layout(self.ctx).map_or(true, |l| {
167177
l.opaque().can_trivially_derive_default()
168178
});
169-
return if layout_can_derive {
179+
return if layout_can_derive &&
180+
!(ty.is_union() &&
181+
self.ctx.options().rust_features().untagged_union()) {
170182
trace!(" we can trivially derive Default for the layout");
171183
ConstrainResult::Same
172184
} else {
@@ -213,7 +225,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
213225
}
214226

215227
TypeKind::Array(t, len) => {
216-
if self.cannot_derive_default.contains(&t) {
228+
if self.is_not_default(t) {
217229
trace!(
218230
" arrays of T for which we cannot derive Default \
219231
also cannot derive Default"
@@ -233,7 +245,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
233245
TypeKind::ResolvedTypeRef(t) |
234246
TypeKind::TemplateAlias(t, _) |
235247
TypeKind::Alias(t) => {
236-
if self.cannot_derive_default.contains(&t) {
248+
if self.is_not_default(t) {
237249
trace!(
238250
" aliases and type refs to T which cannot derive \
239251
Default also cannot derive Default"
@@ -280,7 +292,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
280292
let bases_cannot_derive =
281293
info.base_members().iter().any(|base| {
282294
!self.ctx.whitelisted_items().contains(&base.ty) ||
283-
self.cannot_derive_default.contains(&base.ty)
295+
self.is_not_default(base.ty)
284296
});
285297
if bases_cannot_derive {
286298
trace!(
@@ -296,14 +308,14 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
296308
!self.ctx.whitelisted_items().contains(
297309
&data.ty(),
298310
) ||
299-
self.cannot_derive_default.contains(&data.ty())
311+
self.is_not_default(data.ty())
300312
}
301313
Field::Bitfields(ref bfu) => {
302314
bfu.bitfields().iter().any(|b| {
303315
!self.ctx.whitelisted_items().contains(
304316
&b.ty(),
305317
) ||
306-
self.cannot_derive_default.contains(&b.ty())
318+
self.is_not_default(b.ty())
307319
})
308320
}
309321
});
@@ -319,45 +331,34 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDefault<'ctx, 'gen> {
319331
}
320332

321333
TypeKind::TemplateInstantiation(ref template) => {
322-
if self.ctx.whitelisted_items().contains(
323-
&template.template_definition(),
324-
)
325-
{
326-
let args_cannot_derive =
327-
template.template_arguments().iter().any(|arg| {
328-
self.cannot_derive_default.contains(&arg)
329-
});
330-
if args_cannot_derive {
331-
trace!(
332-
" template args cannot derive Default, so \
333-
insantiation can't either"
334-
);
335-
return self.insert(id);
336-
}
337-
338-
assert!(
339-
!template.template_definition().is_opaque(self.ctx, &()),
340-
"The early ty.is_opaque check should have handled this case"
334+
let args_cannot_derive =
335+
template.template_arguments().iter().any(|arg| {
336+
self.is_not_default(*arg)
337+
});
338+
if args_cannot_derive {
339+
trace!(
340+
" template args cannot derive Default, so \
341+
insantiation can't either"
341342
);
342-
let def_cannot_derive =
343-
self.cannot_derive_default.contains(&template
344-
.template_definition());
345-
if def_cannot_derive {
346-
trace!(
347-
" template definition cannot derive Default, so \
348-
insantiation can't either"
349-
);
350-
return self.insert(id);
351-
}
343+
return self.insert(id);
344+
}
352345

353-
trace!(" template instantiation can derive Default");
354-
ConstrainResult::Same
355-
} else {
346+
assert!(
347+
!template.template_definition().is_opaque(self.ctx, &()),
348+
"The early ty.is_opaque check should have handled this case"
349+
);
350+
let def_cannot_derive =
351+
self.is_not_default(template.template_definition());
352+
if def_cannot_derive {
356353
trace!(
357-
" blacklisted template instantiation cannot derive default"
354+
" template definition cannot derive Default, so \
355+
insantiation can't either"
358356
);
359357
return self.insert(id);
360358
}
359+
360+
trace!(" template instantiation can derive Default");
361+
ConstrainResult::Same
361362
}
362363

363364
TypeKind::Opaque => {

‎src/ir/analysis/derive_hash.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,9 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveHash<'ctx, 'gen> {
133133
let layout_can_derive = ty.layout(self.ctx).map_or(true, |l| {
134134
l.opaque().can_trivially_derive_hash()
135135
});
136-
return if layout_can_derive {
136+
return if layout_can_derive &&
137+
!(ty.is_union() &&
138+
self.ctx.options().rust_features().untagged_union()) {
137139
trace!(" we can trivially derive Hash for the layout");
138140
ConstrainResult::Same
139141
} else {

0 commit comments

Comments
 (0)