Skip to content

Commit

Permalink
Auto merge of #1049 - fitzgen:wrong-parent-id-with-nested-class-decla…
Browse files Browse the repository at this point in the history
…rations, r=emilio

ir: Prefer using known semantic parents

When choosing a parent ID for a type that we are parsing, prefer known semantic parents over the provided parent ID. It seems like we shouldn't even be passing explicit parent IDs around (they're often buggy), and instead should expand the `known_semantic_parent` infrastructure, but I'll leave that to some future work.

Fixes #1048

r? @emilio
  • Loading branch information
bors-servo authored Oct 1, 2017
2 parents c160b20 + 64d73c2 commit e9169b3
Show file tree
Hide file tree
Showing 5 changed files with 519 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,7 @@ impl CompInfo {

let inner = Item::parse(cur, Some(potential_id), ctx)
.expect("Inner ClassDecl");
assert_eq!(ctx.resolve_item(inner).parent_id(), potential_id);

ci.inner_types.push(inner);

Expand Down
22 changes: 13 additions & 9 deletions src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,25 +751,29 @@ impl BindgenContext {
let typerefs = self.collect_typerefs();

for (id, ty, loc, parent_id) in typerefs {
let _resolved =
{
let resolved = Item::from_ty(&ty, loc, parent_id, self)
let _resolved = {
let resolved = Item::from_ty(&ty, loc, parent_id, self)
.unwrap_or_else(|_| {
warn!("Could not resolve type reference, falling back \
to opaque blob");
Item::new_opaque_type(self.next_item_id(), &ty, self)
});
let item = self.items.get_mut(&id).unwrap();

*item.kind_mut().as_type_mut().unwrap().kind_mut() =
TypeKind::ResolvedTypeRef(resolved);
resolved
};
let item = self.items.get_mut(&id).unwrap();
*item.kind_mut().as_type_mut().unwrap().kind_mut() =
TypeKind::ResolvedTypeRef(resolved);

resolved
};

// Something in the STL is trolling me. I don't need this assertion
// right now, but worth investigating properly once this lands.
//
// debug_assert!(self.items.get(&resolved).is_some(), "How?");
//
// if let Some(parent_id) = parent_id {
// assert_eq!(self.items[&resolved].parent_id(), parent_id);
// }
}
}

Expand Down Expand Up @@ -953,7 +957,7 @@ impl BindgenContext {
self.compute_bitfield_units();
self.process_replacements();
}

self.deanonymize_fields();

// And assert once again, because resolving type refs and processing
Expand Down
3 changes: 2 additions & 1 deletion src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,8 @@ impl ClangItemParser for Item {
ctx,
));
}
parent_id.or_else(|| ctx.known_semantic_parent(definition))
ctx.known_semantic_parent(definition)
.or(parent_id)
.unwrap_or(ctx.current_module())
}
None => relevant_parent_id,
Expand Down
Loading

0 comments on commit e9169b3

Please sign in to comment.