Auto merge of #575 - fitzgen:issue-574-anonymous-template-parameters-and-assertion-failure, r=fitzgen

Allow anonymous template types

We have various assertions that the only way that some template parameter related methods will return `None` is if the template definition is marked opaque. These assertions fail in the presence of test cases with unnamed template types, because we never save an IR item for the template type, and subsequently think that the template definition has no template parameters. The assertions are in fact sound and correct, so it doesn't make sense to remove them. Instead it is more correct to save IR items for the anonymous template types and simply let the template usage analysis prevent them from getting
codegen'd.

Fixes #574

r? @emilio

Author: bors-servo

src/ir/comp.rs

@@ -597,14 +597,6 @@ impl CompInfo {
                     ci.packed = true;
                 }
                 CXCursor_TemplateTypeParameter => {
-                    // Yes! You can arrive here with an empty template parameter
-                    // name! Awesome, isn't it?
-                    //
-                    // see tests/headers/empty_template_param_name.hpp
-                    if cur.spelling().is_empty() {
-                        return CXChildVisit_Continue;
-                    }
-
                     let param = Item::named_type(None, cur, ctx)
                         .expect("Item::named_type should't fail when pointing \
                                  at a TemplateTypeParameter");

src/ir/context.rs

@@ -1032,10 +1032,10 @@ impl<'ctx> BindgenContext<'ctx> {
                     }
 
                     return self.instantiate_template(with_id,
-                                              id,
-                                              parent_id,
-                                              ty,
-                                              location)
+                                                     id,
+                                                     parent_id,
+                                                     ty,
+                                                     location)
                         .or_else(|| Some(id));
                 }
 

src/ir/item.rs

@@ -19,6 +19,7 @@ use std::collections::BTreeSet;
 use std::fmt::Write;
 use std::io;
 use std::iter;
+use regex;
 
 /// A trait to get the canonical name from an item.
 ///
