Skip to content

First steps to fix issue #57 #282

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

Merged
merged 4 commits into from
Nov 21, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
38 changes: 36 additions & 2 deletions libbindgen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,43 @@ impl CodeGenerator for CompInfo {
// also don't output template specializations, neither total or partial.
//
// TODO: Generate layout tests for template specializations, yay!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Remove this TODO.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: And this newline.

// TODO (imp): I will keep it like that right now and move it to function later
if self.has_non_type_template_params() ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should still bail out if has_non_type_template_params, I don't think a lot of stuff will happen, since they probably don't have layout, but still seems a bit clearer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please divide this condition as discussed earlier:

if self.has_non_type_template_params() {
    return;
}

if self.is_template_specialization() {
    // ...
    return;
}

self.is_template_specialization() {
return;
self.is_template_specialization() {
let layout = item.kind().expect_type().layout(ctx);
let canonical_name = item.canonical_name(ctx);

if let Some(layout) = layout {

let mut types = String::new();

for arg in self.template_args() {
if let Some(name) = ctx.resolve_type(*arg).name() {
// hope this isn't bad
types.push_str(format!("_{}", name).as_str());
}
}

let fn_name = format!("bindgen_test_layout_template_{}{}", canonical_name, types);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ok probably, but we could just have done something like:

let fn_name = format!("__bindgen_test_layout_template_{}", item.id().as_usize());

This way is prettier, though a bit more likely to cause conflicts.

let fn_name = ctx.rust_ident_raw(&fn_name);
let ident = item.to_rust_ty(ctx);
let prefix = ctx.trait_prefix();
let size_of_expr = quote_expr!(ctx.ext_cx(),
::$prefix::mem::size_of::<$ident>());
let align_of_expr = quote_expr!(ctx.ext_cx(),
::$prefix::mem::align_of::<$ident>());
let size = layout.size;
let align = layout.align;
let item = quote_item!(ctx.ext_cx(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should try to reuse this code with the other tests, that way it's easier to change in the future if needed.

#[test]
fn $fn_name() {
assert_eq!($size_of_expr, $size);
assert_eq!($align_of_expr, $align);
}).unwrap();
result.push(item);
}
return;
}

let applicable_template_args = item.applicable_template_args(ctx);
Expand Down
7 changes: 7 additions & 0 deletions libbindgen/tests/expectations/tests/anon_union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,10 @@ fn bindgen_test_layout_ErrorResult() {
impl Clone for ErrorResult {
fn clone(&self) -> Self { *self }
}
#[test]
fn bindgen_test_layout_template_TErrorResult_int() {
assert_eq!(::std::mem::size_of::<TErrorResult<::std::os::raw::c_int>>() ,
24usize);
assert_eq!(::std::mem::align_of::<TErrorResult<::std::os::raw::c_int>>() ,
8usize);
}
7 changes: 7 additions & 0 deletions libbindgen/tests/expectations/tests/class_with_dtor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,10 @@ fn bindgen_test_layout_WithoutDtor() {
assert_eq!(::std::mem::size_of::<WithoutDtor>() , 8usize);
assert_eq!(::std::mem::align_of::<WithoutDtor>() , 8usize);
}
#[test]
fn bindgen_test_layout_template_HandleWithDtor_int() {
assert_eq!(::std::mem::size_of::<HandleWithDtor<::std::os::raw::c_int>>()
, 8usize);
assert_eq!(::std::mem::align_of::<HandleWithDtor<::std::os::raw::c_int>>()
, 8usize);
}
12 changes: 12 additions & 0 deletions libbindgen/tests/expectations/tests/crtp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ fn bindgen_test_layout_Derived() {
impl Clone for Derived {
fn clone(&self) -> Self { *self }
}
#[test]
fn bindgen_test_layout_template_Base() {
assert_eq!(::std::mem::size_of::<Base<Derived>>() , 1usize);
assert_eq!(::std::mem::align_of::<Base<Derived>>() , 1usize);
}
#[repr(C)]
#[derive(Debug)]
pub struct BaseWithDestructor<T> {
Expand All @@ -41,3 +46,10 @@ fn bindgen_test_layout_DerivedFromBaseWithDestructor() {
assert_eq!(::std::mem::align_of::<DerivedFromBaseWithDestructor>() ,
1usize);
}
#[test]
fn bindgen_test_layout_template_BaseWithDestructor() {
assert_eq!(::std::mem::size_of::<BaseWithDestructor<DerivedFromBaseWithDestructor>>()
, 1usize);
assert_eq!(::std::mem::align_of::<BaseWithDestructor<DerivedFromBaseWithDestructor>>()
, 1usize);
}
14 changes: 14 additions & 0 deletions libbindgen/tests/expectations/tests/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ pub struct Foo<T, U> {
pub m_member_arr: [T; 1usize],
pub _phantom_1: ::std::marker::PhantomData<U>,
}
#[test]
fn bindgen_test_layout_template_Foo_int_int() {
assert_eq!(::std::mem::size_of::<Foo<::std::os::raw::c_int, ::std::os::raw::c_int>>()
, 24usize);
assert_eq!(::std::mem::align_of::<Foo<::std::os::raw::c_int, ::std::os::raw::c_int>>()
, 8usize);
}
extern "C" {
#[link_name = "_Z3bar3FooIiiE"]
pub fn bar(foo: Foo<::std::os::raw::c_int, ::std::os::raw::c_int>);
Expand Down Expand Up @@ -168,3 +175,10 @@ pub struct TemplateWithVar<T> {
pub _address: u8,
pub _phantom_0: ::std::marker::PhantomData<T>,
}
#[test]
fn bindgen_test_layout_template_WithDtor_int() {
assert_eq!(::std::mem::size_of::<WithDtor<::std::os::raw::c_int>>() ,
4usize);
assert_eq!(::std::mem::align_of::<WithDtor<::std::os::raw::c_int>>() ,
4usize);
}