Skip to content

codegen: Add an option to skip comment generation. #444

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 2 commits into from
Jan 26, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ name = "bindgen"
readme = "README.md"
repository = "https://github.com/servo/rust-bindgen"
documentation = "https://docs.rs/bindgen"
version = "0.20.2"
version = "0.20.3"
build = "build.rs"

[badges]
Expand Down
30 changes: 20 additions & 10 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,10 @@ impl CodeGenerator for Type {
let rust_name = ctx.rust_ident(&name);
let mut typedef = aster::AstBuilder::new().item().pub_();

if let Some(comment) = item.comment() {
typedef = typedef.attr().doc(comment);
if ctx.options().generate_comments {
if let Some(comment) = item.comment() {
typedef = typedef.attr().doc(comment);
}
}

// We prefer using `pub use` over `pub type` because of:
Expand Down Expand Up @@ -808,8 +810,10 @@ impl CodeGenerator for CompInfo {

let mut attributes = vec![];
let mut needs_clone_impl = false;
if let Some(comment) = item.comment() {
attributes.push(attributes::doc(comment));
if ctx.options().generate_comments {
if let Some(comment) = item.comment() {
attributes.push(attributes::doc(comment));
}
}
if self.packed() {
attributes.push(attributes::repr_list(&["C", "packed"]));
Expand Down Expand Up @@ -1007,8 +1011,10 @@ impl CodeGenerator for CompInfo {
};

let mut attrs = vec![];
if let Some(comment) = field.comment() {
attrs.push(attributes::doc(comment));
if ctx.options().generate_comments {
Copy link
Member

Choose a reason for hiding this comment

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

This is a whole lot of copy paste... What if we made Item::comment and Field::comment take the context and check themselves? The earlier we push this check, we could even potentially avoid keeping the comments around in memory.

I would be fine with landing this code now and investigating this later, but if you do want to land the code now, please file an issue and maybe mark it as a good first bug to attract new contributors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, but I thought that the alternative would be less readable? Anyway, will file.

if let Some(comment) = field.comment() {
attrs.push(attributes::doc(comment));
}
}
let field_name = match field.name() {
Some(name) => ctx.rust_mangle(name).into_owned(),
Expand Down Expand Up @@ -1705,8 +1711,10 @@ impl CodeGenerator for Enum {
builder = builder.with_attr(attributes::repr("C"));
}

if let Some(comment) = item.comment() {
builder = builder.with_attr(attributes::doc(comment));
if ctx.options().generate_comments {
if let Some(comment) = item.comment() {
builder = builder.with_attr(attributes::doc(comment));
}
}

if !is_constified_enum {
Expand Down Expand Up @@ -2166,8 +2174,10 @@ impl CodeGenerator for Function {

let mut attributes = vec![];

if let Some(comment) = item.comment() {
attributes.push(attributes::doc(comment));
if ctx.options().generate_comments {
if let Some(comment) = item.comment() {
attributes.push(attributes::doc(comment));
}
}

if let Some(mangled) = mangled_name {
Expand Down
26 changes: 14 additions & 12 deletions src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1155,15 +1155,15 @@ pub struct WhitelistedItemsIter<'ctx, 'gen>
{
ctx: &'ctx BindgenContext<'gen>,

// The set of whitelisted items we have seen. If you think of traversing
// whitelisted items like GC tracing, this is the mark bits, and contains
// both black and gray items.
/// The set of whitelisted items we have seen. If you think of traversing
/// whitelisted items like GC tracing, this is the mark bits, and contains
/// both black and gray items.
seen: ItemSet,

// The set of whitelisted items that we have seen but have yet to iterate
// over and collect transitive references from. To return to the GC analogy,
// this is the mark stack, containing the set of gray items which we have
// not finished tracing yet.
/// The set of whitelisted items that we have seen but have yet to iterate
/// over and collect transitive references from. To return to the GC analogy,
/// this is the mark stack, containing the set of gray items which we have
/// not finished tracing yet.
to_iterate: Vec<ItemId>,
}

Expand All @@ -1181,12 +1181,14 @@ impl<'ctx, 'gen> Iterator for WhitelistedItemsIter<'ctx, 'gen>
debug_assert!(self.seen.contains(&id));
debug_assert!(self.ctx.items.contains_key(&id));

let mut sub_types = ItemSet::new();
id.collect_types(self.ctx, &mut sub_types, &());
if self.ctx.options().whitelist_recursively {
let mut sub_types = ItemSet::new();
id.collect_types(self.ctx, &mut sub_types, &());

for id in sub_types {
if self.seen.insert(id) {
self.to_iterate.push(id);
for id in sub_types {
if self.seen.insert(id) {
self.to_iterate.push(id);
}
}
}

Expand Down
33 changes: 33 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,29 @@ impl Builder {
self
}

/// Whether the generated bindings should contain documentation comments or
/// not.
///
/// This ideally will always be true, but it may need to be false until we
/// implement some processing on comments to work around issues as described
/// in:
///
/// https://github.com/servo/rust-bindgen/issues/426
pub fn generate_comments(mut self, doit: bool) -> Self {
self.options.generate_comments = doit;
self
}

/// Whether to whitelist types recursively or not. Defaults to true.
///
/// This can be used to get bindgen to generate _exactly_ the types you want
/// in your bindings, and then import other types manually via other means
/// (like `raw_line`).
pub fn whitelist_recursively(mut self, doit: bool) -> Self {
self.options.whitelist_recursively = doit;
self
}

/// Generate a C/C++ file that includes the header and has dummy uses of
/// every type defined in the header.
pub fn dummy_uses<T: Into<String>>(mut self, dummy_uses: T) -> Builder {
Expand Down Expand Up @@ -495,6 +518,7 @@ pub struct BindgenOptions {
/// The input header file.
pub input_header: Option<String>,


/// Generate a dummy C/C++ file that includes the header and has dummy uses
/// of all types defined therein. See the `uses` module for more.
pub dummy_uses: Option<String>,
Expand All @@ -511,6 +535,13 @@ pub struct BindgenOptions {
///
/// See the builder method description for more details.
pub conservative_inline_namespaces: bool,

/// Wether to keep documentation comments in the generated output. See the
/// documentation for more details.
pub generate_comments: bool,

/// Wether to whitelist types recursively. Defaults to true.
pub whitelist_recursively: bool,
}

impl BindgenOptions {
Expand Down Expand Up @@ -555,6 +586,8 @@ impl Default for BindgenOptions {
type_chooser: None,
codegen_config: CodegenConfig::all(),
conservative_inline_namespaces: false,
generate_comments: true,
whitelist_recursively: true,
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ pub fn builder_from_flags<I>(args: I)
Arg::with_name("no-derive-debug")
.long("no-derive-debug")
.help("Avoid deriving Debug on any type."),
Arg::with_name("no-doc-comments")
.long("no-doc-comments")
.help("Avoid including doc comments in the output, see: \
https://github.com/servo/rust-bindgen/issues/426"),
Arg::with_name("no-recursive-whitelist")
.long("no-recursive-whitelist")
.help("Avoid whitelisting types recursively"),
Arg::with_name("builtins")
.long("builtins")
.help("Output bindings for builtin definitions, e.g. \
Expand Down Expand Up @@ -271,6 +278,14 @@ pub fn builder_from_flags<I>(args: I)
builder = builder.no_convert_floats();
}

if matches.is_present("no-doc-comments") {
builder = builder.generate_comments(false);
}

if matches.is_present("no-recursive-whitelist") {
builder = builder.whitelist_recursively(false);
}

if let Some(opaque_types) = matches.values_of("opaque-type") {
for ty in opaque_types {
builder = builder.opaque_type(ty);
Expand Down
19 changes: 19 additions & 0 deletions tests/expectations/tests/no-comments.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]


#[repr(C)]
#[derive(Debug, Copy)]
pub struct Foo {
pub s: ::std::os::raw::c_int,
}
#[test]
fn bindgen_test_layout_Foo() {
assert_eq!(::std::mem::size_of::<Foo>() , 4usize);
assert_eq!(::std::mem::align_of::<Foo>() , 4usize);
}
impl Clone for Foo {
fn clone(&self) -> Self { *self }
}
20 changes: 20 additions & 0 deletions tests/expectations/tests/no-recursive-whitelisting.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]

pub enum Bar {}

#[repr(C)]
#[derive(Debug, Copy)]
pub struct Foo {
pub baz: *mut Bar,
}
#[test]
fn bindgen_test_layout_Foo() {
assert_eq!(::std::mem::size_of::<Foo>() , 8usize);
assert_eq!(::std::mem::align_of::<Foo>() , 8usize);
}
impl Clone for Foo {
fn clone(&self) -> Self { *self }
}
5 changes: 5 additions & 0 deletions tests/headers/no-comments.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// bindgen-flags: --no-doc-comments

struct Foo {
int s; /*!< Including this will prevent rustc for compiling it */
};
7 changes: 7 additions & 0 deletions tests/headers/no-recursive-whitelisting.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// bindgen-flags: --no-recursive-whitelist --whitelist-type "Foo" --raw-line "pub enum Bar {}"

struct Bar;

struct Foo {
struct Bar* baz;
};