Auto merge of #549 - emilio:call-conv-lost, r=fitzgen

Fix calling convention propagation for function pointers.

This sucks, but works.

The full solution is a refactoring that needs more thought than the time I'm
able to dedicate to bindgen right now, see the comment for details.

r? @fitzgen

Author: bors-servo

src/clang.rs

@@ -633,9 +633,10 @@ impl Eq for Type {}
 impl fmt::Debug for Type {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         write!(fmt,
-               "Type({}, kind: {}, decl: {:?}, canon: {:?})",
+               "Type({}, kind: {}, cconv: {}, decl: {:?}, canon: {:?})",
                self.spelling(),
                type_to_str(self.kind()),
+               self.call_conv(),
                self.declaration(),
                self.declaration().canonical())
     }
@@ -1520,6 +1521,8 @@ pub fn ast_dump(c: &Cursor, depth: isize) -> CXChildVisitResult {
             return;
         }
 
+        print_indent(depth, format!(" {}cconv = {}", prefix, ty.call_conv()));
+
         print_indent(depth,
                      format!(" {}spelling = \"{}\"", prefix, ty.spelling()));
         let num_template_args =

src/ir/function.rs

@@ -5,6 +5,7 @@ use super::dot::DotAttributes;
 use super::item::Item;
 use super::traversal::{EdgeKind, Trace, Tracer};
 use super::ty::TypeKind;
+use ir::derive::CanDeriveDebug;
 use clang;
 use clang_sys::CXCallingConv;
 use parse::{ClangItemParser, ClangSubItemParser, ParseError, ParseResult};
@@ -349,3 +350,26 @@ impl Trace for FunctionSig {
         }
     }
 }
+
+// Function pointers follow special rules, see:
+//
+// https://github.com/servo/rust-bindgen/issues/547,
+// https://github.com/rust-lang/rust/issues/38848,
+// and https://github.com/rust-lang/rust/issues/40158
+//
+// Note that copy is always derived, so we don't need to implement it.
+impl CanDeriveDebug for FunctionSig {
+    type Extra = ();
+
+    fn can_derive_debug(&self, _ctx: &BindgenContext, _: ()) -> bool {
+        const RUST_DERIVE_FUNPTR_LIMIT: usize = 12;
+        if self.argument_types.len() > RUST_DERIVE_FUNPTR_LIMIT {
+            return false;
+        }
+
+        match self.abi {
+            Some(abi::Abi::C) | None => true,
+            _ => false,
+        }
+    }
+}

src/ir/ty.rs

@@ -605,6 +605,13 @@ impl CanDeriveDebug for Type {
             TypeKind::Comp(ref info) => {
                 info.can_derive_debug(ctx, self.layout(ctx))
             }
+            TypeKind::Pointer(inner) => {
+                let inner = ctx.resolve_type(inner);
+                if let TypeKind::Function(ref sig) = *inner.canonical_type(ctx).kind() {
+                    return sig.can_derive_debug(ctx, ());
+                }
+                return true;
+            }
             _ => true,
         }
     }
