Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strongly type ids #1050

Merged
merged 25 commits into from
Oct 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2a93916
Introduce the `TypeId` newtype over `ItemId`
fitzgen Sep 29, 2017
f5b06ed
Remove unused parameter to `codegen::utils::type_from_named`
fitzgen Sep 29, 2017
dd68ec8
Turn `build_templated_path` into `build_path`
fitzgen Sep 29, 2017
bd232fb
Resolve an `Item` with any id type that converts into an `ItemId`
fitzgen Sep 29, 2017
bd32423
Make `TypeKind::{Alias,TemplateAlias,Array,Pointer,Reference}` use `T…
fitzgen Sep 29, 2017
25ce0f6
Make TemplateInstantiation's definition into a TypeId
fitzgen Sep 29, 2017
2a0cee7
Make functions which take an ItemId generic to take any kind of id
fitzgen Sep 29, 2017
95d9741
Make comp fields contain `TypeId`s
fitzgen Sep 29, 2017
3e869f1
Make a bunch more methods take generic ids
fitzgen Sep 30, 2017
c2e94be
Make base members use TypeId rather than ItemId
fitzgen Sep 30, 2017
d83e732
`instantiate_template` should take a `TypeId` for the template defini…
fitzgen Sep 30, 2017
5e75397
Make `Enum::repr` into a `TypeId`
fitzgen Sep 30, 2017
9ad1e6a
A bunch of parsing things should return TypeId
fitzgen Sep 30, 2017
c780d94
Turn `CompInfo::inner_types` into TypeId
fitzgen Sep 30, 2017
9220ffe
Replacement should use TypeId rather than ItemId
fitzgen Sep 30, 2017
2549d5e
Replace `as_type_id_unchecked` calls with checked calls where possible
fitzgen Sep 30, 2017
c912f55
Introduce ModuleId to strongly type IDs pointing at Modules
fitzgen Sep 30, 2017
9b294d3
Put newtype-of-ItemId boilerplate behind a macro
fitzgen Sep 30, 2017
edee094
Introduce VarId for ids pointing to Var
fitzgen Sep 30, 2017
45db21b
Make `CompInfo::inner_vars` use `VarId` instead of `ItemId`
fitzgen Oct 2, 2017
3ef31e8
Introduce the `FunctionId` newtype for ids pointing to functions
fitzgen Oct 2, 2017
3495d03
Make methods/constructors/destructors use FunctionId
fitzgen Oct 2, 2017
a8c34ed
Tighten up `is_unsized` and `has_vtable` checks to operated on TypeId
fitzgen Oct 2, 2017
3b82786
Make `has_destructor` checks operate on TypeId
fitzgen Oct 2, 2017
5f3bf1f
s/lookup_item_id/lookup/ in method names
fitzgen Oct 2, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 29 additions & 38 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub mod struct_layout;
use self::helpers::attributes;
use self::struct_layout::StructLayoutTracker;

use ir::analysis::HasVtable;
use ir::annotations::FieldAccessorKind;
use ir::comment;
use ir::comp::{Base, Bitfield, BitfieldUnit, CompInfo, CompKind, Field,
Expand Down Expand Up @@ -151,12 +152,12 @@ impl<'a> CodegenResult<'a> {
self.saw_objc = true;
}

fn seen(&self, item: ItemId) -> bool {
self.items_seen.contains(&item)
fn seen<Id: Into<ItemId>>(&self, item: Id) -> bool {
self.items_seen.contains(&item.into())
}

fn set_seen(&mut self, item: ItemId) {
self.items_seen.insert(item);
fn set_seen<Id: Into<ItemId>>(&mut self, item: Id) {
self.items_seen.insert(item.into());
}

fn seen_function(&self, name: &str) -> bool {
Expand Down Expand Up @@ -599,7 +600,7 @@ impl CodeGenerator for Type {
// If this is a known named type, disallow generating anything
// for it too.
let spelling = self.name().expect("Unnamed alias?");
if utils::type_from_named(ctx, spelling, inner).is_some() {
if utils::type_from_named(ctx, spelling).is_some() {
return;
}

Expand Down Expand Up @@ -1546,7 +1547,7 @@ impl CodeGenerator for CompInfo {
// NB: We won't include unsized types in our base chain because they
// would contribute to our size given the dummy field we insert for
// unsized types.
if base_ty.is_unsized(ctx, &base.ty) {
if base_ty.is_unsized(ctx, base.ty) {
continue;
}

Expand Down Expand Up @@ -1625,7 +1626,7 @@ impl CodeGenerator for CompInfo {
warn!("Opaque type without layout! Expect dragons!");
}
}
} else if !is_union && !self.is_unsized(ctx, &item.id()) {
} else if !is_union && !self.is_unsized(ctx, item.id().expect_type_id(ctx)) {
if let Some(padding_field) =
layout.and_then(|layout| struct_layout.pad_struct(layout))
{
Expand All @@ -1649,7 +1650,7 @@ impl CodeGenerator for CompInfo {
//
// NOTE: This check is conveniently here to avoid the dummy fields we
// may add for unused template parameters.
if self.is_unsized(ctx, &item.id()) {
if self.is_unsized(ctx, item.id().expect_type_id(ctx)) {
let has_address = if is_opaque {
// Generate the address field if it's an opaque type and
// couldn't determine the layout of the blob.
Expand Down Expand Up @@ -1758,9 +1759,8 @@ impl CodeGenerator for CompInfo {
// FIXME when [issue #465](https://github.com/rust-lang-nursery/rust-bindgen/issues/465) ready
let too_many_base_vtables = self.base_members()
.iter()
.filter(|base| ctx.lookup_item_id_has_vtable(&base.ty))
.count() >
1;
.filter(|base| base.ty.has_vtable(ctx))
.count() > 1;

let should_skip_field_offset_checks = is_opaque ||
too_many_base_vtables;
Expand Down Expand Up @@ -2728,27 +2728,33 @@ where
}
}

impl TryToOpaque for ItemId {
impl<T> TryToOpaque for T
where
T: Copy + Into<ItemId>
{
type Extra = ();

fn try_get_layout(
&self,
ctx: &BindgenContext,
_: &(),
) -> error::Result<Layout> {
ctx.resolve_item(*self).try_get_layout(ctx, &())
ctx.resolve_item((*self).into()).try_get_layout(ctx, &())
}
}

impl TryToRustTy for ItemId {
impl<T> TryToRustTy for T
where
T: Copy + Into<ItemId>
{
type Extra = ();

fn try_to_rust_ty(
&self,
ctx: &BindgenContext,
_: &(),
) -> error::Result<quote::Tokens> {
ctx.resolve_item(*self).try_to_rust_ty(ctx, &())
ctx.resolve_item((*self).into()).try_to_rust_ty(ctx, &())
}
}

Expand Down Expand Up @@ -2889,8 +2895,8 @@ impl TryToRustTy for Type {
inst.try_to_rust_ty(ctx, item)
}
TypeKind::ResolvedTypeRef(inner) => inner.try_to_rust_ty(ctx, &()),
TypeKind::TemplateAlias(inner, _) |
TypeKind::Alias(inner) => {
TypeKind::TemplateAlias(..) |
TypeKind::Alias(..) => {
let template_params = item.used_template_params(ctx)
.unwrap_or(vec![])
.into_iter()
Expand All @@ -2903,12 +2909,11 @@ impl TryToRustTy for Type {
} else if let Some(ty) = utils::type_from_named(
ctx,
spelling,
inner,
)
{
Ok(ty)
} else {
utils::build_templated_path(item, ctx, vec![]) //template_params)
utils::build_path(item, ctx)
}
}
TypeKind::Comp(ref info) => {
Expand All @@ -2919,8 +2924,7 @@ impl TryToRustTy for Type {
return self.try_to_opaque(ctx, item);
}

// let template_params = template_params.unwrap_or(vec![]);
utils::build_templated_path(item, ctx, vec![])
utils::build_path(item, ctx)
}
TypeKind::Opaque => self.try_to_opaque(ctx, item),
TypeKind::BlockPointer => {
Expand Down Expand Up @@ -3326,8 +3330,8 @@ pub fn codegen(context: &mut BindgenContext) -> Vec<quote::Tokens> {
}

mod utils {
use super::{ToRustTyOrOpaque, TryToRustTy, error};
use ir::context::{BindgenContext, ItemId};
use super::{ToRustTyOrOpaque, error};
use ir::context::BindgenContext;
use ir::function::FunctionSig;
use ir::item::{Item, ItemCanonicalPath};
use ir::ty::TypeKind;
Expand Down Expand Up @@ -3549,28 +3553,16 @@ mod utils {
result.extend(old_items.into_iter());
}

pub fn build_templated_path(
pub fn build_path(
item: &Item,
ctx: &BindgenContext,
template_params: Vec<ItemId>,
) -> error::Result<quote::Tokens> {
let path = item.namespace_aware_canonical_path(ctx);

let template_params = template_params
.iter()
.map(|param| param.try_to_rust_ty(ctx, &()))
.collect::<error::Result<Vec<_>>>()?;

let mut tokens = quote! {};
tokens.append_separated(path.into_iter().map(quote::Ident::new), "::");

if template_params.is_empty() {
Ok(tokens)
} else {
Ok(quote! {
#tokens < #( #template_params ),* >
})
}
Ok(tokens)
}

fn primitive_ty(ctx: &BindgenContext, name: &str) -> quote::Tokens {
Expand All @@ -3583,7 +3575,6 @@ mod utils {
pub fn type_from_named(
ctx: &BindgenContext,
name: &str,
_inner: ItemId,
) -> Option<quote::Tokens> {
// FIXME: We could use the inner item to check this is really a
// primitive type but, who the heck overrides these anyway?
Expand Down
10 changes: 6 additions & 4 deletions src/ir/analysis/derive_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ impl<'ctx> CannotDeriveCopy<'ctx> {
}
}

fn insert(&mut self, id: ItemId) -> ConstrainResult {
fn insert<Id: Into<ItemId>>(&mut self, id: Id) -> ConstrainResult {
let id = id.into();
trace!("inserting {:?} into the cannot_derive_copy set", id);

let was_not_already_in_set = self.cannot_derive_copy.insert(id);
Expand All @@ -89,7 +90,8 @@ impl<'ctx> CannotDeriveCopy<'ctx> {

/// A type is not `Copy` if we've determined it is not copy, or if it is
/// blacklisted.
fn is_not_copy(&self, id: ItemId) -> bool {
fn is_not_copy<Id: Into<ItemId>>(&self, id: Id) -> bool {
let id = id.into();
self.cannot_derive_copy.contains(&id) ||
!self.ctx.whitelisted_items().contains(&id)
}
Expand Down Expand Up @@ -222,7 +224,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveCopy<'ctx> {
// NOTE: Take into account that while unions in C and C++ are copied by
// default, the may have an explicit destructor in C++, so we can't
// defer this check just for the union case.
if self.ctx.lookup_item_id_has_destructor(&id) {
if self.ctx.lookup_has_destructor(id.expect_type_id(self.ctx)) {
trace!(" comp has destructor which cannot derive copy");
return self.insert(id);
}
Expand Down Expand Up @@ -307,7 +309,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveCopy<'ctx> {
"The early ty.is_opaque check should have handled this case"
);
let def_cannot_derive = self.is_not_copy(
template.template_definition(),
template.template_definition()
);
if def_cannot_derive {
trace!(
Expand Down
8 changes: 5 additions & 3 deletions src/ir/analysis/derive_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ impl<'ctx> CannotDeriveDebug<'ctx> {
}
}

fn insert(&mut self, id: ItemId) -> ConstrainResult {
fn insert<Id: Into<ItemId>>(&mut self, id: Id) -> ConstrainResult {
let id = id.into();
trace!("inserting {:?} into the cannot_derive_debug set", id);

let was_not_already_in_set = self.cannot_derive_debug.insert(id);
Expand All @@ -90,7 +91,8 @@ impl<'ctx> CannotDeriveDebug<'ctx> {

/// A type is not `Debug` if we've determined it is not debug, or if it is
/// blacklisted.
fn is_not_debug(&self, id: ItemId) -> bool {
fn is_not_debug<Id: Into<ItemId>>(&self, id: Id) -> bool {
let id = id.into();
self.cannot_derive_debug.contains(&id) ||
!self.ctx.whitelisted_items().contains(&id)
}
Expand Down Expand Up @@ -325,7 +327,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveDebug<'ctx> {
"The early ty.is_opaque check should have handled this case"
);
let def_cannot_derive = self.is_not_debug(
template.template_definition(),
template.template_definition()
);
if def_cannot_derive {
trace!(
Expand Down
12 changes: 7 additions & 5 deletions src/ir/analysis/derive_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ impl<'ctx> CannotDeriveDefault<'ctx> {
}
}

fn insert(&mut self, id: ItemId) -> ConstrainResult {
fn insert<Id: Into<ItemId>>(&mut self, id: Id) -> ConstrainResult {
let id = id.into();
trace!("inserting {:?} into the cannot_derive_default set", id);

let was_not_already_in_set = self.cannot_derive_default.insert(id);
Expand All @@ -85,7 +86,8 @@ impl<'ctx> CannotDeriveDefault<'ctx> {
ConstrainResult::Changed
}

fn is_not_default(&self, id: ItemId) -> bool {
fn is_not_default<Id: Into<ItemId>>(&self, id: Id) -> bool {
let id = id.into();
self.cannot_derive_default.contains(&id) ||
!self.ctx.whitelisted_items().contains(&id)
}
Expand Down Expand Up @@ -288,7 +290,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveDefault<'ctx> {

let bases_cannot_derive =
info.base_members().iter().any(|base| {
!self.ctx.whitelisted_items().contains(&base.ty) ||
!self.ctx.whitelisted_items().contains(&base.ty.into()) ||
self.is_not_default(base.ty)
});
if bases_cannot_derive {
Expand All @@ -303,7 +305,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveDefault<'ctx> {
info.fields().iter().any(|f| match *f {
Field::DataMember(ref data) => {
!self.ctx.whitelisted_items().contains(
&data.ty(),
&data.ty().into(),
) ||
self.is_not_default(data.ty())
}
Expand All @@ -318,7 +320,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveDefault<'ctx> {

bfu.bitfields().iter().any(|b| {
!self.ctx.whitelisted_items().contains(
&b.ty(),
&b.ty().into(),
) ||
self.is_not_default(b.ty())
})
Expand Down
23 changes: 12 additions & 11 deletions src/ir/analysis/derive_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ impl<'ctx> CannotDeriveHash<'ctx> {
}
}

fn insert(&mut self, id: ItemId) -> ConstrainResult {
fn insert<Id: Into<ItemId>>(&mut self, id: Id) -> ConstrainResult {
let id = id.into();
trace!("inserting {:?} into the cannot_derive_hash set", id);

let was_not_already_in_set = self.cannot_derive_hash.insert(id);
Expand Down Expand Up @@ -178,7 +179,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {
}

TypeKind::Array(t, len) => {
if self.cannot_derive_hash.contains(&t) {
if self.cannot_derive_hash.contains(&t.into()) {
trace!(
" arrays of T for which we cannot derive Hash \
also cannot derive Hash"
Expand Down Expand Up @@ -222,7 +223,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {
TypeKind::ResolvedTypeRef(t) |
TypeKind::TemplateAlias(t, _) |
TypeKind::Alias(t) => {
if self.cannot_derive_hash.contains(&t) {
if self.cannot_derive_hash.contains(&t.into()) {
trace!(
" aliases and type refs to T which cannot derive \
Hash also cannot derive Hash"
Expand Down Expand Up @@ -263,8 +264,8 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {

let bases_cannot_derive =
info.base_members().iter().any(|base| {
!self.ctx.whitelisted_items().contains(&base.ty) ||
self.cannot_derive_hash.contains(&base.ty)
!self.ctx.whitelisted_items().contains(&base.ty.into()) ||
self.cannot_derive_hash.contains(&base.ty.into())
});
if bases_cannot_derive {
trace!(
Expand All @@ -278,9 +279,9 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {
info.fields().iter().any(|f| match *f {
Field::DataMember(ref data) => {
!self.ctx.whitelisted_items().contains(
&data.ty(),
&data.ty().into(),
) ||
self.cannot_derive_hash.contains(&data.ty())
self.cannot_derive_hash.contains(&data.ty().into())
}
Field::Bitfields(ref bfu) => {
if bfu.layout().align > RUST_DERIVE_IN_ARRAY_LIMIT {
Expand All @@ -293,9 +294,9 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {

bfu.bitfields().iter().any(|b| {
!self.ctx.whitelisted_items().contains(
&b.ty(),
&b.ty().into(),
) ||
self.cannot_derive_hash.contains(&b.ty())
self.cannot_derive_hash.contains(&b.ty().into())
})
}
});
Expand All @@ -311,7 +312,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {
TypeKind::TemplateInstantiation(ref template) => {
let args_cannot_derive =
template.template_arguments().iter().any(|arg| {
self.cannot_derive_hash.contains(&arg)
self.cannot_derive_hash.contains(&arg.into())
});
if args_cannot_derive {
trace!(
Expand All @@ -326,7 +327,7 @@ impl<'ctx> MonotoneFramework for CannotDeriveHash<'ctx> {
"The early ty.is_opaque check should have handled this case"
);
let def_cannot_derive = self.cannot_derive_hash.contains(
&template.template_definition(),
&template.template_definition().into(),
);
if def_cannot_derive {
trace!(
Expand Down
Loading