@@ -1313,8 +1314,19 @@ impl ClangItemParser for Item {
         fn is_template_with_spelling(refd: &clang::Cursor,
                                      spelling: &str)
                                      -> bool {
-            refd.kind() == clang_sys::CXCursor_TemplateTypeParameter &&
-            refd.spelling() == spelling
+            lazy_static! {
+                static ref ANON_TYPE_PARAM_RE: regex::Regex =
+                    regex::Regex::new(r"^type\-parameter\-\d+\-\d+$").unwrap();
+            }
+
+            if refd.kind() != clang_sys::CXCursor_TemplateTypeParameter {
+                return false;
+            }
+
+            let refd_spelling = refd.spelling();
+            refd_spelling == spelling ||
+                // Allow for anonymous template parameters.
+                (refd_spelling.is_empty() && ANON_TYPE_PARAM_RE.is_match(spelling.as_ref()))
         }
 
         let definition = if is_template_with_spelling(&location,

src/ir/ty.rs

@@ -292,8 +292,12 @@ impl Type {
 
     /// Creates a new named type, with name `name`.
     pub fn named(name: String) -> Self {
-        assert!(!name.is_empty());
-        Self::new(Some(name), None, TypeKind::Named, false)
+        let name = if name.is_empty() {
+            None
+        } else {
+            Some(name)
+        };
+        Self::new(name, None, TypeKind::Named, false)
     }
 
     /// Is this a floating point type?
@@ -1127,12 +1131,6 @@ impl Type {
                                                               ctx);
                                         }
                                         CXCursor_TemplateTypeParameter => {
-                                            // See the comment in src/ir/comp.rs
-                                            // about the same situation.
-                                            if cur.spelling().is_empty() {
-                                                return CXChildVisit_Continue;
-                                            }
-
                                             let param =
                                                 Item::named_type(None,
                                                                  cur,

tests/expectations/tests/anonymous-template-types.rs

@@ -0,0 +1,33 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct Foo<T> {
+    pub t_member: T,
+}
+impl <T> Default for Foo<T> {
+    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
+}
+#[repr(C)]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct Bar {
+    pub member: ::std::os::raw::c_schar,
+}
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct Quux<V> {
+    pub v_member: V,
+}
+impl <V> Default for Quux<V> {
+    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
+}
+#[repr(C)]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct Lobo {
+    pub also_member: ::std::os::raw::c_schar,
+}
+pub type AliasWithAnonType = ::std::os::raw::c_schar;

tests/expectations/tests/bad-namespace-parenthood-inheritance.rs

@@ -5,15 +5,15 @@
 
 
 #[repr(C)]
-#[derive(Debug, Default, Copy, Clone)]
+#[derive(Debug, Copy, Clone)]
 pub struct std_char_traits {
     pub _address: u8,
 }
+impl Default for std_char_traits {
+    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
+}
 #[repr(C)]
-#[derive(Debug, Default, Copy)]
+#[derive(Debug, Default, Copy, Clone)]
 pub struct __gnu_cxx_char_traits {
     pub _address: u8,
 }
-impl Clone for __gnu_cxx_char_traits {
-    fn clone(&self) -> Self { *self }
-}

tests/expectations/tests/issue-358.rs

@@ -13,13 +13,10 @@ impl Default for JS_PersistentRooted {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[repr(C)]
-#[derive(Debug, Copy)]
+#[derive(Debug, Copy, Clone)]
 pub struct a {
     pub b: *mut a,
 }
-impl Clone for a {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for a {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }

tests/expectations/tests/issue-544-stylo-creduce.rs

@@ -5,10 +5,7 @@
 
 
 #[repr(C)]
-#[derive(Debug, Default, Copy)]
+#[derive(Debug, Default, Copy, Clone)]
 pub struct a {
     pub _address: u8,
 }
-impl Clone for a {
-    fn clone(&self) -> Self { *self }
-}

tests/expectations/tests/issue-569-non-type-template-params-causing-layout-test-failures.rs

@@ -32,7 +32,7 @@ impl Default for JS_AutoIdVector {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[test]
-fn __bindgen_test_layout_JS_Base_instantiation_16() {
+fn __bindgen_test_layout_JS_Base_instantiation_20() {
     assert_eq!(::std::mem::size_of::<JS_Base>() , 1usize , concat ! (
                "Size of template specialization: " , stringify ! ( JS_Base )
                ));

tests/expectations/tests/issue-574-assertion-failure-in-codegen.rs

@@ -0,0 +1,38 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+#[repr(C)]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct a {
+    pub _address: u8,
+}
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct _bindgen_ty_1 {
+    pub ar: a,
+}
+#[test]
+fn bindgen_test_layout__bindgen_ty_1() {
+    assert_eq!(::std::mem::size_of::<_bindgen_ty_1>() , 1usize , concat ! (
+               "Size of: " , stringify ! ( _bindgen_ty_1 ) ));
+    assert_eq! (::std::mem::align_of::<_bindgen_ty_1>() , 1usize , concat ! (
+                "Alignment of " , stringify ! ( _bindgen_ty_1 ) ));
+    assert_eq! (unsafe {
+                & ( * ( 0 as * const _bindgen_ty_1 ) ) . ar as * const _ as
+                usize } , 0usize , concat ! (
+                "Alignment of field: " , stringify ! ( _bindgen_ty_1 ) , "::"
+                , stringify ! ( ar ) ));
+}
+impl Clone for _bindgen_ty_1 {
+    fn clone(&self) -> Self { *self }
+}
+impl Default for _bindgen_ty_1 {
+    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
+}
+extern "C" {
+    #[link_name = "AutoIdVector"]
+    pub static mut AutoIdVector: _bindgen_ty_1;
+}

tests/headers/anonymous-template-types.hpp

@@ -0,0 +1,24 @@
+// bindgen-flags: -- -std=c++14
+
+template <typename T, typename>
+struct Foo {
+    T t_member;
+};
+
+template <typename U, typename>
+struct Bar {
+    char member;
+};
+
+template <typename, typename V>
+struct Quux {
+    V v_member;
+};
+
+template <typename, typename W>
+struct Lobo {
+    char also_member;
+};
+
+template <typename>
+using AliasWithAnonType = char;

tests/headers/issue-574-assertion-failure-in-codegen.hpp

@@ -0,0 +1,6 @@
+// bindgen-flags: -- -std=c++14
+
+template <class> class a {};
+class {
+  a<int> ar;
+} AutoIdVector;