From cd2818b21b49f515b70c137c40e496040bda7dd7 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Tue, 19 Mar 2024 14:49:02 -0700 Subject: [PATCH] Update with review comments --- bindgen/codegen/mod.rs | 242 ++++++++++++++++++++++------------------- bindgen/ir/comp.rs | 7 +- 2 files changed, 133 insertions(+), 116 deletions(-) diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 29c69d7b38..ffd5b28b96 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -1675,7 +1675,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit { accessor_kind: FieldAccessorKind, parent: &CompInfo, parent_item: &Item, - _last_field: bool, + last_field: bool, result: &mut CodegenResult, struct_layout: &mut StructLayoutTracker, fields: &mut F, @@ -1757,7 +1757,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit { accessor_kind, parent, parent_item, - idx == bfields.len() - 1, + last_field && idx == bfields.len() - 1, result, struct_layout, fields, @@ -2549,116 +2549,13 @@ impl CodeGenerator for CompInfo { } if needs_flexarray_impl { - let prefix = ctx.trait_prefix(); - - let flex_array = - flex_inner_ty.as_ref().map(|ty| quote! { [ #ty ] }); - - let dst_ty_for_impl = quote! { - #canonical_ident < #( #generic_param_names , )* #flex_array > - - }; - let sized_ty_for_impl = quote! { - #canonical_ident < #( #generic_param_names , )* [ #flex_inner_ty; 0 ] > - }; - - let layout = if ctx.options().rust_features().layout_for_ptr { - quote! { - pub fn layout(len: usize) -> ::#prefix::alloc::Layout { - // SAFETY: Null pointers are OK if we don't deref them - unsafe { - let p: *const Self = ::#prefix::ptr::from_raw_parts(::#prefix::ptr::null(), len); - ::#prefix::alloc::Layout::for_value_raw(p) - } - } - } - } else { - quote!() - }; - - let (from_ptr_dst, from_ptr_sized) = if ctx - .options() - .rust_features() - .ptr_metadata - { - ( - quote! { - pub fn fixed(&self) -> (& #sized_ty_for_impl, usize) { - unsafe { - let (ptr, len) = (self as *const Self).to_raw_parts(); - (&*(ptr as *const #sized_ty_for_impl), len) - } - } - - pub fn fixed_mut(&mut self) -> (&mut #sized_ty_for_impl, usize) { - unsafe { - let (ptr, len) = (self as *mut Self).to_raw_parts(); - (&mut *(ptr as *mut #sized_ty_for_impl), len) - - } - } - }, - quote! { - /// Convert a sized prefix to an unsized structure with the given length. - /// - /// SAFETY: Underlying storage is initialized up to at least `len` elements. - pub unsafe fn flex_ref(&self, len: usize) -> &#dst_ty_for_impl { - // SAFETY: Reference is always valid as pointer. Caller is guaranteeing `len`. - unsafe { Self::flex_ptr(self, len) } - } - - /// Convert a mutable sized prefix to an unsized structure with the given length. - /// - /// SAFETY: Underlying storage is initialized up to at least `len` elements. - pub unsafe fn flex_mut_ref(&mut self, len: usize) -> &mut #dst_ty_for_impl { - // SAFETY: Reference is always valid as pointer. Caller is guaranteeing `len`. - unsafe { Self::flex_ptr_mut(self, len).assume_init() } - } - - /// Construct DST variant from a pointer and a size. - /// - /// NOTE: lifetime of returned reference is not tied to any underlying storage. - /// SAFETY: `ptr` is valid. Underlying storage is fully initialized up to at least `len` elements. - pub unsafe fn flex_ptr<'unbounded>(ptr: *const Self, len: usize) -> &'unbounded #dst_ty_for_impl { - unsafe { &*::#prefix::ptr::from_raw_parts(ptr as *const (), len) } - } - - /// Construct mutable DST variant from a pointer and a - /// size. The returned `&mut` reference is initialized - /// pointing to memory referenced by `ptr`, but there's - /// no requirement that that memory be initialized. - /// - /// NOTE: lifetime of returned reference is not tied to any underlying storage. - /// SAFETY: `ptr` is valid. Underlying storage has space for at least `len` elements. - pub unsafe fn flex_ptr_mut<'unbounded>( - ptr: *mut Self, - len: usize, - ) -> ::#prefix::mem::MaybeUninit<&'unbounded mut #dst_ty_for_impl> { - unsafe { - // Initialize reference without ever exposing it, as its possibly uninitialized - let mut uninit = ::#prefix::mem::MaybeUninit::<&mut #dst_ty_for_impl>::uninit(); - (uninit.as_mut_ptr() as *mut *mut #dst_ty_for_impl) - .write(::#prefix::ptr::from_raw_parts_mut(ptr as *mut (), len)); - - uninit - } - } - }, - ) - } else { - (quote!(), quote!()) - }; - - result.push(quote! { - impl #impl_generics_labels #dst_ty_for_impl { - #layout - #from_ptr_dst - } - - impl #impl_generics_labels #sized_ty_for_impl { - #from_ptr_sized - } - }); + result.push(self.generate_flexarray( + ctx, + &canonical_ident, + flex_inner_ty, + &*generic_param_names, + &impl_generics_labels, + )); } if needs_default_impl { @@ -2745,6 +2642,127 @@ impl CodeGenerator for CompInfo { } } +impl CompInfo { + fn generate_flexarray( + &self, + ctx: &BindgenContext, + canonical_ident: &Ident, + flex_inner_ty: Option, + generic_param_names: &[Ident], + impl_generics_labels: &proc_macro2::TokenStream, + ) -> proc_macro2::TokenStream { + let prefix = ctx.trait_prefix(); + + let flex_array = flex_inner_ty.as_ref().map(|ty| quote! { [ #ty ] }); + + let dst_ty_for_impl = quote! { + #canonical_ident < #( #generic_param_names , )* #flex_array > + + }; + let sized_ty_for_impl = quote! { + #canonical_ident < #( #generic_param_names , )* [ #flex_inner_ty; 0 ] > + }; + + let layout = if ctx.options().rust_features().layout_for_ptr { + quote! { + pub fn layout(len: usize) -> ::#prefix::alloc::Layout { + // SAFETY: Null pointers are OK if we don't deref them + unsafe { + let p: *const Self = ::#prefix::ptr::from_raw_parts(::#prefix::ptr::null(), len); + ::#prefix::alloc::Layout::for_value_raw(p) + } + } + } + } else { + quote!() + }; + + let (from_ptr_dst, from_ptr_sized) = if ctx + .options() + .rust_features() + .ptr_metadata + { + ( + quote! { + pub fn fixed(&self) -> (& #sized_ty_for_impl, usize) { + unsafe { + let (ptr, len) = (self as *const Self).to_raw_parts(); + (&*(ptr as *const #sized_ty_for_impl), len) + } + } + + pub fn fixed_mut(&mut self) -> (&mut #sized_ty_for_impl, usize) { + unsafe { + let (ptr, len) = (self as *mut Self).to_raw_parts(); + (&mut *(ptr as *mut #sized_ty_for_impl), len) + + } + } + }, + quote! { + /// Convert a sized prefix to an unsized structure with the given length. + /// + /// SAFETY: Underlying storage is initialized up to at least `len` elements. + pub unsafe fn flex_ref(&self, len: usize) -> &#dst_ty_for_impl { + // SAFETY: Reference is always valid as pointer. Caller is guaranteeing `len`. + unsafe { Self::flex_ptr(self, len) } + } + + /// Convert a mutable sized prefix to an unsized structure with the given length. + /// + /// SAFETY: Underlying storage is initialized up to at least `len` elements. + pub unsafe fn flex_mut_ref(&mut self, len: usize) -> &mut #dst_ty_for_impl { + // SAFETY: Reference is always valid as pointer. Caller is guaranteeing `len`. + unsafe { Self::flex_ptr_mut(self, len).assume_init() } + } + + /// Construct DST variant from a pointer and a size. + /// + /// NOTE: lifetime of returned reference is not tied to any underlying storage. + /// SAFETY: `ptr` is valid. Underlying storage is fully initialized up to at least `len` elements. + pub unsafe fn flex_ptr<'unbounded>(ptr: *const Self, len: usize) -> &'unbounded #dst_ty_for_impl { + unsafe { &*::#prefix::ptr::from_raw_parts(ptr as *const (), len) } + } + + /// Construct mutable DST variant from a pointer and a + /// size. The returned `&mut` reference is initialized + /// pointing to memory referenced by `ptr`, but there's + /// no requirement that that memory be initialized. + /// + /// NOTE: lifetime of returned reference is not tied to any underlying storage. + /// SAFETY: `ptr` is valid. Underlying storage has space for at least `len` elements. + pub unsafe fn flex_ptr_mut<'unbounded>( + ptr: *mut Self, + len: usize, + ) -> ::#prefix::mem::MaybeUninit<&'unbounded mut #dst_ty_for_impl> { + unsafe { + // Initialize reference without ever exposing it, as its possibly uninitialized + let mut uninit = ::#prefix::mem::MaybeUninit::<&mut #dst_ty_for_impl>::uninit(); + (uninit.as_mut_ptr() as *mut *mut #dst_ty_for_impl) + .write(::#prefix::ptr::from_raw_parts_mut(ptr as *mut (), len)); + + uninit + } + } + }, + ) + } else { + (quote!(), quote!()) + }; + + quote! { + impl #impl_generics_labels #dst_ty_for_impl { + #layout + #from_ptr_dst + } + + impl #impl_generics_labels #sized_ty_for_impl { + #from_ptr_sized + } + } + } +} + impl Method { fn codegen_method( &self, diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs index 5ae2d68747..f6c9e629a1 100644 --- a/bindgen/ir/comp.rs +++ b/bindgen/ir/comp.rs @@ -834,10 +834,9 @@ impl CompFields { CompFields::Error => return None, // panic? }; - // XXX correct with padding on end? - match fields.last() { - None | Some(Field::Bitfields(..)) => None, - Some(Field::DataMember(FieldData { ty, .. })) => ctx + match fields.last()? { + Field::Bitfields(..) => None, + Field::DataMember(FieldData { ty, .. }) => ctx .resolve_type(*ty) .is_incomplete_array(ctx) .map(|item| item.expect_type_id(ctx)),