Regression in libclang with template's default type parameters

[emilio]

Using:

$ clang --version                                                                                                                                                                          
clang version 5.0.0 (git@github.com:llvm-mirror/clang 9d034c720c332a901de795c1528f6fbf437531a2) (git@github.com:llvm-mirror/llvm 5c49cf1beb13f4095a0fe1709d0da4ba310e13b1)
Target: x86_64-unknown-linux-gnu

cargo test generates the given diff compared with master:

diff --git a/tests/expectations/tests/what_is_going_on.rs b/tests/expectations/tests/what_is_going_on.rs
index e5194c02..d9af9c11 100644
--- a/tests/expectations/tests/what_is_going_on.rs
+++ b/tests/expectations/tests/what_is_going_on.rs
@@ -29,4 +29,4 @@ pub struct PointTyped<F> {
 impl <F> Default for PointTyped<F> {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
-pub type IntPoint = PointTyped<f32>;
+pub type IntPoint = PointTyped;

[emilio]

cc @fitzgen

[fitzgen]

I just built with master llvm and clang and I'm getting even more failures:

---- header_issue_544_stylo_creduce_2_hpp stdout ----
	diff expected generated
--- expected: "/home/fitzgen/rust-bindgen/tests/expectations/tests/issue-544-stylo-creduce-2.rs"
+++ generated from: "/home/fitzgen/rust-bindgen/tests/headers/issue-544-stylo-creduce-2.hpp"
 /* automatically generated by rust-bindgen */
 
 
 #![allow(non_snake_case)]
 
 
 #[repr(C)]
 pub struct Foo {
-    pub member: Foo_SecondAlias,
+    pub member: Foo_Foo::SecondAlias,
 }
-pub type Foo_FirstAlias = [u8; 0usize];
-pub type Foo_SecondAlias = [u8; 0usize];
+pub type Foo_Foo::FirstAlias = [u8; 0usize];
+pub type Foo_Foo::SecondAlias = [u8; 0usize];
 impl Default for Foo {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
thread 'header_issue_544_stylo_creduce_2_hpp' panicked at 'Header and binding differ!', /home/fitzgen/rust-bindgen/target/debug/build/bindgen-fddff672ca36ebc4/out/tests.rs:97
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- header_issue_569_non_type_template_params_causing_layout_test_failures_hpp stdout ----
	diff expected generated
--- expected: "/home/fitzgen/rust-bindgen/tests/expectations/tests/issue-569-non-type-template-params-causing-layout-test-failures.rs"
+++ generated from: "/home/fitzgen/rust-bindgen/tests/headers/issue-569-non-type-template-params-causing-layout-test-failures.hpp"
 /* automatically generated by rust-bindgen */
 
 
 #![allow(non_snake_case)]
 
 
 pub const ENUM_VARIANT_1: _bindgen_ty_1 = _bindgen_ty_1::ENUM_VARIANT_1;
 pub const ENUM_VARIANT_2: _bindgen_ty_1 = _bindgen_ty_1::ENUM_VARIANT_2;
 #[repr(u32)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub enum _bindgen_ty_1 { ENUM_VARIANT_1 = 0, ENUM_VARIANT_2 = 1, }
-pub type JS_Alias = u8;
+pub type JS_JS::Alias = u8;
 #[repr(C)]
 pub struct JS_Base {
-    pub f: JS_Alias,
+    pub f: JS_JS::Alias,
 }
 impl Default for JS_Base {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[repr(C)]
 pub struct JS_AutoIdVector {
     pub _base: JS_Base,
 }
 #[test]
 fn bindgen_test_layout_JS_AutoIdVector() {
     assert_eq!(::std::mem::size_of::<JS_AutoIdVector>() , 1usize , concat ! (
                "Size of: " , stringify ! ( JS_AutoIdVector ) ));
     assert_eq! (::std::mem::align_of::<JS_AutoIdVector>() , 1usize , concat !
                 ( "Alignment of " , stringify ! ( JS_AutoIdVector ) ));
 }
 impl Default for JS_AutoIdVector {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[test]
 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 )
                ));
     assert_eq!(::std::mem::align_of::<JS_Base>() , 1usize , concat ! (
                "Alignment of template specialization: " , stringify ! (
                JS_Base ) ));
 }
thread 'header_issue_569_non_type_template_params_causing_layout_test_failures_hpp' panicked at 'Header and binding differ!', /home/fitzgen/rust-bindgen/target/debug/build/bindgen-fddff672ca36ebc4/out/tests.rs:81

---- header_replace_template_alias_hpp stdout ----
	diff expected generated
--- expected: "/home/fitzgen/rust-bindgen/tests/expectations/tests/replace_template_alias.rs"
+++ generated from: "/home/fitzgen/rust-bindgen/tests/headers/replace_template_alias.hpp"
 /* automatically generated by rust-bindgen */
 
 
 #![allow(non_snake_case)]
 
 
-/// But the replacement type does use T!
-///
-/// <div rustbindgen replaces="JS::detail::MaybeWrapped" />
-pub type JS_detail_MaybeWrapped<T> = T;
+/// Notice how this doesn't use T.
+pub type JS_detail_JS::detail::MaybeWrapped = ::std::os::raw::c_int;
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
-pub struct JS_Rooted<T> {
-    pub ptr: JS_detail_MaybeWrapped<T>,
+pub struct JS_Rooted {
+    pub ptr: JS_detail_JS::detail::MaybeWrapped,
 }
-impl <T> Default for JS_Rooted<T> {
+impl Default for JS_Rooted {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
+/// But the replacement type does use T!
+///
+/// <div rustbindgen replaces="JS::detail::MaybeWrapped" />
+pub type JS_detail_MaybeWrapped<T> = T;
thread 'header_replace_template_alias_hpp' panicked at 'Header and binding differ!', /home/fitzgen/rust-bindgen/target/debug/build/bindgen-fddff672ca36ebc4/out/tests.rs:32

---- header_template_alias_hpp stdout ----
	diff expected generated
--- expected: "/home/fitzgen/rust-bindgen/tests/expectations/tests/template_alias.rs"
+++ generated from: "/home/fitzgen/rust-bindgen/tests/headers/template_alias.hpp"
 /* automatically generated by rust-bindgen */
 
 
 #![allow(non_snake_case)]
 
 
-pub type JS_detail_Wrapped<T> = T;
+pub type JS_detail_JS::detail::Wrapped<T> = T;
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct JS_Rooted<T> {
-    pub ptr: JS_detail_Wrapped<T>,
+    pub ptr: JS_detail_JS::detail::Wrapped<T>,
 }
 impl <T> Default for JS_Rooted<T> {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
thread 'header_template_alias_hpp' panicked at 'Header and binding differ!', /home/fitzgen/rust-bindgen/target/debug/build/bindgen-fddff672ca36ebc4/out/tests.rs:52

---- header_template_alias_namespace_hpp stdout ----
	diff expected generated
--- expected: "/home/fitzgen/rust-bindgen/tests/expectations/tests/template_alias_namespace.rs"
+++ generated from: "/home/fitzgen/rust-bindgen/tests/headers/template_alias_namespace.hpp"
 /* automatically generated by rust-bindgen */
 
 
 #![allow(non_snake_case)]
 
 
 #[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
 pub mod root {
     #[allow(unused_imports)]
     use self::super::root;
     pub mod JS {
         #[allow(unused_imports)]
         use self::super::super::root;
         pub mod detail {
             #[allow(unused_imports)]
             use self::super::super::super::root;
-            pub type Wrapped<T> = T;
+            pub type JS::detail::Wrapped<T> = T;
         }
         #[repr(C)]
         #[derive(Debug, Copy, Clone)]
         pub struct Rooted<T> {
-            pub ptr: root::JS::detail::Wrapped<T>,
+            pub ptr: root::JS::detail::JS::detail::Wrapped<T>,
         }
         impl <T> Default for Rooted<T> {
             fn default() -> Self { unsafe { ::std::mem::zeroed() } }
         }
     }
 }
thread 'header_template_alias_namespace_hpp' panicked at 'Header and binding differ!', /home/fitzgen/rust-bindgen/target/debug/build/bindgen-fddff672ca36ebc4/out/tests.rs:47

---- header_type_alias_template_specialized_hpp stdout ----
	thread 'header_type_alias_template_specialized_hpp' panicked at 'Should have found the template definition one way or another', /checkout/src/libcore/option.rs:785

---- header_what_is_going_on_hpp stdout ----
	diff expected generated
--- expected: "/home/fitzgen/rust-bindgen/tests/expectations/tests/what_is_going_on.rs"
+++ generated from: "/home/fitzgen/rust-bindgen/tests/headers/what_is_going_on.hpp"
 /* automatically generated by rust-bindgen */
 
 
 #![allow(non_snake_case)]
 
 
 #[repr(C)]
 #[derive(Debug, Default, Copy)]
 pub struct UnknownUnits {
     pub _address: u8,
 }
 #[test]
 fn bindgen_test_layout_UnknownUnits() {
     assert_eq!(::std::mem::size_of::<UnknownUnits>() , 1usize , concat ! (
                "Size of: " , stringify ! ( UnknownUnits ) ));
     assert_eq! (::std::mem::align_of::<UnknownUnits>() , 1usize , concat ! (
                 "Alignment of " , stringify ! ( UnknownUnits ) ));
 }
 impl Clone for UnknownUnits {
     fn clone(&self) -> Self { *self }
 }
 pub type Float = f32;
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct PointTyped<F> {
     pub x: F,
     pub y: F,
 }
 impl <F> Default for PointTyped<F> {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
-pub type IntPoint = PointTyped<f32>;
+pub type IntPoint = PointTyped;
thread 'header_what_is_going_on_hpp' panicked at 'Header and binding differ!', /home/fitzgen/rust-bindgen/target/debug/build/bindgen-fddff672ca36ebc4/out/tests.rs:8


failures:
    header_issue_544_stylo_creduce_2_hpp
    header_issue_569_non_type_template_params_causing_layout_test_failures_hpp
    header_replace_template_alias_hpp
    header_template_alias_hpp
    header_template_alias_namespace_hpp
    header_type_alias_template_specialized_hpp
    header_what_is_going_on_hpp

test result: FAILED. 233 passed; 7 failed; 0 ignored; 0 measured

error: test failed

[emilio]

Well, most of them seem relatively easy to deal with, fortunately.

[emilio]

We should probably setup something in CI for this. For a while I was checking manually with llvm trunk, but...

[fitzgen]

So I've started investigating the what_is_going_on.hpp failure. I added edge labels in #587 and I got this super surprising result:

We are parsing the template parameters as fields of the Comp... no idea what's up yet, still digging.

[fitzgen]

No nevermind... I just misinterpreted that graph.

[fitzgen]

We should probably setup something in CI for this. For a while I was checking manually with llvm trunk, but...

Yeah for sure. I don't think we want to block on llvm master failures, but we could run it with Travis's allow_failures set so we can at least see when issues are coming down the pipeline in a future release.

[fitzgen]

It looks like we are correctly determining that the units type parameter is not used, and shouldn't appear in the bindings.

And we are not finding the default value of the F type parameter, nor generating code for it in the bindings.

[fitzgen]

Here is the IR graph with libclang 3.9:

Note that we have the default type for the F type parameter. I don't see (or remember seeing) any code for resolving default types for template type parameters, so I think libclang just stopped feeding us this information...

[fitzgen]

These aren't showing up in our cursor AST dumps, so we must be getting them from clang_{Cursor,Type}_getTemplateArgs.

[fitzgen]

Trying to figure out how to bisect llvm and clang in lockstep together...

[fitzgen]

Note to self: one can get a monorepo of clang + llvm for bisecting together here: http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo

[fitzgen]

It looks like although the template argumetns are no longer in the AST, they are in the canonical type. Here is a snippet from a clang AST dump:

     type.kind = Unexposed
     type.cconv = 100
     type.spelling = "Foo<bool>"
     type.number-of-template-args = 1
     type.is-variadic? false

     type.canonical.kind = Record
     type.canonical.cconv = 100
     type.canonical.spelling = "Foo<bool, int>"
     type.canonical.number-of-template-args = 2
     type.canonical.is-variadic? false

This is still an unhappy situation however because the types that we get out of clang::Cursor::template_args (which calls clang_Type_getTemplateArgumentAsType) don't have declarations, so we can't find the appropriate named template type item.