Add a macro to derive ByteRepr automatically

Shrinks the translated code
Part of #216

Author: nunoplopes

cpp2rust/converter/converter.cpp

@@ -95,6 +95,7 @@ void Converter::EmitGlobalInits(Model model, std::string &out) {
 void Converter::EmitOpaqueRecords(std::string &out) {
   record_decls_.ForEachUndefined([&](const std::string &name) {
     out += "#[derive(Clone, Copy, Default, ByteRepr, VaArg, FnPtrArg)]";
+    out += "#[byte_size(1)]";
     out += "pub struct ";
     out += name;
     out += ";\n";
@@ -812,6 +813,7 @@ void Converter::EmitRustStructOrUnion(clang::RecordDecl *decl) {
     StrCat(attr, ',');
   }
   StrCat(")]");
+  EmitByteSizeAttr(decl);
 
   // Fields
   auto access = clang::dyn_cast<clang::CXXRecordDecl>(decl)
@@ -837,7 +839,6 @@ void Converter::EmitRustStructOrUnion(clang::RecordDecl *decl) {
   }
   AddCloneTrait(decl);
   AddDefaultTrait(decl);
-  AddByteReprTrait(decl);
 }
 
 void Converter::ConvertLateInstantiatedMethods(clang::CXXRecordDecl *decl) {
@@ -925,7 +926,6 @@ void Converter::EmitRustUnion(clang::RecordDecl *decl) {
   }
 
   AddDefaultTrait(decl);
-  AddByteReprTrait(decl);
 }
 
 bool Converter::VisitCXXRecordDecl(clang::CXXRecordDecl *decl) {
@@ -4675,7 +4675,7 @@ void Converter::ConvertFieldInit(const clang::FieldDecl *field,
   }
 }
 
-void Converter::AddByteReprTrait(const clang::RecordDecl *decl) {}
+void Converter::EmitByteSizeAttr(const clang::RecordDecl *decl) {}
 
 void Converter::ConvertUnsignedArithBinaryOperator(clang::BinaryOperator *op,
                                                    clang::Expr *expr) {

cpp2rust/converter/converter.h

@@ -673,7 +673,7 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
 
   void EmitDefaultStructLiteral(const clang::RecordDecl *decl);
 
-  virtual void AddByteReprTrait(const clang::RecordDecl *decl);
+  virtual void EmitByteSizeAttr(const clang::RecordDecl *decl);
 
   virtual void
   ConvertUnsignedArithBinaryOperator(clang::BinaryOperator *binary_operator,

cpp2rust/converter/converter_lib.cpp

@@ -272,33 +272,6 @@ bool IsMut(clang::QualType qual_type) {
            qual_type->getPointeeType().isConstQualified());
 }
 
-bool TypeImplementsByteRepr(clang::QualType qt) {
-  if (qt->isIntegerType() || qt->isFloatingType() || qt->isEnumeralType()) {
-    return true;
-  }
-  if (qt->isPointerType()) {
-    return true;
-  }
-  if (const auto *arr = qt->getAsArrayTypeUnsafe()) {
-    return TypeImplementsByteRepr(arr->getElementType());
-  }
-  if (const auto *rd = qt->getAsRecordDecl()) {
-    if (rd->isUnion()) {
-      return true;
-    }
-    if (!qt.isTriviallyCopyableType(rd->getASTContext())) {
-      return false;
-    }
-    for (const auto *field : rd->fields()) {
-      if (!TypeImplementsByteRepr(field->getType())) {
-        return false;
-      }
-    }
-    return true;
-  }
-  return false;
-}
-
 bool RustSizeDivergesFromC(clang::QualType qt) {
   qt = qt.getCanonicalType();
   // Records have Rc<RefCell<>> fields that diverge from the C size
@@ -484,10 +457,6 @@ bool HasDefaultedCopyConstructor(const clang::RecordDecl *decl) {
   return !cxx->defaultedCopyConstructorIsDeleted();
 }
 
-bool RecordDerivesByteRepr(const clang::RecordDecl *decl) {
-  return !decl->isUnion() && decl->field_empty();
-}
-
 bool RecordHasOnlyReferenceFields(const clang::RecordDecl *decl) {
   for (auto *field : decl->fields()) {
     if (!field->getType()->isReferenceType()) {

cpp2rust/converter/converter_lib.h

@@ -67,8 +67,6 @@ bool IsUnsignedArithOp(const clang::BinaryOperator *expr);
 
 bool IsMut(clang::QualType qual_type);
 
-bool TypeImplementsByteRepr(clang::QualType qt);
-
 bool RustSizeDivergesFromC(clang::QualType qt);
 
 bool IsMutatingCall(const clang::CallExpr *expr);
@@ -109,8 +107,6 @@ bool HasDefaultedCopyConstructor(const clang::RecordDecl *decl);
 
 bool RecordHasOnlyReferenceFields(const clang::RecordDecl *decl);
 
-bool RecordDerivesByteRepr(const clang::RecordDecl *decl);
-
 bool HasDefaultedCopyAssignment(const clang::RecordDecl *decl);
 
 bool IsRValueConvertingConstructor(const clang::CXXConstructorDecl *ctor);

cpp2rust/converter/models/converter_refcount.cpp

@@ -626,7 +626,11 @@ void ConverterRefCount::EmitRustUnion(clang::RecordDecl *decl) {
       ctx_, ctx_.getCanonicalTagType(decl),
       std::vector<std::string>(attrs.begin(), attrs.end()));
 
-  StrCat(std::format("pub struct {} {{ __bytes: Value<Box<[u8]>> }}", name));
+  auto size = ctx_.getTypeSizeInChars(ctx_.getCanonicalTagType(decl));
+  StrCat(std::format("#[derive(ByteRepr)] #[byte_size({0})] pub struct {1} {{ "
+                     "#[offset(0)] #[byte_size({0})] __bytes: Value<Box<[u8]>> "
+                     "}}",
+                     size.getQuantity(), name));
 
   StrCat("impl", name);
   {
@@ -647,84 +651,12 @@ void ConverterRefCount::EmitRustUnion(clang::RecordDecl *decl) {
 
   AddCloneTrait(decl);
   AddDefaultTrait(decl);
-  AddByteReprTrait(decl);
 }
 
-void ConverterRefCount::AddByteReprTrait(const clang::RecordDecl *decl) {
-  if (RecordDerivesByteRepr(decl)) {
-    return;
-  }
-
-  auto struct_name = GetRecordName(decl);
-
-  if (!TypeImplementsByteRepr(ctx_.getCanonicalTagType(decl))) {
-    // The size locates the fields of the elements of arrays of structs.
-    StrCat(std::format("impl ByteRepr for {}", struct_name));
-    PushBrace brace(*this);
-    if (!decl->isUnion()) {
-      StrCat(std::format("fn byte_size() -> usize {{ {} }}",
-                         ctx_.getTypeSize(ctx_.getCanonicalTagType(decl)) / 8));
-    }
-    return;
-  }
-
-  StrCat("impl ByteRepr for ", struct_name);
-  PushBrace impl_brace(*this);
-
-  if (decl->isUnion()) {
-    StrCat(std::format("fn byte_size() -> usize {{ {} }}",
-                       ctx_.getTypeSize(ctx_.getCanonicalTagType(decl)) / 8));
-    StrCat("fn to_bytes(&self, buf: &mut [u8]) { "
-           "buf.copy_from_slice(&self.__bytes.borrow()); }");
-    StrCat(std::format("fn from_bytes(buf: &[u8]) -> Self {{ {} {{ __bytes: "
-                       "Rc::new(RefCell::new(Box::from(buf))) }} }}",
-                       struct_name));
-    return;
-  }
-
-  const auto &layout = ctx_.getASTRecordLayout(decl);
-
-  StrCat(std::format("fn byte_size() -> usize {{ {} }}",
-                     ctx_.getTypeSize(ctx_.getCanonicalTagType(decl)) / 8));
-
-  StrCat("fn to_bytes(&self, buf: &mut [u8])");
-  {
-    PushBrace fn_brace(*this);
-    unsigned idx = 0;
-    for (auto *field : decl->fields()) {
-      auto byte_off = layout.getFieldOffset(idx) / 8;
-      auto byte_size = ctx_.getTypeSize(field->getType()) / 8;
-      auto value = "self." + GetNamedDeclAsString(field);
-      if (IsValueField(ctx_, field)) {
-        value = std::format("(*{}.borrow())", value);
-      }
-      StrCat(std::format("{}.to_bytes(&mut buf[{}..{}]);", value, byte_off,
-                         byte_off + byte_size));
-      ++idx;
-    }
-  }
-
-  StrCat("fn from_bytes(buf: &[u8]) -> Self");
-  {
-    PushBrace fn_brace(*this);
-    StrCat("Self");
-    PushBrace lit_brace(*this);
-    unsigned idx = 0;
-    for (auto *field : decl->fields()) {
-      auto byte_off = layout.getFieldOffset(idx) / 8;
-      auto byte_size = ctx_.getTypeSize(field->getType()) / 8;
-      PushConversionKind push(*this, ConversionKind::FullRefCount);
-      std::string storage_ty = ToString(field->getType());
-      Unwrap(storage_ty, "Value<", ">");
-      auto value = std::format("<{}>::from_bytes(&buf[{}..{}])", storage_ty,
-                               byte_off, byte_off + byte_size);
-      if (IsValueField(ctx_, field)) {
-        value = std::format("Rc::new(RefCell::new({}))", value);
-      }
-      StrCat(std::format("{}: {},", GetNamedDeclAsString(field), value));
-      ++idx;
-    }
-  }
+void ConverterRefCount::EmitByteSizeAttr(const clang::RecordDecl *decl) {
+  StrCat(std::format(
+      "#[byte_size({})]",
+      ctx_.getTypeSizeInChars(ctx_.getCanonicalTagType(decl)).getQuantity()));
 }
 
 std::string
@@ -745,6 +677,14 @@ bool ConverterRefCount::VisitFieldDecl(clang::FieldDecl *decl) {
   const auto &layout = ctx_.getASTRecordLayout(decl->getParent());
   StrCat(std::format("#[offset({})]",
                      layout.getFieldOffset(decl->getFieldIndex()) / 8));
+  // Only arithmetic types have the same size in Rust as in C.
+  auto type = decl->getType().getCanonicalType();
+  if (!type->isIntegerType() &&
+      !type->isSpecificBuiltinType(clang::BuiltinType::Float) &&
+      !type->isSpecificBuiltinType(clang::BuiltinType::Double)) {
+    StrCat(std::format("#[byte_size({})]",
+                       ctx_.getTypeSizeInChars(decl->getType()).getQuantity()));
+  }
   PushConversionKind push(*this, IsValueField(ctx_, decl)
                                      ? ConversionKind::FullRefCount
                                      : ConversionKind::Pointee);
@@ -2356,10 +2296,8 @@ ConverterRefCount::GetStructAttributes(const clang::RecordDecl *decl) {
 
   // Gives access to the fields through pointers to them.
   attrs.emplace_back("Record");
-
-  if (RecordDerivesByteRepr(decl)) {
-    attrs.emplace_back("ByteRepr");
-  }
+  // Gives access to the bytes of the struct, at the offsets of its fields.
+  attrs.emplace_back("ByteRepr");
 
   if (RecordImplementsClone(decl)) {
     attrs.emplace_back("VaArg");

cpp2rust/converter/models/converter_refcount.h

@@ -53,7 +53,7 @@ class ConverterRefCount final : public Converter {
 
   bool RecordDerivesClone(const clang::RecordDecl *decl);
 
-  void AddByteReprTrait(const clang::RecordDecl *decl) override;
+  void EmitByteSizeAttr(const clang::RecordDecl *decl) override;
 
   bool
   VisitUnaryExprOrTypeTraitExpr(clang::UnaryExprOrTypeTraitExpr *expr) override;

libcc2rs-macros/src/byte_repr.rs

@@ -2,28 +2,67 @@
 // Distributed under the MIT license that can be found in the LICENSE file.
 
 use proc_macro::TokenStream;
-use syn::{Data, DeriveInput, Fields, parse_macro_input};
+use quote::quote;
+use syn::{Attribute, Data, DeriveInput, Expr, Fields, parse_macro_input};
+
+fn byte_size(attrs: &[Attribute]) -> Option<Expr> {
+    attrs
+        .iter()
+        .find(|attr| attr.path().is_ident("byte_size"))
+        .map(|attr| attr.parse_args().unwrap())
+}
 
 pub fn expand(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DeriveInput);
     let name = &input.ident;
-    let empty = match &input.data {
-        Data::Struct(s) => match &s.fields {
-            Fields::Unit => true,
-            Fields::Named(f) => f.named.is_empty(),
-            Fields::Unnamed(f) => f.unnamed.is_empty(),
-        },
-        _ => false,
+    let Data::Struct(data) = &input.data else {
+        panic!("derive(ByteRepr) is only supported on structs, `{name}` is not one");
     };
-    if !empty {
-        panic!("derive(ByteRepr) is only supported on empty structs, `{name}` is not empty");
-    }
+    let fields = match &data.fields {
+        Fields::Named(fields) => fields.named.iter().collect(),
+        Fields::Unit => Vec::new(),
+        Fields::Unnamed(_) => {
+            panic!("derive(ByteRepr) is not supported on tuple structs, `{name}` is one")
+        }
+    };
+
+    let size = byte_size(&input.attrs)
+        .unwrap_or_else(|| panic!("struct `{name}` has no #[byte_size(...)]"));
+
+    // The bytes of each field are at its offset, and span the byte size of
+    // its type, unless given by its byte_size attribute.
+    let ranges: Vec<_> = fields
+        .iter()
+        .map(|field| {
+            let ty = &field.ty;
+            let offset = crate::record::offset(name, field);
+            let size = byte_size(&field.attrs)
+                .map(|size| quote! { #size })
+                .unwrap_or_else(|| quote! { <#ty as ::libcc2rs::ByteRepr>::byte_size() });
+            quote! { (#offset)..(#offset) + #size }
+        })
+        .collect();
+    let idents: Vec<_> = fields.iter().map(|field| &field.ident).collect();
+    let types: Vec<_> = fields.iter().map(|field| &field.ty).collect();
+
     let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
-    quote::quote! {
+    quote! {
         impl #impl_generics ::libcc2rs::ByteRepr for #name #ty_generics #where_clause {
             #[inline]
             fn byte_size() -> usize {
-                1
+                #size
+            }
+
+            #[allow(unused_variables)]
+            fn to_bytes(&self, buf: &mut [u8]) {
+                #(::libcc2rs::ByteRepr::to_bytes(&self.#idents, &mut buf[#ranges]);)*
+            }
+
+            #[allow(unused_variables)]
+            fn from_bytes(buf: &[u8]) -> Self {
+                Self {
+                    #(#idents: <#types as ::libcc2rs::ByteRepr>::from_bytes(&buf[#ranges]),)*
+                }
             }
         }
     }

libcc2rs-macros/src/lib.rs

@@ -85,12 +85,22 @@ pub fn goto(_input: TokenStream) -> TokenStream {
 }
 
 //     #[derive(ByteRepr)]
-//     pub struct S;
+//     #[byte_size(16)]
+//     pub struct S {
+//         #[offset(0)]
+//         pub x: i32,
+//         #[offset(4)]
+//         #[byte_size(12)]
+//         pub a: Value<Box<[i32]>>,
+//         ...
+//     }
 //
-// Adds ByteRepr implementation for S. Currently only empty structs are handled. Non-empty structs
-// panic.
+// Implements libcc2rs::ByteRepr for S, laying out each field at the byte
+// offset given by its offset attribute (see derive(Record)). The byte_size
+// attribute gives the byte size of S in C, and of the fields whose size in C
+// differs from the byte_size() of their type (e.g., arrays and pointers).
 
-#[proc_macro_derive(ByteRepr)]
+#[proc_macro_derive(ByteRepr, attributes(offset, byte_size))]
 pub fn derive_byte_repr(input: TokenStream) -> TokenStream {
     byte_repr::expand(input)
 }

libcc2rs-macros/src/record.rs

@@ -3,7 +3,19 @@
 
 use proc_macro::TokenStream;
 use quote::{format_ident, quote};
-use syn::{Data, DeriveInput, Expr, Fields, parse_macro_input};
+use syn::{Data, DeriveInput, Expr, Field, Fields, Ident, parse_macro_input};
+
+// The byte offset of `field` of struct `name`, given by its offset attribute.
+pub fn offset(name: &Ident, field: &Field) -> Expr {
+    let ident = field.ident.as_ref().unwrap();
+    field
+        .attrs
+        .iter()
+        .find(|attr| attr.path().is_ident("offset"))
+        .unwrap_or_else(|| panic!("field `{name}::{ident}` has no #[offset(...)]"))
+        .parse_args()
+        .unwrap()
+}
 
 pub fn expand(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DeriveInput);
@@ -16,17 +28,7 @@ pub fn expand(input: TokenStream) -> TokenStream {
         Fields::Named(fields) => fields
             .named
             .iter()
-            .map(|field| {
-                let ident = field.ident.clone().unwrap();
-                let offset: Expr = field
-                    .attrs
-                    .iter()
-                    .find(|attr| attr.path().is_ident("offset"))
-                    .unwrap_or_else(|| panic!("field `{name}::{ident}` has no #[offset(...)]"))
-                    .parse_args()
-                    .unwrap();
-                (ident, offset)
-            })
+            .map(|field| (field.ident.clone().unwrap(), offset(name, field)))
             .collect(),
         _ => Vec::new(),
     };