From d0f07f98b59fe1d6256e5dd821d4dee90e450a48 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 18 Feb 2016 12:15:02 -0800 Subject: [PATCH 1/8] Spit errors and warnings to stdout. The correct thing to do here is to use env_logger, but that was causing cargo troubles for me, and this is preferable to swallowing them. --- src/bin/bindgen.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bin/bindgen.rs b/src/bin/bindgen.rs index 2e31983e1f..99ad96d65d 100644 --- a/src/bin/bindgen.rs +++ b/src/bin/bindgen.rs @@ -18,11 +18,11 @@ struct StdLogger; impl Logger for StdLogger { fn error(&self, msg: &str) { - error!("{}", msg); + println!("{}", msg); } fn warn(&self, msg: &str) { - warn!("{}", msg); + println!("{}", msg); } } From 6be9f9fbc97e7ce809edd1f3be5763b524044779 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 18 Feb 2016 14:45:45 -0800 Subject: [PATCH 2/8] Add the -ignore-functions argument. --- src/bin/bindgen.rs | 6 ++++++ src/lib.rs | 3 +++ src/parser.rs | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/src/bin/bindgen.rs b/src/bin/bindgen.rs index 99ad96d65d..4a90b3dc35 100644 --- a/src/bin/bindgen.rs +++ b/src/bin/bindgen.rs @@ -99,6 +99,10 @@ fn parse_args(args: &[String]) -> ParseResult { options.builtins = true; ix += 1; } + "-ignore-functions" => { + options.ignore_functions = true; + ix += 1; + } "-allow-unknown-types" => { options.fail_on_unknown_type = false; ix += 1; @@ -138,6 +142,8 @@ Options: matching any rule are bound to. -builtins Output bindings for builtin definitions (for example __builtin_va_list) + -ignore-functions Don't generate bindings for functions and methods. + This is useful when you only care about struct layouts. -allow-unknown-types Don't fail if we encounter types we do not support, instead treat them as void -emit-clang-ast Output the ast (for debugging purposes) diff --git a/src/lib.rs b/src/lib.rs index cd73860bf0..304b2f881c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -104,6 +104,7 @@ pub struct BindgenOptions { pub builtins: bool, pub links: Vec<(String, LinkType)>, pub emit_ast: bool, + pub ignore_functions: bool, pub fail_on_unknown_type: bool, pub override_enum_ty: String, pub clang_args: Vec, @@ -116,6 +117,7 @@ impl Default for BindgenOptions { builtins: false, links: Vec::new(), emit_ast: false, + ignore_functions: false, fail_on_unknown_type: true, override_enum_ty: "".to_string(), clang_args: Vec::new() @@ -224,6 +226,7 @@ fn parse_headers(options: &BindgenOptions, logger: &Logger) -> Result, pub emit_ast: bool, pub fail_on_unknown_type: bool, + pub ignore_functions: bool, pub override_enum_ty: Option, pub clang_args: Vec, } @@ -590,6 +591,10 @@ fn visit_composite(cursor: &Cursor, parent: &Cursor, ci.base_members += 1; } CXCursor_CXXMethod => { + if ctx.options.ignore_functions { + return CXChildVisit_Continue; + } + let linkage = cursor.linkage(); if linkage != CXLinkage_External { return CXChildVisit_Continue; From 589c26e421de6284213f57fe3821035a2f810034 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 18 Feb 2016 16:41:32 -0800 Subject: [PATCH 3/8] Handle 1 << 63 as enum value. --- src/gen.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/gen.rs b/src/gen.rs index ae56e3af97..5286152e1f 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -1132,10 +1132,18 @@ fn cenum_to_rs(ctx: &mut GenCtx, name: String, comment: String, items: Vec Date: Wed, 2 Mar 2016 20:06:52 -0800 Subject: [PATCH 4/8] Don't try to convert standard int types in rust_type_id. It looks like mwu added this, but I'm pretty sure it's a category error. This function appears to be designed to reproducibly permute C identifiers so that they don't conflict with builtin rust types. It's specifically _not_ a type translator (which would happen at the type level, rather than the string level), and using it as such with callers like ctypedef_to_rs causes us to generate things like: type u64 = u64; While processing stdint.h, which is clearly wrong. --- src/gen.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/gen.rs b/src/gen.rs index 5286152e1f..3dbeccb854 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -76,20 +76,8 @@ fn rust_type_id(ctx: &mut GenCtx, name: String) -> String { s.push_str(&name); s } else { - match name.as_str() { - "int8_t" => "i8".to_string(), - "uint8_t" => "u8".to_string(), - "int16_t" => "i16".to_string(), - "uint16_t" => "u16".to_string(), - "int32_t" => "i32".to_string(), - "uint32_t" => "u32".to_string(), - "int64_t" => "i64".to_string(), - "uint64_t" => "u64".to_string(), - _ => { - let (n, _) = rust_id(ctx, name); - n - } - } + let (n, _) = rust_id(ctx, name); + n } } From 949151af46499cfd4eca076fb75877052611fc04 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 3 Mar 2016 16:48:11 -0800 Subject: [PATCH 5/8] Stop patching in placeholder names for CompInfo and EnumInfo instances during code generator. As best as I can tell, it's done this way (rather than my way) because bindgen tries to recognize struct and enums typedefs of the form: /* This is a common idiom in C, not so much in C++ */ typdef struct { ... } Foo; The intention, I think, is to avoid generating rust code for a struct with a placeholder name followed by a typedef, and just give the struct the right name initially. This seems like a reasonable goal, though not a particularly important one. However, in my testing this never actually happens, because we end up calling unnamed_name anyway during the GComp(ci) case of gen_mod before we get to evaluting the typedef. So let's just remove that stuff and simplify the code. This lets us remove all the borrow_mut calls during code generation, which seems necessary for soundness. --- src/gen.rs | 53 ++++++---------------------------------------------- src/types.rs | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 49 deletions(-) diff --git a/src/gen.rs b/src/gen.rs index 3dbeccb854..f46b372054 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -24,7 +24,6 @@ use types::*; struct GenCtx<'r> { ext_cx: base::ExtCtxt<'r>, - unnamed_ty: usize, span: Span } @@ -81,15 +80,6 @@ fn rust_type_id(ctx: &mut GenCtx, name: String) -> String { } } -fn unnamed_name(ctx: &mut GenCtx, name: String, filename: String) -> String { - return if name.is_empty() { - ctx.unnamed_ty += 1; - format!("{}_unnamed_{}", filename, ctx.unnamed_ty) - } else { - name - }; -} - fn comp_name(kind: CompKind, name: &String) -> String { match kind { CompKind::Struct => struct_name(name), @@ -250,7 +240,6 @@ pub fn gen_mod(links: &[(String, LinkType)], globs: Vec, span: Span) -> let mut feature_gated_cfgs = vec![]; let mut ctx = GenCtx { ext_cx: base::ExtCtxt::new(sess, Vec::new(), cfg, &mut feature_gated_cfgs), - unnamed_ty: 0, span: span }; ctx.ext_cx.bt_push(ExpnInfo { @@ -300,35 +289,19 @@ pub fn gen_mod(links: &[(String, LinkType)], globs: Vec, span: Span) -> defs.extend(ctypedef_to_rs(&mut ctx, t.name.clone(), t.comment.clone(), &t.ty).into_iter()) }, GCompDecl(ci) => { - { - let mut c = ci.borrow_mut(); - c.name = unnamed_name(&mut ctx, c.name.clone(), c.filename.clone()); - } let c = ci.borrow().clone(); defs.push(opaque_to_rs(&mut ctx, comp_name(c.kind, &c.name))); }, GComp(ci) => { - { - let mut c = ci.borrow_mut(); - c.name = unnamed_name(&mut ctx, c.name.clone(), c.filename.clone()); - } let c = ci.borrow().clone(); defs.extend(comp_to_rs(&mut ctx, comp_name(c.kind, &c.name), c).into_iter()) }, GEnumDecl(ei) => { - { - let mut e = ei.borrow_mut(); - e.name = unnamed_name(&mut ctx, e.name.clone(), e.filename.clone()); - } let e = ei.borrow().clone(); defs.push(opaque_to_rs(&mut ctx, enum_name(&e.name))); }, GEnum(ei) => { - { - let mut e = ei.borrow_mut(); - e.name = unnamed_name(&mut ctx, e.name.clone(), e.filename.clone()); - } let e = ei.borrow().clone(); defs.extend(cenum_to_rs(&mut ctx, enum_name(&e.name), e.comment, e.items, e.layout).into_iter()) }, @@ -659,24 +632,12 @@ fn ctypedef_to_rs(ctx: &mut GenCtx, name: String, comment: String, ty: &Type) -> return match *ty { TComp(ref ci) => { - let is_empty = ci.borrow().name.is_empty(); - if is_empty { - ci.borrow_mut().name = name.clone(); - let c = ci.borrow().clone(); - comp_to_rs(ctx, name, c) - } else { - vec!(mk_item(ctx, name, comment, ty)) - } + assert!(!ci.borrow().name.is_empty()); + vec!(mk_item(ctx, name, comment, ty)) }, TEnum(ref ei) => { - let is_empty = ei.borrow().name.is_empty(); - if is_empty { - ei.borrow_mut().name = name.clone(); - let e = ei.borrow().clone(); - cenum_to_rs(ctx, name, e.comment, e.items, e.layout) - } else { - vec!(mk_item(ctx, name, comment, ty)) - } + assert!(!ei.borrow().name.is_empty()); + vec!(mk_item(ctx, name, comment, ty)) }, _ => vec!(mk_item(ctx, name, comment, ty)) } @@ -1614,16 +1575,14 @@ fn cty_to_rs(ctx: &mut GenCtx, ty: &Type, allow_bool: bool) -> ast::Ty { mk_ty(ctx, false, vec!(id)) }, &TComp(ref ci) => { - let mut c = ci.borrow_mut(); - c.name = unnamed_name(ctx, c.name.clone(), c.filename.clone()); + let mut c = ci.borrow(); let args = c.args.iter().map(|gt| { P(cty_to_rs(ctx, gt, allow_bool)) }).collect(); mk_ty_args(ctx, false, vec!(comp_name(c.kind, &c.name)), args) }, &TEnum(ref ei) => { - let mut e = ei.borrow_mut(); - e.name = unnamed_name(ctx, e.name.clone(), e.filename.clone()); + let mut e = ei.borrow(); mk_ty(ctx, false, vec!(enum_name(&e.name))) } }; diff --git a/src/types.rs b/src/types.rs index 64adeeb32c..7f389462e2 100644 --- a/src/types.rs +++ b/src/types.rs @@ -194,11 +194,22 @@ pub struct CompInfo { pub layout: Layout, } +static mut UNNAMED_COUNTER: u32 = 0; + +fn unnamed_name(name: String, filename: &String) -> String { + return if name.is_empty() { + let n = unsafe { UNNAMED_COUNTER += 1; UNNAMED_COUNTER }; + format!("{}_unnamed_{}", filename, n) + } else { + name + }; +} + impl CompInfo { pub fn new(name: String, filename: String, comment: String, kind: CompKind, members: Vec, layout: Layout) -> CompInfo { CompInfo { kind: kind, - name: name, + name: unnamed_name(name, &filename), filename: filename, comment: comment, members: members, @@ -253,7 +264,7 @@ pub struct EnumInfo { impl EnumInfo { pub fn new(name: String, filename: String, kind: IKind, items: Vec, layout: Layout) -> EnumInfo { EnumInfo { - name: name, + name: unnamed_name(name, &filename), comment: String::new(), filename: filename, items: items, From 283d0376bcca61d1313e9104a6e28014459c128c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 5 Mar 2016 01:00:58 +0100 Subject: [PATCH 6/8] gen: Allow empty union members --- src/gen.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/gen.rs b/src/gen.rs index f46b372054..b7fddc9f87 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -1246,17 +1246,25 @@ fn gen_bitfield_method(ctx: &mut GenCtx, bindgen_name: &String, } fn gen_fullbitfield_method(ctx: &mut GenCtx, bindgen_name: &String, - field_type: &Type, bitfields: &Vec<(String, u32)>) -> ast::ImplItem { + field_type: &Type, bitfields: &[(String, u32)]) -> ast::ImplItem { let field_type = cty_to_rs(ctx, field_type, false); let mut args = vec!(); + let mut unnamed: usize = 0; for &(ref name, width) in bitfields.iter() { + let ident = if name.is_empty() { + unnamed += 1; + let dummy = format!("unnamed_bitfield{}", unnamed); + ctx.ext_cx.ident_of(&dummy) + } else { + ctx.ext_cx.ident_of(name) + }; args.push(ast::Arg { ty: P(type_for_bitfield_width(ctx, width)), pat: P(ast::Pat { id: ast::DUMMY_NODE_ID, node: ast::PatIdent( ast::BindingMode::ByValue(ast::MutImmutable), - respan(ctx.span, ctx.ext_cx.ident_of(name)), + respan(ctx.span, ident), None ), span: ctx.span @@ -1275,8 +1283,15 @@ fn gen_fullbitfield_method(ctx: &mut GenCtx, bindgen_name: &String, let mut offset = 0; let mut exprs = quote_expr!(&ctx.ext_cx, 0); + let mut unnamed: usize = 0; for &(ref name, width) in bitfields.iter() { - let name_ident = ctx.ext_cx.ident_of(&name); + let name_ident = if name.is_empty() { + unnamed += 1; + let dummy = format!("unnamed_bitfield{}", unnamed); + ctx.ext_cx.ident_of(&dummy) + } else { + ctx.ext_cx.ident_of(name) + }; exprs = quote_expr!(&ctx.ext_cx, $exprs | (($name_ident as $field_type) << $offset) ); From 62ad73e6d74c59957f98d3880c814532f44f5d62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 7 Mar 2016 21:10:26 +0100 Subject: [PATCH 7/8] Use full paths in generation. --- src/gen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gen.rs b/src/gen.rs index b7fddc9f87..310db2bddd 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -759,7 +759,7 @@ fn cstruct_to_rs(ctx: &mut GenCtx, name: String, ci: CompInfo) -> Vec), + ty: quote_ty!(&ctx.ext_cx, ::std::marker::PhantomData<$inner_type>), attrs: vec!(), })); } From 6d372e092b1bdbc7af7e536093431af784463e67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 7 Mar 2016 21:10:53 +0100 Subject: [PATCH 8/8] Fix test compilation --- tests/support.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/support.rs b/tests/support.rs index a83bbc0230..f2a5abb14a 100644 --- a/tests/support.rs +++ b/tests/support.rs @@ -38,7 +38,7 @@ pub fn assert_bind_eq(filename: &str, reference_items_str: &str) let mut parser = parse::new_parser_from_source_str(ext_cx.parse_sess(), ext_cx.cfg(), "".to_string(), reference_items_str.to_string()); let mut reference_items = Vec::new(); - while let Some(item) = parser.parse_item() { + while let Ok(Some(item)) = parser.parse_item() { reference_items.push(item); } @@ -47,7 +47,7 @@ pub fn assert_bind_eq(filename: &str, reference_items_str: &str) // rendered versions, which is not beautiful, but should work. let reference_rendered = render_items(&reference_items); let generated_rendered = render_items(&generated_items); - + if reference_rendered != generated_rendered { println!("Generated bindings for {} do not match the reference bindings.", filename); println!("");