Skip to content

Commit

Permalink
Avoid recursion more robustly when spotting DQTs.
Browse files Browse the repository at this point in the history 
Code to identify relevant dependent qualified type names
is run with a given 'comp' loaned. We previously tried to avoid
recursing into that loaned ItemId, but this didn't work in some
circumstances when we resolved one ItemId to get another.

This takes an alternative approach where we fallibly try
to resolve the item.
  • Loading branch information
@adetaylor
adetaylor committed Apr 23, 2021
1 parent 5642a8f commit f660959
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 69 deletions.
13 changes: 3 additions & 10 deletions src/ir/comp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,6 @@ impl CompFields {
fn identify_associated_type_fields(
&mut self,
ctx: &BindgenContext,
do_not_recurse_into: TypeId,
) -> (Vec<TypeId>, Vec<String>) {
let mut no_fields = vec![];
let mut type_results = vec![];
Expand All @@ -890,11 +889,7 @@ impl CompFields {
// but returns None (as far as I can see) so we never
// make a EdgeKind::ContainedDependentQualifiedType
let depended_qualified_type_fields = resolved
.get_dependent_qualified_type_field_names(
ctx,
tid,
do_not_recurse_into,
);
.get_dependent_qualified_type_field_names(ctx, tid);
if !depended_qualified_type_fields.is_empty() {
let (tids, mut names): (Vec<_>, Vec<_>) =
depended_qualified_type_fields.into_iter().unzip();
Expand Down Expand Up @@ -1709,11 +1704,9 @@ impl CompInfo {
pub fn identify_associated_type_fields(
&mut self,
ctx: &BindgenContext,
do_not_recurse_into: TypeId,
) -> Vec<String> {
let (mut typeids, fieldnames) = self
.fields
.identify_associated_type_fields(ctx, do_not_recurse_into);
let (mut typeids, fieldnames) =
self.fields.identify_associated_type_fields(ctx);
self.dependent_qualified_type_params.append(&mut typeids);
fieldnames
}
Expand Down
50 changes: 31 additions & 19 deletions src/ir/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,6 @@ If you encounter an error missing from this list, please file an issue or a PR!"

self.resolve_typerefs();
self.compute_bitfield_units();
self.identify_associated_type_params();
self.process_replacements();

self.deanonymize_fields();
Expand Down Expand Up @@ -1179,6 +1178,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"
self.compute_has_float();
self.compute_cannot_derive_hash();
self.compute_cannot_derive_partialord_partialeq_or_eq();
self.identify_associated_type_params();

let ret = cb(&self);
(ret, self.options)
Expand All @@ -1198,15 +1198,14 @@ If you encounter an error missing from this list, please file an issue or a PR!"
fn identify_associated_type_params(&mut self) {
let mut used_inner_type_names = HashSet::default();
for id in self.get_comp_item_ids() {
let tid = id.expect_type_id(self);
self.with_loaned_item(id, |ctx, item| {
let fieldnames = item
.kind_mut()
.as_type_mut()
.unwrap()
.as_comp_mut()
.unwrap()
.identify_associated_type_fields(ctx, tid);
.identify_associated_type_fields(ctx);
used_inner_type_names.extend(fieldnames.into_iter());
});
}
Expand Down Expand Up @@ -2748,27 +2747,40 @@ impl ItemResolver {

/// Finish configuring and perform the actual item resolution.
pub fn resolve(self, ctx: &BindgenContext) -> &Item {
match self.resolve_fallible(ctx) {
Some(item) => item,
None => panic!("Not an item: {:?}", self.id),
}
}

/// Finish configuring and perform the actual item resolution.
pub fn resolve_fallible(self, ctx: &BindgenContext) -> Option<&Item> {
assert!(ctx.collected_typerefs());

let mut id = self.id;
loop {
let item = ctx.resolve_item(id);
let ty_kind = item.as_type().map(|t| t.kind());
match ty_kind {
Some(&TypeKind::ResolvedTypeRef(next_id))
if self.through_type_refs =>
{
id = next_id.into();
}
// We intentionally ignore template aliases here, as they are
// more complicated, and don't represent a simple renaming of
// some type.
Some(&TypeKind::Alias(next_id))
if self.through_type_aliases =>
{
id = next_id.into();
let item = ctx.resolve_item_fallible(id);
match item {
None => return None,
Some(item) => {
let ty_kind = item.as_type().map(|t| t.kind());
match ty_kind {
Some(&TypeKind::ResolvedTypeRef(next_id))
if self.through_type_refs =>
{
id = next_id.into();
}
// We intentionally ignore template aliases here, as they are
// more complicated, and don't represent a simple renaming of
// some type.
Some(&TypeKind::Alias(next_id))
if self.through_type_aliases =>
{
id = next_id.into();
}
_ => return Some(item),
}
}
_ => return item,
}
}
}
Expand Down
65 changes: 25 additions & 40 deletions src/ir/ty.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,57 +138,42 @@ impl Type {
&self,
ctx: &BindgenContext,
tid: TypeId,
do_not_recurse_into: TypeId,
) -> Vec<(TypeId, String)> {
match &self.kind {
let types_to_recurse_into: Vec<TypeId> = match &self.kind {
TypeKind::DependentQualifiedType(_, field_name) => {
vec![(tid, field_name.clone())]
return vec![(tid, field_name.clone())];
}
TypeKind::TemplateInstantiation(ta) => ta
.template_arguments()
.template_arguments() // prob here
.iter()
.chain(std::iter::once(&ta.template_definition()))
.filter(|tid| *tid != &do_not_recurse_into)
.map(|tid| {
let item = tid
.into_resolver()
.through_type_refs()
.through_type_aliases()
.resolve(ctx);
let tid = item.id().expect_type_id(ctx);
item.expect_type()
.get_dependent_qualified_type_field_names(
ctx,
tid,
do_not_recurse_into,
)
.into_iter()
})
.flatten()
.cloned()
.chain(std::iter::once(ta.template_definition()))
.collect(),
TypeKind::TemplateAlias(inner_type, params) => params
.iter()
.chain(std::iter::once(inner_type))
.filter(|tid| *tid != &do_not_recurse_into)
.map(|tid| {
let item = tid
.into_resolver()
.cloned()
.collect(),
_ => return vec![],
};
types_to_recurse_into
.into_iter()
.filter_map(
|tid| {
tid.into_resolver()
.through_type_refs()
.through_type_aliases()
.resolve(ctx);
let tid = item.id().expect_type_id(ctx);
item.expect_type()
.get_dependent_qualified_type_field_names(
ctx,
tid,
do_not_recurse_into,
)
.into_iter()
})
.flatten()
.collect(),
_ => vec![],
}
.resolve_fallible(ctx)
}, // avoid recursing back to the item we have loaned.
)
.map(|item| {
let tid = item.id().expect_type_id(ctx);
item.expect_type()
.get_dependent_qualified_type_field_names(ctx, tid)
.into_iter()
})
.flatten()
.collect()
}

/// Is this a template instantiation type?
Expand Down

0 comments on commit f660959

Please sign in to comment.