Support #[repr(packed(N))] on Rust 1.33+

Fixes https://github.com/rust-lang/rust-bindgen/issues/537.

9 files changed, 222 insertions, 21 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d4f7c57f..01be62f2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -56,11 +56,14 @@ Released YYYY/MM/DD
 
 ## Changed
 
+- `#pragma pack(n)` is now translated to `#[repr(C, packed(n))]` when targeting Rust 1.33+. [#537][]
+
+[#537]: https://github.com/rust-lang-nursery/rust-bindgen/issues/537
+
 * Bitfield enums now use `#[repr(transparent)]` instead of `#[repr(C)]` when targeting Rust 1.28+. [#1474][]
 
 [#1474]: https://github.com/rust-lang-nursery/rust-bindgen/issues/1474
 
-
 ## Deprecated
 
 * TODO (or remove section if none)
diff --git a/src/codegen/helpers.rs b/src/codegen/helpers.rs
index d6377194..457979b6 100644
--- a/src/codegen/helpers.rs
+++ b/src/codegen/helpers.rs
@@ -7,6 +7,7 @@ use quote::TokenStreamExt;
 
 pub mod attributes {
     use proc_macro2::{Ident, Span, TokenStream};
+    use std::str::FromStr;
 
     pub fn repr(which: &str) -> TokenStream {
         let which = Ident::new(which, Span::call_site());
@@ -16,7 +17,7 @@ pub mod attributes {
     }
 
     pub fn repr_list(which_ones: &[&str]) -> TokenStream {
-        let which_ones = which_ones.iter().cloned().map(|one| Ident::new(one, Span::call_site()));
+        let which_ones = which_ones.iter().cloned().map(|one| TokenStream::from_str(one).expect("repr to be valid"));
         quote! {
             #[repr( #( #which_ones ),* )]
         }
diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
index 6fd7e7d3..d3395edf 100644
--- a/src/codegen/mod.rs
+++ b/src/codegen/mod.rs
@@ -1673,7 +1673,10 @@ impl CodeGenerator for CompInfo {
             attributes.push(attributes::doc(comment));
         }
         if packed && !is_opaque {
-            attributes.push(attributes::repr_list(&["C", "packed"]));
+            let n = layout.map_or(1, |l| l.align);
+            assert!(ctx.options().rust_features().repr_packed_n || n == 1);
+            let packed_repr = if n == 1 { "packed".to_string() } else { format!("packed({})", n) };
+            attributes.push(attributes::repr_list(&["C", &packed_repr]));
         } else {
             attributes.push(attributes::repr("C"));
         }
diff --git a/src/features.rs b/src/features.rs
index 1f700039..b5a63402 100644
--- a/src/features.rs
+++ b/src/features.rs
@@ -102,6 +102,8 @@ macro_rules! rust_target_base {
             => Stable_1_27 => 1.27;
             /// Rust stable 1.28
             => Stable_1_28 => 1.28;
+            /// Rust stable 1.33
+            => Stable_1_33 => 1.33;
             /// Nightly rust
             => Nightly => nightly;
         );
@@ -190,6 +192,10 @@ rust_feature_def!(
         /// repr(transparent) ([PR](https://github.com/rust-lang/rust/pull/51562))
         => repr_transparent;
     }
+    Stable_1_33 {
+        /// repr(packed(N)) ([PR](https://github.com/rust-lang/rust/pull/57049))
+        => repr_packed_n;
+    }
     Nightly {
         /// `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202))
         => thiscall_abi;
diff --git a/src/ir/comp.rs b/src/ir/comp.rs
index fd4c8274..12a8fa32 100644
--- a/src/ir/comp.rs
+++ b/src/ir/comp.rs
@@ -1649,16 +1649,19 @@ impl IsOpaque for CompInfo {
             return true;
         }
 
-        // We don't have `#[repr(packed = "N")]` in Rust yet, so the best we can
-        // do is make this struct opaque.
-        //
-        // See https://github.com/rust-lang-nursery/rust-bindgen/issues/537 and
-        // https://github.com/rust-lang/rust/issues/33158
-        if self.is_packed(ctx, layout) && layout.map_or(false, |l| l.align > 1) {
-            warn!("Found a type that is both packed and aligned to greater than \
-                   1; Rust doesn't have `#[repr(packed = \"N\")]` yet, so we \
-                   are treating it as opaque");
-            return true;
+        if !ctx.options().rust_features().repr_packed_n {
+            // If we don't have `#[repr(packed(N)]`, the best we can
+            // do is make this struct opaque.
+            //
+            // See https://github.com/rust-lang-nursery/rust-bindgen/issues/537 and
+            // https://github.com/rust-lang/rust/issues/33158
+            if self.is_packed(ctx, layout) && layout.map_or(false, |l| l.align > 1) {
+                warn!("Found a type that is both packed and aligned to greater than \
+                       1; Rust before version 1.33 doesn't have `#[repr(packed(N))]`, so we \
+                       are treating it as opaque. You may wish to set bindgen's rust target \
+                       version to 1.33 or later to enable `#[repr(packed(N))]` support.");
+                return true;
+            }
         }
 
         false
diff --git a/tests/expectations/tests/issue-537-repr-packed-n.rs b/tests/expectations/tests/issue-537-repr-packed-n.rs
new file mode 100644
index 00000000..1c930a8b
--- /dev/null
+++ b/tests/expectations/tests/issue-537-repr-packed-n.rs
@@ -0,0 +1,151 @@
+/* automatically generated by rust-bindgen */
+
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+#![cfg(feature = "nightly")]
+
+/// This should not be opaque; we can see the attributes and can pack the
+/// struct.
+#[repr(C, packed)]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct AlignedToOne {
+    pub i: ::std::os::raw::c_int,
+}
+#[test]
+fn bindgen_test_layout_AlignedToOne() {
+    assert_eq!(
+        ::std::mem::size_of::<AlignedToOne>(),
+        4usize,
+        concat!("Size of: ", stringify!(AlignedToOne))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<AlignedToOne>(),
+        1usize,
+        concat!("Alignment of ", stringify!(AlignedToOne))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<AlignedToOne>())).i as *const _ as usize },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(AlignedToOne),
+            "::",
+            stringify!(i)
+        )
+    );
+}
+/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
+#[repr(C, packed(2))]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct AlignedToTwo {
+    pub i: ::std::os::raw::c_int,
+}
+#[test]
+fn bindgen_test_layout_AlignedToTwo() {
+    assert_eq!(
+        ::std::mem::size_of::<AlignedToTwo>(),
+        4usize,
+        concat!("Size of: ", stringify!(AlignedToTwo))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<AlignedToTwo>(),
+        2usize,
+        concat!("Alignment of ", stringify!(AlignedToTwo))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<AlignedToTwo>())).i as *const _ as usize },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(AlignedToTwo),
+            "::",
+            stringify!(i)
+        )
+    );
+}
+/// This should not be opaque because although `libclang` doesn't give us the
+/// `#pragma pack(1)`, we can detect that alignment is 1 and add
+/// `#[repr(packed)]` to the struct ourselves.
+#[repr(C, packed)]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct PackedToOne {
+    pub x: ::std::os::raw::c_int,
+    pub y: ::std::os::raw::c_int,
+}
+#[test]
+fn bindgen_test_layout_PackedToOne() {
+    assert_eq!(
+        ::std::mem::size_of::<PackedToOne>(),
+        8usize,
+        concat!("Size of: ", stringify!(PackedToOne))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<PackedToOne>(),
+        1usize,
+        concat!("Alignment of ", stringify!(PackedToOne))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<PackedToOne>())).x as *const _ as usize },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(PackedToOne),
+            "::",
+            stringify!(x)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<PackedToOne>())).y as *const _ as usize },
+        4usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(PackedToOne),
+            "::",
+            stringify!(y)
+        )
+    );
+}
+/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
+#[repr(C, packed(2))]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct PackedToTwo {
+    pub x: ::std::os::raw::c_int,
+    pub y: ::std::os::raw::c_int,
+}
+#[test]
+fn bindgen_test_layout_PackedToTwo() {
+    assert_eq!(
+        ::std::mem::size_of::<PackedToTwo>(),
+        8usize,
+        concat!("Size of: ", stringify!(PackedToTwo))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<PackedToTwo>(),
+        2usize,
+        concat!("Alignment of ", stringify!(PackedToTwo))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<PackedToTwo>())).x as *const _ as usize },
+        0usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(PackedToTwo),
+            "::",
+            stringify!(x)
+        )
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<PackedToTwo>())).y as *const _ as usize },
+        4usize,
+        concat!(
+            "Offset of field: ",
+            stringify!(PackedToTwo),
+            "::",
+            stringify!(y)
+        )
+    );
+}
diff --git a/tests/expectations/tests/issue-537.rs b/tests/expectations/tests/issue-537.rs
index 8913db36..94c6dd3b 100644
--- a/tests/expectations/tests/issue-537.rs
+++ b/tests/expectations/tests/issue-537.rs
@@ -37,8 +37,8 @@ fn bindgen_test_layout_AlignedToOne() {
         )
     );
 }