@@ -636,6 +643,7 @@ impl CanDeriveDefault for Type {
             TypeKind::ObjCSel |
             TypeKind::ObjCInterface(..) |
             TypeKind::Enum(..) => false,
+
             TypeKind::Function(..) |
             TypeKind::Int(..) |
             TypeKind::Float(..) |
@@ -877,6 +885,7 @@ impl Type {
         let kind = match ty_kind {
             CXType_Unexposed if *ty != canonical_ty &&
                                 canonical_ty.kind() != CXType_Invalid &&
+                                ty.ret_type().is_none() &&
                                 // Sometime clang desugars some types more than
                                 // what we need, specially with function
                                 // pointers.
@@ -1132,7 +1141,32 @@ impl Type {
             CXType_ObjCObjectPointer |
             CXType_MemberPointer |
             CXType_Pointer => {
-                let inner = Item::from_ty_or_ref(ty.pointee_type().unwrap(),
+                // Fun fact: the canonical type of a pointer type may sometimes
+                // contain information we need but isn't present in the concrete
+                // type (yeah, I'm equally wat'd).
+                //
+                // Yet we still have trouble if we unconditionally trust the
+                // canonical type, like too-much desugaring (sigh).
+                //
+                // See tests/headers/call-conv-field.h for an example.
+                //
+                // Since for now the only identifier cause of breakage is the
+                // ABI for function pointers, and different ABI mixed with
+                // problematic stuff like that one is _extremely_ unlikely and
+                // can be bypassed via blacklisting, we do the check explicitly
+                // (as hacky as it is).
+                //
+                // Yet we should probably (somehow) get the best of both worlds,
+                // presumably special-casing function pointers as a whole, yet
+                // someone is going to need to care about typedef'd function
+                // pointers, etc, which isn't trivial given function pointers
+                // are mostly unexposed. I don't have the time for it right now.
+                let mut pointee = ty.pointee_type().unwrap();
+                let canonical_pointee = canonical_ty.pointee_type().unwrap();
+                if pointee.call_conv() != canonical_pointee.call_conv() {
+                    pointee = canonical_pointee;
+                }
+                let inner = Item::from_ty_or_ref(pointee,
                                                  location,
                                                  None,
                                                  ctx);

tests/expectations/tests/call-conv-field.rs

@@ -0,0 +1,42 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+#[repr(C)]
+#[derive(Copy)]
+pub struct JNINativeInterface_ {
+    pub GetVersion: ::std::option::Option<unsafe extern "stdcall" fn(env:
+                                                                         *mut ::std::os::raw::c_void)
+                                              -> ::std::os::raw::c_int>,
+    pub __hack: ::std::os::raw::c_ulonglong,
+}
+#[test]
+fn bindgen_test_layout_JNINativeInterface_() {
+    assert_eq!(::std::mem::size_of::<JNINativeInterface_>() , 16usize , concat
+               ! ( "Size of: " , stringify ! ( JNINativeInterface_ ) ));
+    assert_eq! (::std::mem::align_of::<JNINativeInterface_>() , 8usize ,
+                concat ! (
+                "Alignment of " , stringify ! ( JNINativeInterface_ ) ));
+    assert_eq! (unsafe {
+                & ( * ( 0 as * const JNINativeInterface_ ) ) . GetVersion as *
+                const _ as usize } , 0usize , concat ! (
+                "Alignment of field: " , stringify ! ( JNINativeInterface_ ) ,
+                "::" , stringify ! ( GetVersion ) ));
+    assert_eq! (unsafe {
+                & ( * ( 0 as * const JNINativeInterface_ ) ) . __hack as *
+                const _ as usize } , 8usize , concat ! (
+                "Alignment of field: " , stringify ! ( JNINativeInterface_ ) ,
+                "::" , stringify ! ( __hack ) ));
+}
+impl Clone for JNINativeInterface_ {
+    fn clone(&self) -> Self { *self }
+}
+impl Default for JNINativeInterface_ {
+    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
+}
+extern "stdcall" {
+    #[link_name = "_bar@0"]
+    pub fn bar();
+}

tests/expectations/tests/derive-fn-ptr.rs

@@ -0,0 +1,46 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+pub type my_fun_t =
+    ::std::option::Option<unsafe extern "C" fn(arg1: ::std::os::raw::c_int,
+                                               arg2: ::std::os::raw::c_int,
+                                               arg3: ::std::os::raw::c_int,
+                                               arg4: ::std::os::raw::c_int,
+                                               arg5: ::std::os::raw::c_int,
+                                               arg6: ::std::os::raw::c_int,
+                                               arg7: ::std::os::raw::c_int,
+                                               arg8: ::std::os::raw::c_int,
+                                               arg9: ::std::os::raw::c_int,
+                                               arg10: ::std::os::raw::c_int,
+                                               arg11: ::std::os::raw::c_int,
+                                               arg12: ::std::os::raw::c_int,
+                                               arg13: ::std::os::raw::c_int,
+                                               arg14: ::std::os::raw::c_int,
+                                               arg15: ::std::os::raw::c_int,
+                                               arg16: ::std::os::raw::c_int)>;
+#[repr(C)]
+#[derive(Copy)]
+pub struct Foo {
+    pub callback: my_fun_t,
+}
+#[test]
+fn bindgen_test_layout_Foo() {
+    assert_eq!(::std::mem::size_of::<Foo>() , 8usize , concat ! (
+               "Size of: " , stringify ! ( Foo ) ));
+    assert_eq! (::std::mem::align_of::<Foo>() , 8usize , concat ! (
+                "Alignment of " , stringify ! ( Foo ) ));
+    assert_eq! (unsafe {
+                & ( * ( 0 as * const Foo ) ) . callback as * const _ as usize
+                } , 0usize , concat ! (
+                "Alignment of field: " , stringify ! ( Foo ) , "::" ,
+                stringify ! ( callback ) ));
+}
+impl Clone for Foo {
+    fn clone(&self) -> Self { *self }
+}
+impl Default for Foo {
+    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
+}

tests/headers/call-conv-field.h

@@ -0,0 +1,10 @@
+// bindgen-flags: -- -target i686-pc-win32
+// bindgen-unstable
+
+struct JNINativeInterface_ {
+  int (__stdcall *GetVersion)(void *env);
+  unsigned long long __hack; // A hack so the field alignment is the same than
+                             // for 64-bit, where we run CI.
+};
+
+__stdcall void bar();

tests/headers/derive-fn-ptr.h

@@ -0,0 +1,8 @@
+typedef void (*my_fun_t)(int, int, int, int,
+                         int, int, int, int,
+                         int, int, int, int,
+                         int, int, int, int);
+
+struct Foo {
+  my_fun_t callback;
+};