diff options
| -rw-r--r-- | src/ir/context.rs | 10 | ||||
| -rw-r--r-- | src/ir/named.rs | 56 | ||||
| -rw-r--r-- | tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs | 80 | ||||
| -rw-r--r-- | tests/headers/issue-584-stylo-template-analysis-panic.hpp | 13 |
4 files changed, 145 insertions, 14 deletions
diff --git a/src/ir/context.rs b/src/ir/context.rs index 9b9ad8bd..d00e4899 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -631,6 +631,14 @@ impl<'ctx> BindgenContext<'ctx> { /// This method may only be called during the codegen phase, because the /// template usage information is only computed as we enter the codegen /// phase. + /// + /// If the item is blacklisted, then we say that it always uses the template + /// parameter. This is a little subtle. The template parameter usage + /// analysis only considers whitelisted items, and if any blacklisted item + /// shows up in the generated bindings, it is the user's responsibility to + /// manually provide a definition for them. To give them the most + /// flexibility when doing that, we assume that they use every template + /// parameter and always pass template arguments through in instantiations. pub fn uses_template_parameter(&self, item: ItemId, template_param: ItemId) @@ -643,7 +651,7 @@ impl<'ctx> BindgenContext<'ctx> { .expect("should have found template parameter usage if we're in codegen") .get(&item) .map(|items_used_params| items_used_params.contains(&template_param)) - .unwrap_or(false) + .unwrap_or_else(|| self.resolve_item(item).is_hidden(self)) } // This deserves a comment. Builtin types don't get a valid declaration, so diff --git a/src/ir/named.rs b/src/ir/named.rs index 67b36915..1af98a26 100644 --- a/src/ir/named.rs +++ b/src/ir/named.rs @@ -131,7 +131,7 @@ use super::item::ItemSet; use super::template::AsNamed; use super::traversal::{EdgeKind, Trace}; use super::ty::{TemplateDeclaration, TypeKind}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt; /// An analysis in the monotone framework. @@ -263,6 +263,8 @@ pub struct UsedTemplateParameters<'ctx, 'gen> used: HashMap<ItemId, Option<ItemSet>>, dependencies: HashMap<ItemId, Vec<ItemId>>, + + whitelisted_items: HashSet<ItemId>, } impl<'ctx, 'gen> UsedTemplateParameters<'ctx, 'gen> { @@ -316,21 +318,30 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> { -> UsedTemplateParameters<'ctx, 'gen> { let mut used = HashMap::new(); let mut dependencies = HashMap::new(); + let whitelisted_items: HashSet<_> = ctx.whitelisted_items().collect(); - for item in ctx.whitelisted_items() { + for item in whitelisted_items.iter().cloned() { dependencies.entry(item).or_insert(vec![]); used.insert(item, Some(ItemSet::new())); { // We reverse our natural IR graph edges to find dependencies // between nodes. - item.trace(ctx, - &mut |sub_item, _| { - dependencies.entry(sub_item) - .or_insert(vec![]) - .push(item); - }, - &()); + item.trace(ctx, &mut |sub_item, _| { + // We won't be generating code for items that aren't + // whitelisted, so don't bother keeping track of their + // template parameters. But isn't whitelisting the + // transitive closure of reachable items from the explicitly + // whitelisted items? Usually! The exception is explicitly + // blacklisted items. + if !whitelisted_items.contains(&sub_item) { + return; + } + + dependencies.entry(sub_item) + .or_insert(vec![]) + .push(item); + }, &()); } // Additionally, whether a template instantiation's template @@ -361,6 +372,7 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> { ctx: ctx, used: used, dependencies: dependencies, + whitelisted_items: whitelisted_items, } } @@ -395,6 +407,19 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> { used_by_this_id.insert(id); } + // We say that blacklisted items use all of their template + // parameters. The blacklisted type is most likely implemented + // explicitly by the user, since it won't be in the generated + // bindings, and we don't know exactly what they'll to with template + // parameters, but we can push the issue down the line to them. + Some(&TypeKind::TemplateInstantiation(ref inst)) + if !self.whitelisted_items.contains(&inst.template_definition()) => { + let args = inst.template_arguments() + .iter() + .filter_map(|a| a.as_named(self.ctx, &())); + used_by_this_id.extend(args); + } + // A template instantiation's concrete template argument is // only used if the template declaration uses the // corresponding template parameter. @@ -404,12 +429,12 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> { let params = decl.self_template_params(self.ctx) .unwrap_or(vec![]); + for (arg, param) in args.iter().zip(params.iter()) { - let used_by_definition = self.used - [&inst.template_definition()] + let used_by_def = self.used[&inst.template_definition()] .as_ref() .unwrap(); - if used_by_definition.contains(param) { + if used_by_def.contains(param) { if let Some(named) = arg.as_named(self.ctx, &()) { used_by_this_id.insert(named); } @@ -421,7 +446,12 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> { // parameter usage. _ => { item.trace(self.ctx, &mut |sub_id, edge_kind| { - if sub_id == id || !Self::consider_edge(edge_kind) { + // Ignore ourselves, since union with ourself is a + // no-op. Ignore edges that aren't relevant to the + // analysis. Ignore edges to blacklisted items. + if sub_id == id || + !Self::consider_edge(edge_kind) || + !self.whitelisted_items.contains(&sub_id) { return; } diff --git a/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs b/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs new file mode 100644 index 00000000..91b8e49a --- /dev/null +++ b/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs @@ -0,0 +1,80 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + +pub type RefPtr<T> = T; + +#[repr(C)] +#[derive(Debug, Copy)] +pub struct b { + pub _base: g, +} +#[test] +fn bindgen_test_layout_b() { + assert_eq!(::std::mem::size_of::<b>() , 1usize , concat ! ( + "Size of: " , stringify ! ( b ) )); + assert_eq! (::std::mem::align_of::<b>() , 1usize , concat ! ( + "Alignment of " , stringify ! ( b ) )); +} +impl Clone for b { + fn clone(&self) -> Self { *self } +} +impl Default for b { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +#[repr(C)] +#[derive(Debug, Default, Copy)] +pub struct A { + pub _address: u8, +} +pub type A_a = b; +#[test] +fn bindgen_test_layout_A() { + assert_eq!(::std::mem::size_of::<A>() , 1usize , concat ! ( + "Size of: " , stringify ! ( A ) )); + assert_eq! (::std::mem::align_of::<A>() , 1usize , concat ! ( + "Alignment of " , stringify ! ( A ) )); +} +impl Clone for A { + fn clone(&self) -> Self { *self } +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct e<c> { + pub d: RefPtr<c>, +} +impl <c> Default for e<c> { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct f { + pub _address: u8, +} +#[repr(C)] +#[derive(Debug, Copy)] +pub struct g { + pub h: f, +} +#[test] +fn bindgen_test_layout_g() { + assert_eq!(::std::mem::size_of::<g>() , 1usize , concat ! ( + "Size of: " , stringify ! ( g ) )); + assert_eq! (::std::mem::align_of::<g>() , 1usize , concat ! ( + "Alignment of " , stringify ! ( g ) )); + assert_eq! (unsafe { & ( * ( 0 as * const g ) ) . h as * const _ as usize + } , 0usize , concat ! ( + "Alignment of field: " , stringify ! ( g ) , "::" , stringify + ! ( h ) )); +} +impl Clone for g { + fn clone(&self) -> Self { *self } +} +impl Default for g { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +extern "C" { + #[link_name = "_Z25Servo_Element_GetSnapshotv"] + pub fn Servo_Element_GetSnapshot() -> A; +} diff --git a/tests/headers/issue-584-stylo-template-analysis-panic.hpp b/tests/headers/issue-584-stylo-template-analysis-panic.hpp new file mode 100644 index 00000000..3bf00e5d --- /dev/null +++ b/tests/headers/issue-584-stylo-template-analysis-panic.hpp @@ -0,0 +1,13 @@ +// bindgen-flags: --blacklist-type RefPtr --whitelist-function 'Servo_.*' --raw-line 'pub type RefPtr<T> = T;' -- -std=c++14 +template <class> class RefPtr; +class b; +class A { + typedef b a; +}; +template <class c> class e { RefPtr<c> d; }; +template <class> class f {}; +class g { + f<e<int>> h; +}; +class b : g {}; +A Servo_Element_GetSnapshot(); |