-/// This should be opaque because although we can see the attributes, Rust
-/// doesn't have `#[repr(packed = "N")]` yet.
+/// This should be opaque because although we can see the attributes, Rust before
+/// 1.33 doesn't have `#[repr(packed(N))]`.
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct AlignedToTwo {
@@ -100,8 +100,8 @@ fn bindgen_test_layout_PackedToOne() {
     );
 }
 /// In this case, even if we can detect the weird alignment triggered by
-/// `#pragma pack(2)`, we can't do anything about it because Rust doesn't have
-/// `#[repr(packed = "N")]`. Therefore, we must make it opaque.
+/// `#pragma pack(2)`, we can't do anything about it because Rust before 1.33
+/// doesn't have `#[repr(packed(N))]`. Therefore, we must make it opaque.
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct PackedToTwo {
diff --git a/tests/headers/issue-537-repr-packed-n.h b/tests/headers/issue-537-repr-packed-n.h
new file mode 100644
index 00000000..f4c0070a
--- /dev/null
+++ b/tests/headers/issue-537-repr-packed-n.h
@@ -0,0 +1,34 @@
+// bindgen-flags: --raw-line '#![cfg(feature = "nightly")]' --rust-target 1.33
+
+/// This should not be opaque; we can see the attributes and can pack the
+/// struct.
+struct AlignedToOne {
+    int i;
+} __attribute__ ((packed,aligned(1)));
+
+/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
+struct AlignedToTwo {
+    int i;
+} __attribute__ ((packed,aligned(2)));
+
+#pragma pack(1)
+
+/// This should not be opaque because although `libclang` doesn't give us the
+/// `#pragma pack(1)`, we can detect that alignment is 1 and add
+/// `#[repr(packed)]` to the struct ourselves.
+struct PackedToOne {
+    int x;
+    int y;
+};
+
+#pragma pack()
+
+#pragma pack(2)
+
+/// This should be be packed because Rust 1.33 has `#[repr(packed(N))]`.
+struct PackedToTwo {
+    int x;
+    int y;
+};
+
+#pragma pack()
diff --git a/tests/headers/issue-537.h b/tests/headers/issue-537.h
index 9a1072a7..a773199f 100644
--- a/tests/headers/issue-537.h
+++ b/tests/headers/issue-537.h
@@ -4,8 +4,8 @@ struct AlignedToOne {
     int i;
 } __attribute__ ((packed,aligned(1)));
 
-/// This should be opaque because although we can see the attributes, Rust
-/// doesn't have `#[repr(packed = "N")]` yet.
+/// This should be opaque because although we can see the attributes, Rust before
+/// 1.33 doesn't have `#[repr(packed(N))]`.
 struct AlignedToTwo {
     int i;
 } __attribute__ ((packed,aligned(2)));
@@ -25,8 +25,8 @@ struct PackedToOne {
 #pragma pack(2)
 
 /// In this case, even if we can detect the weird alignment triggered by
-/// `#pragma pack(2)`, we can't do anything about it because Rust doesn't have
-/// `#[repr(packed = "N")]`. Therefore, we must make it opaque.
+/// `#pragma pack(2)`, we can't do anything about it because Rust before 1.33
+/// doesn't have `#[repr(packed(N))]`. Therefore, we must make it opaque.
 struct PackedToTwo {
     int x;
     int y;