Skip to content

Option to avoid generating layout tests #632

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

Merged
merged 2 commits into from
Apr 14, 2017
Merged

Option to avoid generating layout tests #632

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
451ba0d
Option to avoid generating layout tests
dimbleby Apr 14, 2017
b8dd5c9
code review
dimbleby Apr 14, 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
131 changes: 68 additions & 63 deletions src/codegen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,9 @@ impl CodeGenerator for TemplateInstantiation {
// Although uses of instantiations don't need code generation, and are
// just converted to rust types in fields, vars, etc, we take this
// opportunity to generate tests for their layout here.
if !ctx.options().layout_tests {
return
}

let layout = item.kind().expect_type().layout(ctx);

Expand Down Expand Up @@ -1498,76 +1501,78 @@ impl CodeGenerator for CompInfo {
.codegen(ctx, result, whitelisted_items, &());
}

if let Some(layout) = layout {
let fn_name = format!("bindgen_test_layout_{}", canonical_name);
let fn_name = ctx.rust_ident_raw(&fn_name);
let type_name = ctx.rust_ident_raw(&canonical_name);
let prefix = ctx.trait_prefix();
let size_of_expr = quote_expr!(ctx.ext_cx(),
::$prefix::mem::size_of::<$type_name>());
let align_of_expr = quote_expr!(ctx.ext_cx(),
::$prefix::mem::align_of::<$type_name>());
let size = layout.size;
let align = layout.align;

let check_struct_align = if align > mem::size_of::<*mut ()>() {
// FIXME when [RFC 1358](https://github.com/rust-lang/rust/issues/33626) ready
None
} else {
quote_item!(ctx.ext_cx(),
assert_eq!($align_of_expr,
$align,
concat!("Alignment of ", stringify!($type_name)));
)
};
if ctx.options().layout_tests {
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, we can't early return in this one because there is more work happening afterwards. It would be awesome if this layout test generation was factored into its own function, but I realize that is an extant problem that is a bit out of scope for this PR.

if let Some(layout) = layout {
let fn_name = format!("bindgen_test_layout_{}", canonical_name);
let fn_name = ctx.rust_ident_raw(&fn_name);
let type_name = ctx.rust_ident_raw(&canonical_name);
let prefix = ctx.trait_prefix();
let size_of_expr = quote_expr!(ctx.ext_cx(),
::$prefix::mem::size_of::<$type_name>());
let align_of_expr = quote_expr!(ctx.ext_cx(),
::$prefix::mem::align_of::<$type_name>());
let size = layout.size;
let align = layout.align;

let check_struct_align = if align > mem::size_of::<*mut ()>() {
// FIXME when [RFC 1358](https://github.com/rust-lang/rust/issues/33626) ready
None
} else {
quote_item!(ctx.ext_cx(),
assert_eq!($align_of_expr,
$align,
concat!("Alignment of ", stringify!($type_name)));
)
};

// FIXME when [issue #465](https://github.com/servo/rust-bindgen/issues/465) ready
let too_many_base_vtables = self.base_members()
.iter()
.filter(|base| {
ctx.resolve_type(base.ty).has_vtable(ctx)
})
.count() > 1;
// FIXME when [issue #465](https://github.com/servo/rust-bindgen/issues/465) ready
let too_many_base_vtables = self.base_members()
.iter()
.filter(|base| {
ctx.resolve_type(base.ty).has_vtable(ctx)
})
.count() > 1;

let should_skip_field_offset_checks = item.is_opaque(ctx) ||
too_many_base_vtables;
let should_skip_field_offset_checks = item.is_opaque(ctx) ||
too_many_base_vtables;

let check_field_offset = if should_skip_field_offset_checks {
None
} else {
let asserts = self.fields()
.iter()
.filter(|field| field.bitfield().is_none())
.flat_map(|field| {
field.name().and_then(|name| {
field.offset().and_then(|offset| {
let field_offset = offset / 8;
let field_name = ctx.rust_ident(name);

quote_item!(ctx.ext_cx(),
assert_eq!(unsafe { &(*(0 as *const $type_name)).$field_name as *const _ as usize },
$field_offset,
concat!("Alignment of field: ", stringify!($type_name), "::", stringify!($field_name)));
)
let check_field_offset = if should_skip_field_offset_checks {
None
} else {
let asserts = self.fields()
.iter()
.filter(|field| field.bitfield().is_none())
.flat_map(|field| {
field.name().and_then(|name| {
field.offset().and_then(|offset| {
let field_offset = offset / 8;
let field_name = ctx.rust_ident(name);

quote_item!(ctx.ext_cx(),
assert_eq!(unsafe { &(*(0 as *const $type_name)).$field_name as *const _ as usize },
$field_offset,
concat!("Alignment of field: ", stringify!($type_name), "::", stringify!($field_name)));
)
})
})
})
}).collect::<Vec<P<ast::Item>>>();
}).collect::<Vec<P<ast::Item>>>();

Some(asserts)
};
Some(asserts)
};

let item = quote_item!(ctx.ext_cx(),
#[test]
fn $fn_name() {
assert_eq!($size_of_expr,
$size,
concat!("Size of: ", stringify!($type_name)));
let item = quote_item!(ctx.ext_cx(),
#[test]
fn $fn_name() {
assert_eq!($size_of_expr,
$size,
concat!("Size of: ", stringify!($type_name)));

$check_struct_align
$check_field_offset
})
.unwrap();
result.push(item);
$check_struct_align
$check_field_offset
})
.unwrap();
result.push(item);
}
}

let mut method_names = Default::default();
Expand Down
66 changes: 40 additions & 26 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,29 +214,33 @@ impl Builder {
})
.count();

if self.options.derive_debug == false {
if !self.options.layout_tests {
output_vector.push("--no-layout-tests".into());
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for updating this method too!

}

if !self.options.derive_debug {
output_vector.push("--no-derive-debug".into());
}

if self.options.derive_default == false {
if !self.options.derive_default {
output_vector.push("--no-derive-default".into());
} else {
output_vector.push("--with-derive-default".into());
}

if self.options.generate_comments == false {
if !self.options.generate_comments {
output_vector.push("--no-doc-comments".into());
}

if self.options.whitelist_recursively == false {
if !self.options.whitelist_recursively {
output_vector.push("--no-recursive-whitelist".into());
}

if self.options.objc_extern_crate == true {
if self.options.objc_extern_crate {
output_vector.push("--objc-extern-crate".into());
}

if self.options.builtins == true {
if self.options.builtins {
output_vector.push("--builtins".into());
}

Expand All @@ -259,21 +263,21 @@ impl Builder {
output_vector.push(dummy.clone());
}

if self.options.emit_ast == true {
if self.options.emit_ast {
output_vector.push("--emit-clang-ast".into());
}

if self.options.emit_ir == true {
if self.options.emit_ir {
output_vector.push("--emit-ir".into());
}
if let Some(ref graph) = self.options.emit_ir_graphviz {
output_vector.push("--emit-ir-graphviz".into());
output_vector.push(graph.clone())
}
if self.options.enable_cxx_namespaces == true {
if self.options.enable_cxx_namespaces {
output_vector.push("--enable-cxx-namespaces".into());
}
if self.options.disable_name_namespacing == true {
if self.options.disable_name_namespacing {
output_vector.push("--disable-name-namespacing".into());
}

Expand All @@ -286,36 +290,36 @@ impl Builder {
})
.count();

if self.options.codegen_config.functions == false {
if !self.options.codegen_config.functions {
output_vector.push("--ignore-functions".into());
}

output_vector.push("--generate".into());

//Temporary placeholder for below 4 options
let mut options:Vec<String> = Vec::new();
if self.options.codegen_config.functions == true {
if self.options.codegen_config.functions {
options.push("function".into());
}
if self.options.codegen_config.types == true {
if self.options.codegen_config.types {
options.push("types".into());
}
if self.options.codegen_config.vars == true {
if self.options.codegen_config.vars {
options.push("vars".into());
}
if self.options.codegen_config.methods == true {
if self.options.codegen_config.methods {
options.push("methods".into());
}
if self.options.codegen_config.constructors == true {
if self.options.codegen_config.constructors {
options.push("constructors".into());
}
if self.options.codegen_config.destructors == true {
if self.options.codegen_config.destructors {
options.push("destructors".into());
}

output_vector.push(options.join(","));

if self.options.codegen_config.methods == false{
if !self.options.codegen_config.methods {
output_vector.push("--ignore-methods".into());
}

Expand All @@ -328,15 +332,15 @@ impl Builder {
})
.count();

if self.options.convert_floats == false {
if !self.options.convert_floats {
output_vector.push("--no-convert-floats".into());
}

if self.options.prepend_enum_name == false {
if !self.options.prepend_enum_name {
output_vector.push("--no-prepend-enum-name".into());
}

if self.options.unstable_rust == false {
if !self.options.unstable_rust {
output_vector.push("--no-unstable-rust".into());
}

Expand Down Expand Up @@ -368,11 +372,11 @@ impl Builder {
})
.count();

if self.options.use_core == true {
if self.options.use_core {
output_vector.push("--use-core".into());
}

if self.options.conservative_inline_namespaces == true {
if self.options.conservative_inline_namespaces {
output_vector.push("--conservative-inline-namespaces".into());
}

Expand Down Expand Up @@ -582,6 +586,12 @@ impl Builder {
self
}

/// Set whether layout tests should be generated.
pub fn layout_tests(mut self, doit: bool) -> Self {
self.options.layout_tests = doit;
self
}

/// Set whether `Debug` should be derived by default.
pub fn derive_debug(mut self, doit: bool) -> Self {
self.options.derive_debug = doit;
Expand Down Expand Up @@ -785,11 +795,14 @@ pub struct BindgenOptions {
/// True if we should avoid mangling names with namespaces.
pub disable_name_namespacing: bool,

/// True if we shold derive Debug trait implementations for C/C++ structures
/// True if we should generate layout tests for generated structures.
pub layout_tests: bool,

/// True if we should derive Debug trait implementations for C/C++ structures
/// and types.
pub derive_debug: bool,

/// True if we shold derive Default trait implementations for C/C++ structures
/// True if we should derive Default trait implementations for C/C++ structures
/// and types.
pub derive_default: bool,

Expand Down Expand Up @@ -901,6 +914,7 @@ impl Default for BindgenOptions {
emit_ast: false,
emit_ir: false,
emit_ir_graphviz: None,
layout_tests: true,
derive_debug: true,
derive_default: false,
enable_cxx_namespaces: false,
Expand Down Expand Up @@ -1237,4 +1251,4 @@ fn commandline_flag_unit_test_function() {

assert!(test_cases.iter().all(|ref x| command_line_flags.contains(x)) );

}
}
7 changes: 7 additions & 0 deletions src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ pub fn builder_from_flags<I>
.takes_value(true)
.multiple(true)
.number_of_values(1),
Arg::with_name("no-layout-tests")
.long("no-layout-tests")
.help("Avoid generating layout tests for any type."),
Arg::with_name("no-derive-debug")
.long("no-derive-debug")
.help("Avoid deriving Debug on any type."),
Expand Down Expand Up @@ -233,6 +236,10 @@ pub fn builder_from_flags<I>
builder = builder.emit_builtins();
}

if matches.is_present("no-layout-tests") {
builder = builder.layout_tests(false);
}

if matches.is_present("no-derive-debug") {
builder = builder.derive_debug(false);
}
Expand Down