Skip to content

Change return type of TemplateParameters' methods from Option to just… #1064

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Change return type of TemplateParameters' methods from Option to just… #1064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e8f3b4
Change return type of TemplateParameters' methods from Option to just…
hgallagher1993 Oct 7, 2017
8dcaa35
Fixed logic error in codegen/mod.rs and updated tests
hgallagher1993 Oct 12, 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
112 changes: 57 additions & 55 deletions src/codegen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,20 +284,21 @@ impl AppendImplicitTemplateParams for quote::Tokens {
_ => {},
}

if let Some(params) = item.used_template_params(ctx) {
if params.is_empty() {
return;
}
let params = item.used_template_params(ctx);

let params = params.into_iter().map(|p| {
p.try_to_rust_ty(ctx, &())
.expect("template params cannot fail to be a rust type")
});

self.append(quote! {
< #( #params ),* >
});
if params.is_empty() {
return;
}

let params = params.into_iter().map(|p| {
p.try_to_rust_ty(ctx, &())
.expect("template params cannot fail to be a rust type")
});

self.append(quote! {
< #( #params ),* >
});

}
}

Expand Down Expand Up @@ -461,11 +462,9 @@ impl CodeGenerator for Var {
// We can't generate bindings to static variables of templates. The
// number of actual variables for a single declaration are open ended
// and we don't know what instantiations do or don't exist.
let type_params = item.all_template_params(ctx);
if let Some(params) = type_params {
if !params.is_empty() {
return;
}

if !item.all_template_params(ctx).is_empty() {
return
}

let ty = self.ty().to_rust_ty_or_opaque(ctx, &());
Expand Down Expand Up @@ -619,15 +618,15 @@ impl CodeGenerator for Type {
return;
}

let mut outer_params = item.used_template_params(ctx)
.and_then(|ps| if ps.is_empty() {
None
let mut outer_params =
if !item.used_template_params(ctx).is_empty() {
item.used_template_params(ctx).clone()
} else {
Some(ps)
});
Vec::new()
};

let inner_rust_type = if item.is_opaque(ctx, &()) {
outer_params = None;
outer_params = Vec::new();
self.to_opaque(ctx, item)
} else {
// Its possible that we have better layout information than
Expand Down Expand Up @@ -682,7 +681,7 @@ impl CodeGenerator for Type {
'A'...'Z' | 'a'...'z' | '0'...'9' | ':' | '_' | ' ' => true,
_ => false,
}) &&
outer_params.is_none() &&
outer_params.is_empty() &&
inner_item.expect_type().canonical_type(ctx).is_enum()
{
tokens.append(quote! {
Expand All @@ -701,8 +700,8 @@ impl CodeGenerator for Type {
pub type #rust_name
});

if let Some(params) = outer_params {
let params: Vec<_> = params.into_iter()
if !outer_params.is_empty() {
let params: Vec<_> = outer_params.into_iter()
.filter_map(|p| p.as_template_param(ctx, &()))
.collect();
if params.iter().any(|p| ctx.resolve_type(*p).is_invalid_type_param()) {
Expand Down Expand Up @@ -1401,7 +1400,7 @@ impl CodeGenerator for CompInfo {
//
// NB: We generate a proper struct to avoid struct/function name
// collisions.
if self.is_forward_declaration() && used_template_params.is_none() {
if self.is_forward_declaration() && used_template_params.is_empty() {
let struct_name = item.canonical_name(ctx);
let struct_name = ctx.rust_ident_raw(struct_name);
let tuple_struct = quote! {
Expand Down Expand Up @@ -1584,7 +1583,9 @@ impl CodeGenerator for CompInfo {

let mut generic_param_names = vec![];

if let Some(ref params) = used_template_params {
let ref params = used_template_params;

if !params.is_empty() {
for (idx, ty) in params.iter().enumerate() {
let param = ctx.resolve_type(*ty);
let name = param.name().unwrap();
Expand All @@ -1594,13 +1595,14 @@ impl CodeGenerator for CompInfo {
let prefix = ctx.trait_prefix();
let field_name = ctx.rust_ident(format!("_phantom_{}", idx));
fields.push(quote! {
pub #field_name : ::#prefix::marker::PhantomData<
::#prefix::cell::UnsafeCell<#ident>
> ,
});
pub #field_name : ::#prefix::marker::PhantomData<
::#prefix::cell::UnsafeCell<#ident>
> ,
});
}
}


let generics = if !generic_param_names.is_empty() {
let generic_param_names = generic_param_names.clone();
quote! {
Expand Down Expand Up @@ -1642,7 +1644,7 @@ impl CodeGenerator for CompInfo {
ctx.options().derive_copy
{
derives.push("Copy");
if used_template_params.is_some() {
if !used_template_params.is_empty() {
// FIXME: This requires extra logic if you have a big array in a
// templated struct. The reason for this is that the magic:
// fn clone(&self) -> Self { *self }
Expand Down Expand Up @@ -1726,7 +1728,7 @@ impl CodeGenerator for CompInfo {
);
}

if used_template_params.is_none() {
if used_template_params.is_empty() {
if !is_opaque {
for var in self.inner_vars() {
ctx.resolve_item(*var).codegen(ctx, result, &());
Expand Down Expand Up @@ -2928,10 +2930,10 @@ impl TryToRustTy for Type {
TypeKind::TemplateAlias(..) |
TypeKind::Alias(..) => {
let template_params = item.used_template_params(ctx)
.unwrap_or(vec![])
.into_iter()
.filter(|param| param.is_template_param(ctx, &()))
.collect::<Vec<_>>();

.into_iter()
.filter(|param| param.is_template_param(ctx, &()))
.collect::<Vec<_>>();

let spelling = self.name().expect("Unnamed alias?");
if item.is_opaque(ctx, &()) && !template_params.is_empty() {
Expand All @@ -2949,7 +2951,7 @@ impl TryToRustTy for Type {
TypeKind::Comp(ref info) => {
let template_params = item.used_template_params(ctx);
if info.has_non_type_template_params() ||
(item.is_opaque(ctx, &()) && template_params.is_some())
(item.is_opaque(ctx, &()) && !template_params.is_empty())
{
return self.try_to_opaque(ctx, item);
}
Expand Down Expand Up @@ -3043,18 +3045,18 @@ impl TryToRustTy for TemplateInstantiation {
let def_path = def.namespace_aware_canonical_path(ctx);
ty.append_separated(def_path.into_iter().map(|p| ctx.rust_ident(p)), "::");

let def_params = match def.self_template_params(ctx) {
Some(params) => params,
None => {
// This can happen if we generated an opaque type for a partial
// template specialization, and we've hit an instantiation of
// that partial specialization.
extra_assert!(
def.is_opaque(ctx, &())
);
return Err(error::Error::InstantiationOfOpaqueType);
}
};
let def_params = def.self_template_params(ctx);

if def_params.is_empty() {
// This can happen if we generated an opaque type for a partial
// template specialization, and we've hit an instantiation of
// that partial specialization.
extra_assert!(
def.is_opaque(ctx, &())
);
return Err(error::Error::InstantiationOfOpaqueType);

}

// TODO: If the definition type is a template class/struct
// definition's member template definition, it could rely on
Expand Down Expand Up @@ -3132,12 +3134,12 @@ impl CodeGenerator for Function {
// instantiations is open ended and we have no way of knowing which
// monomorphizations actually exist.
let type_params = item.all_template_params(ctx);
if let Some(params) = type_params {
if !params.is_empty() {
return;
}

if !type_params.is_empty() {
return;
}


let name = self.name();
let mut canonical_name = item.canonical_name(ctx);
let mangled_name = self.mangled_name();
Expand Down
4 changes: 2 additions & 2 deletions src/ir/analysis/derive_copy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ impl<'ctx> MonotoneFramework for CannotDeriveCopy<'ctx> {
}

// https://github.com/rust-lang/rust/issues/36640
if info.self_template_params(self.ctx).is_some() ||
item.used_template_params(self.ctx).is_some()
if !info.self_template_params(self.ctx).is_empty() ||
!item.used_template_params(self.ctx).is_empty()
{
trace!(
" comp cannot derive copy because issue 36640"
Expand Down
5 changes: 2 additions & 3 deletions src/ir/analysis/template_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ impl<'ctx> UsedTemplateParameters<'ctx> {
let decl = self.ctx.resolve_type(instantiation.template_definition());
let args = instantiation.template_arguments();

let params = decl.self_template_params(self.ctx).unwrap_or(vec![]);
let params = decl.self_template_params(self.ctx);

debug_assert!(this_id != instantiation.template_definition());
let used_by_def = self.used
Expand Down Expand Up @@ -418,8 +418,7 @@ impl<'ctx> MonotoneFramework for UsedTemplateParameters<'ctx> {
// Although template definitions should always have
// template parameters, there is a single exception:
// opaque templates. Hence the unwrap_or.
let params =
decl.self_template_params(ctx).unwrap_or(vec![]);
let params = decl.self_template_params(ctx);

for (arg, param) in args.iter().zip(params.iter()) {
let arg = arg.into_resolver()
Expand Down
10 changes: 3 additions & 7 deletions src/ir/comp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1585,12 +1585,8 @@ impl TemplateParameters for CompInfo {
fn self_template_params(
&self,
_ctx: &BindgenContext,
) -> Option<Vec<TypeId>> {
if self.template_params.is_empty() {
None
} else {
Some(self.template_params.clone())
}
) -> Vec<TypeId> {
self.template_params.clone()
}
}

Expand All @@ -1601,7 +1597,7 @@ impl Trace for CompInfo {
where
T: Tracer,
{
let params = item.all_template_params(context).unwrap_or(vec![]);
let params = item.all_template_params(context);
for p in params {
tracer.visit_kind(p.into(), EdgeKind::TemplateParameterDefinition);
}
Expand Down
72 changes: 37 additions & 35 deletions src/ir/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1306,10 +1306,14 @@ impl BindgenContext {
let mut used_params = HashMap::new();
for &id in self.whitelisted_items() {
used_params.entry(id).or_insert(
id.self_template_params(self).map_or(
Default::default(),
|params| params.into_iter().map(|p| p.into()).collect(),
),
if !id.self_template_params(self).is_empty() {
id.self_template_params(self)
.into_iter()
.map(|p| p.into())
.collect()
} else {
Default::default()
}
);
}
self.used_template_parameters = Some(used_params);
Expand Down Expand Up @@ -1492,14 +1496,13 @@ impl BindgenContext {
.and_then(|canon_decl| {
self.get_resolved_type(&canon_decl).and_then(
|template_decl_id| {
template_decl_id.num_self_template_params(self).map(
|num_params| {
(
*canon_decl.cursor(),
template_decl_id.into(),
num_params,
)
},
Some(
(
*canon_decl.cursor(),
template_decl_id.into(),
template_decl_id
.num_self_template_params(self),
)
)
},
)
Expand All @@ -1521,14 +1524,13 @@ impl BindgenContext {
.cloned()
})
.and_then(|template_decl| {
template_decl.num_self_template_params(self).map(
|num_template_params| {
(
*template_decl.decl(),
template_decl.id(),
num_template_params,
)
},
Some(
(
*template_decl.decl(),
template_decl.id(),
template_decl
.num_self_template_params(self),
)
)
})
})
Expand Down Expand Up @@ -1576,16 +1578,16 @@ impl BindgenContext {
) -> Option<TypeId> {
use clang_sys;

let num_expected_args = match self.resolve_type(template)
.num_self_template_params(self) {
Some(n) => n,
None => {
warn!(
"Tried to instantiate a template for which we could not \
determine any template parameters"
);
return None;
}
let num_expected_args = self.resolve_type(template)
.num_self_template_params(self);

if num_expected_args == 0 {
warn!(
"Tried to instantiate a template for which we could not \
determine any template parameters"
);

return None
};

let mut args = vec![];
Expand Down Expand Up @@ -2562,13 +2564,13 @@ impl TemplateParameters for PartialType {
fn self_template_params(
&self,
_ctx: &BindgenContext,
) -> Option<Vec<TypeId>> {
) -> Vec<TypeId> {
// Maybe at some point we will eagerly parse named types, but for now we
// don't and this information is unavailable.
None
Vec::new()
}

fn num_self_template_params(&self, _ctx: &BindgenContext) -> Option<usize> {
fn num_self_template_params(&self, _ctx: &BindgenContext) -> usize {
// Wouldn't it be nice if libclang would reliably give us this
// information‽
match self.decl().kind() {
Expand All @@ -2587,9 +2589,9 @@ impl TemplateParameters for PartialType {
};
clang_sys::CXChildVisit_Continue
});
Some(num_params)
num_params
}
_ => None,
_ => 0,
}
}
}
Loading