Auto merge of #906 - emilio:stylo-build-woes, r=fitzgen,Manishearth

ir: Sanitize base names more aggressively

This fixes stylo build failures when template instantiations tests caused a rust
compile error due to a bad test name.

This commit makes names better sanitized, preventing similar errors in the
future.

Author: bors-servo

src/ir/context.rs

@@ -1452,13 +1452,7 @@ impl<'ctx> BindgenContext<'ctx> {
             _ => return None,
         };
 
-        let mut spelling = ty.spelling();
-        // avoid the allocation if possible
-        if spelling.contains(' ') {
-            // These names are used in generated test names,
-            // they should be valid identifiers
-            spelling = spelling.replace(' ', "_");
-        }
+        let spelling = ty.spelling();
         let is_const = ty.is_const();
         let layout = ty.fallible_layout().ok();
         let ty = Type::new(Some(spelling), layout, type_kind, is_const);

src/ir/item.rs

@@ -695,18 +695,32 @@ impl Item {
             }
             ItemKind::Type(ref ty) => {
                 let name = match *ty.kind() {
-                    TypeKind::ResolvedTypeRef(..) => panic!("should have resolved this in name_target()"),
+                    TypeKind::ResolvedTypeRef(..) => {
+                        panic!("should have resolved this in name_target()")
+                    }
                     TypeKind::Pointer(inner) => {
                         ctx.resolve_item(inner)
                            .expect_type().name()
-                           .map(|name| format!("ptr_{}", name))
+                           .map(|name| {
+                               format!("ptr_{}", Type::sanitize_name(name))
+                           })
                     }
                     TypeKind::Array(inner, length) => {
                         ctx.resolve_item(inner)
                            .expect_type().name()
-                           .map(|name| format!("array_{}_{}", name, length))
+                           .map(|name| {
+                               format!(
+                                   "array_{}_{}",
+                                   Type::sanitize_name(name),
+                                   length,
+                                )
+                            })
+                    }
+                    _ => {
+                        ty.name()
+                          .map(Type::sanitize_name)
+                          .map(Into::into)
                     }
-                    _ => ty.name().map(ToOwned::to_owned)
                 };
                 name.unwrap_or_else(|| {
                     format!("_bindgen_ty_{}", self.exposed_id(ctx))

src/ir/ty.rs

@@ -13,6 +13,7 @@ use super::template::{AsTemplateParam, TemplateInstantiation, TemplateParameters
 use super::traversal::{EdgeKind, Trace, Tracer};
 use clang::{self, Cursor};
 use parse::{ClangItemParser, ParseError, ParseResult};
+use std::borrow::Cow;
 use std::io;
 use std::mem;
 
@@ -272,6 +273,18 @@ impl Type {
         clang::is_valid_identifier(name)
     }
 
+    /// Takes `name`, and returns a suitable identifier representation for it.
+    pub fn sanitize_name<'a>(name: &'a str) -> Cow<'a, str> {
+        if Self::is_valid_identifier(name) {
+            return Cow::Borrowed(name)
+        }
+
+        let name = name.replace(|c| c == ' ' || c == ':' || c == '.' , "_");
+        debug_assert!(Self::is_valid_identifier(&name));
+
+        Cow::Owned(name)
+    }
+
     /// See safe_canonical_type.
     pub fn canonical_type<'tr>(&'tr self,
                                ctx: &'tr BindgenContext)

tests/expectations/tests/template.rs

@@ -29,10 +29,16 @@ extern "C" {
     pub fn bar(foo: Foo<::std::os::raw::c_int>);
 }
 #[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct mozilla_Foo {
+    _unused: [u8; 0],
+}
+#[repr(C)]
 #[derive(Debug, Copy, Hash)]
 pub struct C {
     pub mB: B<::std::os::raw::c_uint>,
     pub mBConstPtr: B<*const ::std::os::raw::c_int>,
+    pub mBConstStructPtr: B<*const mozilla_Foo>,
     pub mBConst: B<::std::os::raw::c_int>,
     pub mBVolatile: B<::std::os::raw::c_int>,
     pub mBConstBool: B<bool>,
@@ -41,7 +47,7 @@ pub struct C {
 }
 #[test]
 fn bindgen_test_layout_C() {
-    assert_eq!(::std::mem::size_of::<C>() , 32usize , concat ! (
+    assert_eq!(::std::mem::size_of::<C>() , 40usize , concat ! (
                "Size of: " , stringify ! ( C ) ));
     assert_eq! (::std::mem::align_of::<C>() , 8usize , concat ! (
                 "Alignment of " , stringify ! ( C ) ));
@@ -54,29 +60,34 @@ fn bindgen_test_layout_C() {
                 } , 8usize , concat ! (
                 "Alignment of field: " , stringify ! ( C ) , "::" , stringify
                 ! ( mBConstPtr ) ));
+    assert_eq! (unsafe {
+                & ( * ( 0 as * const C ) ) . mBConstStructPtr as * const _ as
+                usize } , 16usize , concat ! (
+                "Alignment of field: " , stringify ! ( C ) , "::" , stringify
+                ! ( mBConstStructPtr ) ));
     assert_eq! (unsafe {
                 & ( * ( 0 as * const C ) ) . mBConst as * const _ as usize } ,
-                16usize , concat ! (
+                24usize , concat ! (
                 "Alignment of field: " , stringify ! ( C ) , "::" , stringify
                 ! ( mBConst ) ));
     assert_eq! (unsafe {
                 & ( * ( 0 as * const C ) ) . mBVolatile as * const _ as usize
-                } , 20usize , concat ! (
+                } , 28usize , concat ! (
                 "Alignment of field: " , stringify ! ( C ) , "::" , stringify
                 ! ( mBVolatile ) ));
     assert_eq! (unsafe {
                 & ( * ( 0 as * const C ) ) . mBConstBool as * const _ as usize
-                } , 24usize , concat ! (
+                } , 32usize , concat ! (
                 "Alignment of field: " , stringify ! ( C ) , "::" , stringify
                 ! ( mBConstBool ) ));
     assert_eq! (unsafe {
                 & ( * ( 0 as * const C ) ) . mBConstChar as * const _ as usize
-                } , 26usize , concat ! (
+                } , 34usize , concat ! (
                 "Alignment of field: " , stringify ! ( C ) , "::" , stringify
                 ! ( mBConstChar ) ));
     assert_eq! (unsafe {
                 & ( * ( 0 as * const C ) ) . mBArray as * const _ as usize } ,
-                28usize , concat ! (
+                36usize , concat ! (
                 "Alignment of field: " , stringify ! ( C ) , "::" , stringify
                 ! ( mBArray ) ));
 }
@@ -331,6 +342,17 @@ fn __bindgen_test_layout_B_open0_ptr_const_int_close0_instantiation() {
                B<*const ::std::os::raw::c_int> ) ));
 }
 #[test]
+fn __bindgen_test_layout_B_open0_ptr_const_mozilla__Foo_close0_instantiation() {
+    assert_eq!(::std::mem::size_of::<B<*const mozilla_Foo>>() , 8usize ,
+               concat ! (
+               "Size of template specialization: " , stringify ! (
+               B<*const mozilla_Foo> ) ));
+    assert_eq!(::std::mem::align_of::<B<*const mozilla_Foo>>() , 8usize ,
+               concat ! (
+               "Alignment of template specialization: " , stringify ! (
+               B<*const mozilla_Foo> ) ));
+}
+#[test]
 fn __bindgen_test_layout_B_open0_const_int_close0_instantiation() {
     assert_eq!(::std::mem::size_of::<B<::std::os::raw::c_int>>() , 4usize ,
                concat ! (

tests/headers/template.hpp

@@ -7,14 +7,19 @@ template<typename T, typename U> class Foo {
 };
 
 template<typename T> class B {
-    T m_member;
+    T m_member { 0 };
 };
 
 void bar(Foo<int, int> foo);
 
+namespace mozilla {
+class Foo;
+};
+
 struct C {
     B<unsigned int> mB;
     B<const int*> mBConstPtr;
+    B<const mozilla::Foo*> mBConstStructPtr;
     B<const int> mBConst;
     B<volatile int> mBVolatile;
     B<const bool> mBConstBool;