Merge pull request #1567 from dtolnay/repralign

Improve repr(align) implementation

Author: dtolnay

book/src/shared.md

@@ -247,8 +247,13 @@ C++ data type:
 
 ## Alignment
 
-Enforcing minimum alignment for structs using `repr(align(x))` is supported within the
-CXX bridge module. The alignment value must be a power of two from 1 up to 2<sup>29</sup>.
+The attribute `repr(align(…))` sets a minimum required alignment for a shared
+struct. The alignment value must be a power of two in the range 2<sup>0</sup> to
+2<sup>13</sup>.
+
+This turns into an [`alignas`] specifier in C++.
+
+[`alignas`]: https://en.cppreference.com/w/cpp/language/alignas.html
 
 ```rust,noplayground
 #[cxx::bridge]

gen/src/builtin.rs

@@ -32,6 +32,7 @@ pub(crate) struct Builtins<'a> {
     pub is_complete: bool,
     pub destroy: bool,
     pub deleter_if: bool,
+    pub alignmax: bool,
     pub content: Content<'a>,
 }
 
@@ -228,6 +229,34 @@ pub(super) fn write(out: &mut OutFile) {
         writeln!(out, "}};");
     }
 
+    if builtin.alignmax {
+        include.cstddef = true;
+        out.next_section();
+        writeln!(out, "#ifndef CXXBRIDGE_ALIGNMAX");
+        writeln!(out, "#define CXXBRIDGE_ALIGNMAX");
+        // This would be cleaner as the following, but GCC does not implement
+        // that correctly. <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64236>
+        //
+        //     template <::std::size_t... N>
+        //     class alignas(N...) alignmax {};
+        //
+        // Next, it could be this, but MSVC does not implement this correctly.
+        //
+        //     template <::std::size_t... N>
+        //     class alignmax { alignas(N...) union {} members; };
+        //
+        writeln!(out, "template <::std::size_t N>");
+        writeln!(out, "class alignas(N) aligned {{}};");
+        writeln!(out, "template <typename... T>");
+        writeln!(
+            out,
+            "class alignmax_t {{ alignas(T...) union {{}} members; }};",
+        );
+        writeln!(out, "template <::std::size_t... N>");
+        writeln!(out, "using alignmax = alignmax_t<aligned<N>...>;");
+        writeln!(out, "#endif // CXXBRIDGE_ALIGNMAX");
+    }
+
     out.end_block(Block::Namespace("repr"));
 
     out.begin_block(Block::Namespace("detail"));

gen/src/write.rs

@@ -11,8 +11,8 @@ use crate::syntax::set::UnorderedSet;
 use crate::syntax::symbol::{self, Symbol};
 use crate::syntax::trivial::{self, TrivialReason};
 use crate::syntax::{
-    derive, mangle, Alignment, Api, Doc, Enum, ExternFn, ExternType, FnKind, Lang, Pair, Signature,
-    Struct, Trait, Type, TypeAlias, Types, Var,
+    derive, mangle, Api, Doc, Enum, ExternFn, ExternType, FnKind, Lang, Pair, Signature, Struct,
+    Trait, Type, TypeAlias, Types, Var,
 };
 use proc_macro2::Ident;
 
@@ -280,12 +280,23 @@ fn write_struct<'a>(out: &mut OutFile<'a>, strct: &'a Struct, methods: &[&Extern
     writeln!(out, "#ifndef {}", guard);
     writeln!(out, "#define {}", guard);
     write_doc(out, "", &strct.doc);
-    let alignment = if let Some(Alignment::Align(x)) = strct.alignment {
-        format!("alignas({}) ", x)
-    } else {
-        String::from("")
-    };
-    writeln!(out, "struct {}{} final {{", alignment, strct.name.cxx);
+    write!(out, "struct");
+    if let Some(align) = &strct.align {
+        out.builtin.alignmax = true;
+        writeln!(out, " alignas(::rust::repr::alignmax<");
+        writeln!(out, "  {},", align.base10_parse::<u32>().unwrap());
+        for (i, field) in strct.fields.iter().enumerate() {
+            write!(out, "  alignof(");
+            write_type(out, &field.ty);
+            write!(out, ")");
+            if i + 1 != strct.fields.len() {
+                write!(out, ",");
+            }
+            writeln!(out);
+        }
+        write!(out, ">)");
+    }
+    writeln!(out, " {} final {{", strct.name.cxx);
 
     for field in &strct.fields {
         write_doc(out, "  ", &field.doc);

macro/src/expand.rs

@@ -7,12 +7,12 @@ use crate::syntax::qualified::QualifiedName;
 use crate::syntax::report::Errors;
 use crate::syntax::symbol::Symbol;
 use crate::syntax::{
-    self, check, mangle, Alignment, Api, Doc, Enum, ExternFn, ExternType, FnKind, Impl, Lang,
-    Lifetimes, Pair, Signature, Struct, Trait, Type, TypeAlias, Types,
+    self, check, mangle, Api, Doc, Enum, ExternFn, ExternType, FnKind, Impl, Lang, Lifetimes, Pair,
+    Signature, Struct, Trait, Type, TypeAlias, Types,
 };
 use crate::type_id::Crate;
 use crate::{derive, generics};
-use proc_macro2::{Ident, Literal, Span, TokenStream};
+use proc_macro2::{Ident, Span, TokenStream};
 use quote::{format_ident, quote, quote_spanned, ToTokens};
 use std::mem;
 use syn::{parse_quote, punctuated, Generics, Lifetime, Result, Token};
@@ -155,7 +155,6 @@ fn expand(ffi: Module, doc: Doc, attrs: OtherAttrs, apis: &[Api], types: &Types)
 fn expand_struct(strct: &Struct) -> TokenStream {
     let ident = &strct.name.rust;
     let doc = &strct.doc;
-    let alignment = &strct.alignment;
     let attrs = &strct.attrs;
     let generics = &strct.generics;
     let type_id = type_id(&strct.name);
@@ -179,20 +178,13 @@ fn expand_struct(strct: &Struct) -> TokenStream {
         }
     };
 
-    let mut repr = quote! { #[repr(C)] };
-    if let Some(Alignment::Align(x)) = alignment {
-        // Suffix isn't allowed in repr(align)
-        let x = Literal::u32_unsuffixed(*x);
-        repr = quote! {
-            #repr
-            #[repr(align(#x))]
-        }
-    }
+    let align = strct.align.as_ref().map(|align| quote!(, align(#align)));
+
     quote! {
         #doc
         #derives
         #attrs
-        #repr
+        #[repr(C #align)]
         #struct_def
 
         #[automatically_derived]

syntax/attrs.rs

@@ -78,7 +78,7 @@ pub(crate) fn parse(cx: &mut Errors, attrs: Vec<Attribute>, mut parser: Parser)
                 }
             }
         } else if attr_path.is_ident("repr") {
-            match attr.parse_args_with(parse_repr_attribute) {
+            match attr.parse_args::<Repr>() {
                 Ok(attr) => {
                     if let Some(repr) = &mut parser.repr {
                         **repr = Some(attr);
@@ -230,10 +230,6 @@ fn parse_derive_attribute(cx: &mut Errors, input: ParseStream) -> Result<Vec<Der
     Ok(derives)
 }
 
-fn parse_repr_attribute(input: ParseStream) -> Result<Repr> {
-    input.parse::<Repr>()
-}
-
 fn parse_cxx_name_attribute(meta: &Meta) -> Result<ForeignName> {
     if let Meta::NameValue(meta) = meta {
         match &meta.value {

syntax/mod.rs

@@ -51,10 +51,6 @@ pub(crate) use self::names::ForeignName;
 pub(crate) use self::parse::parse_items;
 pub(crate) use self::types::Types;
 
-pub enum Alignment {
-    Align(u32),
-}
-
 pub(crate) enum Api {
     #[allow(dead_code)] // only used by cxx-build, not cxxbridge-macro
     Include(Include),
@@ -113,7 +109,7 @@ pub(crate) struct Struct {
     pub cfg: CfgExpr,
     pub doc: Doc,
     pub derives: Vec<Derive>,
-    pub alignment: Option<Alignment>,
+    pub align: Option<LitInt>,
     #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build
     pub attrs: OtherAttrs,
     #[allow(dead_code)] // only used by cxxbridge-macro, not cxx-build

syntax/parse.rs

@@ -6,7 +6,7 @@ use crate::syntax::report::Errors;
 use crate::syntax::repr::Repr;
 use crate::syntax::Atom::*;
 use crate::syntax::{
-    attrs, error, Alignment, Api, Array, Derive, Doc, Enum, EnumRepr, ExternFn, ExternType, FnKind,
+    attrs, error, Api, Array, Derive, Doc, Enum, EnumRepr, ExternFn, ExternType, FnKind,
     ForeignName, Impl, Include, IncludeKind, Lang, Lifetimes, NamedType, Namespace, Pair, Ptr,
     Receiver, Ref, Signature, SliceRef, Struct, Ty1, Type, TypeAlias, Var, Variant,
 };
@@ -79,10 +79,13 @@ fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) ->
         },
     );
 
-    let alignment = if let Some(Repr::Align(x)) = repr {
-        Some(Alignment::Align(x))
-    } else {
-        None
+    let align = match repr {
+        Some(Repr::Align(align)) => Some(align),
+        Some(Repr::Atom(_atom, span)) => {
+            cx.push(Error::new(span, "unsupported alignment on a struct"));
+            None
+        }
+        None => None,
     };
 
     let named_fields = match item.fields {
@@ -186,7 +189,7 @@ fn parse_struct(cx: &mut Errors, mut item: ItemStruct, namespace: &Namespace) ->
         cfg,
         doc,
         derives,
-        alignment,
+        align,
         attrs,
         visibility,
         struct_token,
@@ -207,7 +210,7 @@ fn parse_enum(cx: &mut Errors, item: ItemEnum, namespace: &Namespace) -> Api {
     let mut rust_name = None;
     let attrs = attrs::parse(
         cx,
-        item.attrs.clone(),
+        item.attrs,
         attrs::Parser {
             cfg: Some(&mut cfg),
             doc: Some(&mut doc),
@@ -232,9 +235,9 @@ fn parse_enum(cx: &mut Errors, item: ItemEnum, namespace: &Namespace) -> Api {
     }
 
     let repr = match repr {
-        Some(Repr::Atom(atom)) => Some(atom),
-        Some(Repr::Align(_)) => {
-            cx.error(&item, "repr(align) on enums is not supported");
+        Some(Repr::Atom(atom, _span)) => Some(atom),
+        Some(Repr::Align(align)) => {
+            cx.error(align, "C++ does not support custom alignment on an enum");
             None
         }
         None => None,

syntax/repr.rs

@@ -1,11 +1,11 @@
 use crate::syntax::Atom::{self, *};
+use proc_macro2::{Ident, Span};
 use syn::parse::{Error, Parse, ParseStream, Result};
-use syn::{Ident, LitInt};
+use syn::{parenthesized, Expr, LitInt};
 
-#[derive(Copy, Clone, PartialEq)]
 pub(crate) enum Repr {
-    Align(u32),
-    Atom(Atom),
+    Align(LitInt),
+    Atom(Atom, Span),
 }
 
 impl Parse for Repr {
@@ -15,27 +15,35 @@ impl Parse for Repr {
         if let Some(atom) = Atom::from(&ident) {
             match atom {
                 U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 | Isize if input.is_empty() => {
-                    return Ok(Repr::Atom(atom));
+                    return Ok(Repr::Atom(atom, ident.span()));
                 }
                 _ => {}
             }
         } else if ident == "align" {
             let content;
-            syn::parenthesized!(content in input);
-            let alignment: u32 = content.parse::<LitInt>()?.base10_parse()?;
-            if !alignment.is_power_of_two() {
+            parenthesized!(content in input);
+            let align_expr: Expr = content.fork().parse()?;
+            if !matches!(align_expr, Expr::Lit(_)) {
                 return Err(Error::new_spanned(
-                    begin.token_stream(),
-                    "invalid `repr(align)` attribute: not a power of two",
+                    align_expr,
+                    "invalid repr(align) attribute: an arithmetic expression is not supported",
                 ));
             }
-            if alignment > 2u32.pow(29) {
+            let align_lit: LitInt = content.parse()?;
+            let align: u32 = align_lit.base10_parse()?;
+            if !align.is_power_of_two() {
                 return Err(Error::new_spanned(
-                    begin.token_stream(),
-                    "invalid `repr(align)` attribute: larger than 2^29",
+                    align_lit,
+                    "invalid repr(align) attribute: not a power of two",
                 ));
             }
-            return Ok(Repr::Align(alignment));
+            if align > 2u32.pow(13) {
+                return Err(Error::new_spanned(
+                    align_lit,
+                    "invalid repr(align) attribute: larger than 2^13",
+                ));
+            }
+            return Ok(Repr::Align(align_lit));
         }
         Err(Error::new_spanned(
             begin.token_stream(),

tests/ffi/lib.rs

@@ -88,7 +88,7 @@ pub mod ffi {
     }
 
     #[repr(align(4))]
-    pub struct StructWithAlignment4 {
+    pub struct OveralignedStruct {
         b: [u8; 4],
     }
 

tests/ffi/tests.cc

@@ -19,7 +19,7 @@ extern "C" bool cxx_test_suite_r_is_correct(const tests::R *) noexcept;
 
 namespace tests {
 
-static_assert(4 == alignof(StructWithAlignment4), "expected 4 byte alignment");
+static_assert(4 == alignof(OveralignedStruct), "expected 4 byte alignment");
 
 static constexpr char SLICE_DATA[] = "2020";