Auto merge of #110 - heycam:stable-gen-names, r=emilio

Give vtables and anonymous items more stable generated names (fixes #60)

r? @emilio

This works pretty well.  There are two remaining things in stylo's structs files that have identifiers that look like they won't be that stable: the anonymous enum for the NODE_* flags at the top level, and the `typedef union { ... } nsStyleUnion`.  There are various anonymous enums and other things at the top level in system headers that cause these identifiers to have generated IDs in them higher than 1 and 2.

Probably for anonymous enums we could just avoid generating a rust enum altogether, since having the static consts should be sufficient.  I tried to mess with the codegen to automatically treat `typedef union { ... } nsStyleUnion` like `union nsStyleUnion { ... }` but it seems the way clang exposes the typedef and union are as two adjacent cursors rather than a parent-child relationship, so it's not so easy.

Author: bors-servo

Cargo.toml

@@ -23,6 +23,7 @@ quasi_codegen = "0.20"
 
 [dependencies]
 clang-sys = "0.8.0"
+lazy_static = "0.1.*"
 libc = "0.2"
 log = "0.3"
 env_logger = "0.3"

src/codegen/mod.rs

@@ -445,8 +445,8 @@ impl<'a> CodeGenerator for Vtable<'a> {
 }
 
 impl<'a> ItemCanonicalName for Vtable<'a> {
-    fn canonical_name(&self, _ctx: &BindgenContext) -> String {
-        format!("bindgen_vtable_{}", self.item_id)
+    fn canonical_name(&self, ctx: &BindgenContext) -> String {
+        format!("{}__bindgen_vtable", self.item_id.canonical_name(ctx))
     }
 }
 

src/ir/item.rs

@@ -1,10 +1,11 @@
+use regex::Regex;
 use super::context::BindgenContext;
 use super::item_kind::ItemKind;
 use super::ty::{Type, TypeKind};
 use super::function::Function;
 use super::module::Module;
 use super::annotations::Annotations;
-use std::fmt;
+use std::cell::Cell;
 use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};
 use parse::{ClangItemParser, ClangSubItemParser, ParseError, ParseResult};
 use clang;
@@ -46,13 +47,6 @@ pub trait ItemCanonicalPath {
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct ItemId(usize);
 
-impl fmt::Display for ItemId {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        try!(write!(fmt, "_bindgen_id_"));
-        self.0.fmt(fmt)
-    }
-}
-
 pub static NEXT_ITEM_ID: AtomicUsize = ATOMIC_USIZE_INIT;
 
 impl ItemId {
@@ -96,6 +90,11 @@ impl ItemCanonicalPath for ItemId {
 pub struct Item {
     /// This item's id.
     id: ItemId,
+    /// The item's local id, unique only amongst its siblings.  Only used
+    /// for anonymous items.  Lazily initialized in local_id().
+    local_id: Cell<Option<usize>>,
+    /// The next local id to use for a child.
+    next_child_local_id: Cell<usize>,
     /// A doc comment over the item, if any.
     comment: Option<String>,
     /// Annotations extracted from the doc comment, or the default ones
@@ -120,6 +119,8 @@ impl Item {
         debug_assert!(id != parent_id || kind.is_module());
         Item {
             id: id,
+            local_id: Cell::new(None),
+            next_child_local_id: Cell::new(1),
             parent_id: parent_id,
             comment: comment,
             annotations: annotations.unwrap_or_default(),
@@ -147,6 +148,16 @@ impl Item {
         &mut self.kind
     }
 
+    pub fn local_id(&self, ctx: &BindgenContext) -> usize {
+        if self.local_id.get().is_none() {
+            let parent = ctx.resolve_item(self.parent_id);
+            let local_id = parent.next_child_local_id.get();
+            parent.next_child_local_id.set(local_id + 1);
+            self.local_id.set(Some(local_id));
+        }
+        self.local_id.get().unwrap()
+    }
+
     /// Returns whether this item is a top-level item, from the point of view of
     /// bindgen.
     ///
@@ -435,7 +446,6 @@ impl Item {
                         ty.name()
                     }
                 }.map(ToOwned::to_owned)
-                 .unwrap_or_else(|| format!("_bindgen_ty{}", self.id()))
             }
             ItemKind::Function(ref fun) => {
                 let mut base = fun.name().to_owned();
@@ -464,30 +474,68 @@ impl Item {
                         }
                     }
                 }
-                base
+                Some(base)
             }
             ItemKind::Var(ref var) => {
-                var.name().to_owned()
+                Some(var.name().to_owned())
             }
             ItemKind::Module(ref module) => {
                 module.name().map(ToOwned::to_owned)
-                    .unwrap_or_else(|| format!("_bindgen_mod{}", self.id()))
             }
         };
 
         let parent = ctx.resolve_item(self.parent_id());
         let parent_is_namespace = parent.is_module();
+
         if self.is_toplevel(ctx) || (parent_is_namespace && count_namespaces) {
+            let base_name = self.make_exposed_name(None, base_name, ctx);
             return ctx.rust_mangle(&base_name).into_owned();
         }
 
         // TODO: allow modification of the mangling functions, maybe even per
         // item type?
-        let parent = parent.canonical_name(ctx);
-        if parent.is_empty() {
-            base_name.to_owned()
-        } else {
-            format!("{}_{}", parent, base_name)
+        let parent_name = parent.canonical_name(ctx);
+        self.make_exposed_name(Some(parent_name), base_name, ctx)
+    }
+
+    fn exposed_id(&self, ctx: &BindgenContext) -> String {
+        // Only use local ids for enums, classes, structs and union types.  All
+        // other items use their global id.
+        let ty_kind = self.kind().as_type().map(|t| t.kind());
+        if let Some(ty_kind) = ty_kind {
+            match *ty_kind {
+                TypeKind::Comp(..) |
+                TypeKind::Enum(..) => return self.local_id(ctx).to_string(),
+                _ => {}
+            }
+        }
+        format!("id_{}", self.id().0)
+    }
+
+    fn make_exposed_name(&self,
+                         parent_name: Option<String>,
+                         base_name: Option<String>,
+                         ctx: &BindgenContext) -> String {
+        lazy_static! {
+            static ref RE_ENDS_WITH_BINDGEN_TY: Regex = Regex::new(r"_bindgen_ty(_\d+)+$").unwrap();
+            static ref RE_ENDS_WITH_BINDGEN_MOD: Regex = Regex::new(r"_bindgen_mod(_\d+)+$").unwrap();
+        }
+        let (re, kind) = match *self.kind() {
+            ItemKind::Module(..) => (&*RE_ENDS_WITH_BINDGEN_MOD, "mod"),
+            _ => (&*RE_ENDS_WITH_BINDGEN_TY, "ty"),
+        };
+        let parent_name = parent_name.and_then(|n| if n.is_empty() { None } else { Some(n) });
+        match (parent_name, base_name) {
+            (Some(parent), Some(base)) => format!("{}_{}", parent, base),
+            (Some(parent), None) => {
+                if re.is_match(parent.as_str()) {
+                    format!("{}_{}", parent, self.exposed_id(ctx))
+                } else {
+                    format!("{}__bindgen_{}_{}", parent, kind, self.exposed_id(ctx))
+                }
+            }
+            (None, Some(base)) => base,
+            (None, None) => format!("_bindgen_{}_{}", kind, self.exposed_id(ctx)),
         }
     }
 

src/lib.rs

@@ -16,6 +16,8 @@ extern crate libc;
 extern crate regex;
 #[macro_use]
 extern crate log;
+#[macro_use]
+extern crate lazy_static;
 
 mod clangll;
 mod clang;

tests/expectations/anon_enum.rs

@@ -10,11 +10,10 @@ pub struct Test {
     pub foo: ::std::os::raw::c_int,
     pub bar: f32,
 }
-pub const Test_T_NONE: Test__bindgen_ty_bindgen_id_6 =
-    Test__bindgen_ty_bindgen_id_6::T_NONE;
+pub const Test_T_NONE: Test__bindgen_ty_1 = Test__bindgen_ty_1::T_NONE;
 #[repr(u32)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub enum Test__bindgen_ty_bindgen_id_6 { T_NONE = 0, }
+pub enum Test__bindgen_ty_1 { T_NONE = 0, }
 #[test]
 fn bindgen_test_layout_Test() {
     assert_eq!(::std::mem::size_of::<Test>() , 8usize);

tests/expectations/anon_enum_whitelist.rs

@@ -4,10 +4,8 @@
 #![allow(non_snake_case)]
 
 
-pub const NODE_FLAG_FOO: _bindgen_ty_bindgen_id_1 =
-    _bindgen_ty_bindgen_id_1::NODE_FLAG_FOO;
-pub const NODE_FLAG_BAR: _bindgen_ty_bindgen_id_1 =
-    _bindgen_ty_bindgen_id_1::NODE_FLAG_BAR;
+pub const NODE_FLAG_FOO: _bindgen_ty_1 = _bindgen_ty_1::NODE_FLAG_FOO;
+pub const NODE_FLAG_BAR: _bindgen_ty_1 = _bindgen_ty_1::NODE_FLAG_BAR;
 #[repr(u32)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub enum _bindgen_ty_bindgen_id_1 { NODE_FLAG_FOO = 0, NODE_FLAG_BAR = 1, }
+pub enum _bindgen_ty_1 { NODE_FLAG_FOO = 0, NODE_FLAG_BAR = 1, }

tests/expectations/anon_union.rs

@@ -28,7 +28,7 @@ impl <T> ::std::marker::Copy for __BindgenUnionField<T> { }
 #[derive(Debug, Copy, Clone)]
 pub struct TErrorResult<T> {
     pub mResult: ::std::os::raw::c_int,
-    pub __bindgen_anon_1: TErrorResult__bindgen_ty_bindgen_id_10<T>,
+    pub __bindgen_anon_1: TErrorResult__bindgen_ty_1<T>,
     pub mMightHaveUnreported: bool,
     pub mUnionState: TErrorResult_UnionState,
     pub _phantom_0: ::std::marker::PhantomData<T>,
@@ -52,7 +52,7 @@ pub struct TErrorResult_DOMExceptionInfo<T> {
 }
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
-pub struct TErrorResult__bindgen_ty_bindgen_id_10<T> {
+pub struct TErrorResult__bindgen_ty_1<T> {
     pub mMessage: __BindgenUnionField<*mut TErrorResult_Message<T>>,
     pub mDOMExceptionInfo: __BindgenUnionField<*mut TErrorResult_DOMExceptionInfo<T>>,
     pub bindgen_union_field: u64,

tests/expectations/class_with_inner_struct.rs

@@ -28,8 +28,8 @@ impl <T> ::std::marker::Copy for __BindgenUnionField<T> { }
 #[derive(Debug, Copy)]
 pub struct A {
     pub c: ::std::os::raw::c_uint,
-    pub named_union: A__bindgen_ty_bindgen_id_9,
-    pub __bindgen_anon_1: A__bindgen_ty_bindgen_id_14,
+    pub named_union: A__bindgen_ty_1,
+    pub __bindgen_anon_1: A__bindgen_ty_2,
 }
 #[repr(C)]
 #[derive(Debug, Copy)]
@@ -47,31 +47,30 @@ impl Clone for A_Segment {
 }
 #[repr(C)]
 #[derive(Debug, Copy)]
-pub struct A__bindgen_ty_bindgen_id_9 {
+pub struct A__bindgen_ty_1 {
     pub f: __BindgenUnionField<::std::os::raw::c_int>,
     pub bindgen_union_field: u32,
 }
 #[test]
-fn bindgen_test_layout_A__bindgen_ty_bindgen_id_9() {
-    assert_eq!(::std::mem::size_of::<A__bindgen_ty_bindgen_id_9>() , 4usize);
-    assert_eq!(::std::mem::align_of::<A__bindgen_ty_bindgen_id_9>() , 4usize);
+fn bindgen_test_layout_A__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<A__bindgen_ty_1>() , 4usize);
+    assert_eq!(::std::mem::align_of::<A__bindgen_ty_1>() , 4usize);
 }
-impl Clone for A__bindgen_ty_bindgen_id_9 {
+impl Clone for A__bindgen_ty_1 {
     fn clone(&self) -> Self { *self }
 }
 #[repr(C)]
 #[derive(Debug, Copy)]
-pub struct A__bindgen_ty_bindgen_id_14 {
+pub struct A__bindgen_ty_2 {
     pub d: __BindgenUnionField<::std::os::raw::c_int>,
     pub bindgen_union_field: u32,
 }
 #[test]
-fn bindgen_test_layout_A__bindgen_ty_bindgen_id_14() {
-    assert_eq!(::std::mem::size_of::<A__bindgen_ty_bindgen_id_14>() , 4usize);
-    assert_eq!(::std::mem::align_of::<A__bindgen_ty_bindgen_id_14>() ,
-               4usize);
+fn bindgen_test_layout_A__bindgen_ty_2() {
+    assert_eq!(::std::mem::size_of::<A__bindgen_ty_2>() , 4usize);
+    assert_eq!(::std::mem::align_of::<A__bindgen_ty_2>() , 4usize);
 }
-impl Clone for A__bindgen_ty_bindgen_id_14 {
+impl Clone for A__bindgen_ty_2 {
     fn clone(&self) -> Self { *self }
 }
 #[test]
@@ -121,57 +120,51 @@ pub enum StepSyntax {
 #[derive(Debug, Copy)]
 pub struct C {
     pub d: ::std::os::raw::c_uint,
-    pub __bindgen_anon_1: C__bindgen_ty_bindgen_id_31,
+    pub __bindgen_anon_1: C__bindgen_ty_1,
 }
 #[repr(C)]
 #[derive(Debug, Copy)]
-pub struct C__bindgen_ty_bindgen_id_31 {
-    pub mFunc: __BindgenUnionField<C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_32>,
-    pub __bindgen_anon_1: __BindgenUnionField<C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_43>,
+pub struct C__bindgen_ty_1 {
+    pub mFunc: __BindgenUnionField<C__bindgen_ty_1_1>,
+    pub __bindgen_anon_1: __BindgenUnionField<C__bindgen_ty_1_2>,
     pub bindgen_union_field: [u32; 4usize],
 }
 #[repr(C)]
 #[derive(Debug, Copy)]
-pub struct C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_32 {
+pub struct C__bindgen_ty_1_1 {
     pub mX1: f32,
     pub mY1: f32,
     pub mX2: f32,
     pub mY2: f32,
 }
 #[test]
-fn bindgen_test_layout_C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_32() {
-    assert_eq!(::std::mem::size_of::<C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_32>()
-               , 16usize);
-    assert_eq!(::std::mem::align_of::<C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_32>()
-               , 4usize);
+fn bindgen_test_layout_C__bindgen_ty_1_1() {
+    assert_eq!(::std::mem::size_of::<C__bindgen_ty_1_1>() , 16usize);
+    assert_eq!(::std::mem::align_of::<C__bindgen_ty_1_1>() , 4usize);
 }
-impl Clone for C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_32 {
+impl Clone for C__bindgen_ty_1_1 {
     fn clone(&self) -> Self { *self }
 }
 #[repr(C)]
 #[derive(Debug, Copy)]
-pub struct C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_43 {
+pub struct C__bindgen_ty_1_2 {
     pub mStepSyntax: StepSyntax,
     pub mSteps: ::std::os::raw::c_uint,
 }
 #[test]
-fn bindgen_test_layout_C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_43() {
-    assert_eq!(::std::mem::size_of::<C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_43>()
-               , 8usize);
-    assert_eq!(::std::mem::align_of::<C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_43>()
-               , 4usize);
+fn bindgen_test_layout_C__bindgen_ty_1_2() {
+    assert_eq!(::std::mem::size_of::<C__bindgen_ty_1_2>() , 8usize);
+    assert_eq!(::std::mem::align_of::<C__bindgen_ty_1_2>() , 4usize);
 }
-impl Clone for C__bindgen_ty_bindgen_id_31__bindgen_ty_bindgen_id_43 {
+impl Clone for C__bindgen_ty_1_2 {
     fn clone(&self) -> Self { *self }
 }
 #[test]
-fn bindgen_test_layout_C__bindgen_ty_bindgen_id_31() {
-    assert_eq!(::std::mem::size_of::<C__bindgen_ty_bindgen_id_31>() ,
-               16usize);
-    assert_eq!(::std::mem::align_of::<C__bindgen_ty_bindgen_id_31>() ,
-               4usize);
+fn bindgen_test_layout_C__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<C__bindgen_ty_1>() , 16usize);
+    assert_eq!(::std::mem::align_of::<C__bindgen_ty_1>() , 4usize);
 }
-impl Clone for C__bindgen_ty_bindgen_id_31 {
+impl Clone for C__bindgen_ty_1 {
     fn clone(&self) -> Self { *self }
 }
 #[repr(C)]

tests/expectations/const_enum_unnamed.rs

@@ -4,23 +4,20 @@
 #![allow(non_snake_case)]
 
 
-pub const FOO_BAR: _bindgen_ty_bindgen_id_1 =
-    _bindgen_ty_bindgen_id_1::FOO_BAR;
-pub const FOO_BAZ: _bindgen_ty_bindgen_id_1 =
-    _bindgen_ty_bindgen_id_1::FOO_BAZ;
+pub const FOO_BAR: _bindgen_ty_1 = _bindgen_ty_1::FOO_BAR;
+pub const FOO_BAZ: _bindgen_ty_1 = _bindgen_ty_1::FOO_BAZ;
 #[repr(u32)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub enum _bindgen_ty_bindgen_id_1 { FOO_BAR = 0, FOO_BAZ = 1, }
+pub enum _bindgen_ty_1 { FOO_BAR = 0, FOO_BAZ = 1, }
 #[repr(C)]
 #[derive(Debug, Copy)]
 pub struct Foo {
     pub _address: u8,
 }
-pub const Foo_FOO_BAR: Foo__bindgen_ty_bindgen_id_5 =
-    Foo__bindgen_ty_bindgen_id_5::FOO_BAR;
+pub const Foo_FOO_BAR: Foo__bindgen_ty_1 = Foo__bindgen_ty_1::FOO_BAR;
 #[repr(u32)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub enum Foo__bindgen_ty_bindgen_id_5 { FOO_BAR = 10, }
+pub enum Foo__bindgen_ty_1 { FOO_BAR = 10, }
 #[test]
 fn bindgen_test_layout_Foo() {
     assert_eq!(::std::mem::size_of::<Foo>() , 1usize);