diff --git a/hdf5-derive/src/lib.rs b/hdf5-derive/src/lib.rs index 2f73fb0f..6028fc14 100644 --- a/hdf5-derive/src/lib.rs +++ b/hdf5-derive/src/lib.rs @@ -17,7 +17,8 @@ pub fn derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as DeriveInput); let name = input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let body = impl_trait(&name, &input.data, &input.attrs, &ty_generics); + let type_descriptor_body = impl_type_descriptor(&name, &input.data, &input.attrs, &ty_generics); + let init_skipped_fields_body = impl_initialize_skipped_fields(&input.data); // Determine name of parent crate, even if renamed using "package" // CARGO_CRATE_NAME is the name of the actual crate being compiled (e.g., "simple" for examples) @@ -50,7 +51,11 @@ pub fn derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream { unsafe impl #impl_generics _h5::types::H5Type for #name #ty_generics #where_clause { #[inline] fn type_descriptor() -> _h5::types::TypeDescriptor { - #body + #type_descriptor_body + } + #[inline] + unsafe fn init_skipped_fields(element: &mut ::std::mem::MaybeUninit) { + #init_skipped_fields_body } } }; @@ -127,6 +132,22 @@ fn is_phantom_data(ty: &Type) -> bool { } } +fn is_hdf5_skip(attrs: &[Attribute]) -> bool { + let mut skip = false; + let attr = match attrs.iter().find(|a| a.path().is_ident("hdf5")) { + Some(a) => a, + None => return false, + }; + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("skip") { + skip = true; + } + Ok(()) + }) + .ok(); + skip +} + fn find_repr(attrs: &[Attribute], expected: &[&str]) -> Option { let mut repr = None; for attr in attrs.iter() { @@ -173,7 +194,7 @@ where iter.map(func).collect() } -fn impl_trait( +fn impl_type_descriptor( ty: &Ident, data: &Data, attrs: &[Attribute], ty_generics: &TypeGenerics, ) -> TokenStream { match *data { @@ -181,8 +202,11 @@ fn impl_trait( Fields::Unit => syn::Error::new(ty.span(), "cannot derive `H5Type` for unit structs") .into_compile_error(), Fields::Named(ref fields) => { - let fields: Vec<_> = - fields.named.iter().filter(|f| !is_phantom_data(&f.ty)).collect(); + let fields: Vec<_> = fields + .named + .iter() + .filter(|f| !is_phantom_data(&f.ty) && !is_hdf5_skip(&f.attrs)) + .collect(); if fields.is_empty() { return syn::Error::new(ty.span(), "cannot derive `H5Type` for empty structs") .into_compile_error(); @@ -214,7 +238,7 @@ fn impl_trait( .unnamed .iter() .enumerate() - .filter(|&(_, f)| !is_phantom_data(&f.ty)) + .filter(|&(_, f)| !is_phantom_data(&f.ty) && !is_hdf5_skip(&f.attrs)) .map(|(i, f)| (Index::from(i), f)) .unzip(); if fields.is_empty() { @@ -285,3 +309,51 @@ fn impl_trait( .to_compile_error(), } } + +fn impl_initialize_skipped_fields(data: &Data) -> TokenStream { + match *data { + Data::Struct(ref data) => match data.fields { + Fields::Named(ref fields) => { + let skipped_fields = fields + .named + .iter() + .filter(|field| is_hdf5_skip(&field.attrs)) + .filter_map(|field| field.ident.as_ref()) + .map(|field| quote!(#field)) + .collect::>(); + impl_initialize_skipped_fields_body(&skipped_fields) + } + Fields::Unnamed(ref fields) => { + let skipped_fields = fields + .unnamed + .iter() + .enumerate() + .filter(|&(_, field)| is_hdf5_skip(&field.attrs)) + .map(|(index, _)| Index::from(index)) + .map(|index| quote!(#index)) + .collect::>(); + impl_initialize_skipped_fields_body(&skipped_fields) + } + Fields::Unit => quote! {}, + }, + _ => quote! {}, + } +} + +fn impl_initialize_skipped_fields_body(fields: &[TokenStream]) -> TokenStream { + if fields.is_empty() { + return quote! {}; + } + + let fields = fields.iter(); + quote! { + let ptr = element.as_mut_ptr(); + + #( + ::std::ptr::write_unaligned( + ::std::ptr::addr_of_mut!((*ptr).#fields), + ::core::default::Default::default(), + ); + )* + } +} diff --git a/hdf5-derive/tests/test.rs b/hdf5-derive/tests/test.rs index dbb6555c..122cae0c 100644 --- a/hdf5-derive/tests/test.rs +++ b/hdf5-derive/tests/test.rs @@ -250,3 +250,89 @@ fn test_phantom_data() { assert_eq!(G3::::type_descriptor(), C3::type_descriptor()); assert_eq!(G4::::type_descriptor(), C4::type_descriptor()); } + +#[cfg(test)] +mod test_hdf5_skip_attribute { + use super::*; + macro_rules! check_fields { + ($ty:ty, $($field_name:expr => $field:tt),+ $(,)?) => {{ + let desc = <$ty as hdf5::types::H5Type>::type_descriptor(); + assert_eq!(desc.size(), std::mem::size_of::<$ty>(), "Total size mismatch"); + + let hdf5::types::TypeDescriptor::Compound(compound) = desc else { + panic!("Expected TypeDescriptor::Compound"); + }; + + let s = std::mem::MaybeUninit::<$ty>::uninit(); + let s_ptr = s.as_ptr(); + let mut expected_field_count = 0; + + $( + expected_field_count += 1; + let found = compound.fields.iter().find(|f| f.name == $field_name) + .unwrap_or_else(|| panic!("Field '{}' not found in HDF5 descriptor", $field_name)); + + let expected_offset = unsafe { + std::ptr::addr_of!((*s_ptr).$field) as usize - s_ptr as usize + }; + + assert_eq!( + found.offset, expected_offset, + "Offset mismatch for field '{}': HDF5 says {}, memory says {}", + $field_name, found.offset, expected_offset + ); + )+ + + assert_eq!( + compound.fields.len(), expected_field_count, + "Expected exactly {} unskipped fields", expected_field_count + ); + }}; + } + #[test] + fn test_skip_repr_c() { + #[derive(H5Type)] + #[repr(C)] + struct ReprCStruct { + a: u8, + #[hdf5(skip)] + _skipped_1: u32, + b: u64, + #[hdf5(skip)] + _skipped_2: Vec, + c: u16, + } + check_fields!(ReprCStruct, "a" => a, "b" => b, "c" => c); + } + #[test] + fn test_skip_repr_packed() { + #[derive(H5Type)] + #[repr(packed)] + struct PackedStruct { + a: u8, + #[hdf5(skip)] + _skipped: u64, + b: u32, + } + check_fields!(PackedStruct, "a" => a, "b" => b); + } + #[test] + fn test_skip_tuple_struct() { + #[derive(H5Type)] + #[repr(C)] + struct TupleStruct(u16, #[hdf5(skip)] String, u64); + check_fields!(TupleStruct, "0" => 0, "1" => 2); + } + #[test] + fn test_skip_generics() { + #[derive(H5Type)] + #[repr(C)] + struct GenericStruct { + x: T, + #[hdf5(skip)] + _skipped: Vec, + y: U, + } + check_fields!(GenericStruct, "x" => x, "y" => y); + } +} diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index 00aa2f1b..461a69b7 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -1,5 +1,5 @@ use std::fmt::{self, Display}; -use std::mem; +use std::mem::{self, MaybeUninit}; use std::os::raw::c_void; use crate::array::VarLenArray; @@ -302,6 +302,17 @@ impl TypeDescriptor { pub unsafe trait H5Type: 'static { /// Returns a descriptor for an equivalent HDF5 datatype. fn type_descriptor() -> TypeDescriptor; + /** + Structs with `#[hdf5(skip)]` fields need to have them initialized because they are not present in the HDF5 file. + The derive macro implementation initializes skipped fields + using the Default trait. This function is called for each element of a read buffer before reading the data from the HDF5 file, it partially initialize the struct by only initializing the skipped fields. + */ + #[allow(unused_variables)] + unsafe fn init_skipped_fields(element: &mut MaybeUninit) + where + Self: Sized, + { + } } macro_rules! impl_h5type { diff --git a/hdf5/src/hl/container.rs b/hdf5/src/hl/container.rs index 9b68d55e..07692bab 100644 --- a/hdf5/src/hl/container.rs +++ b/hdf5/src/hl/container.rs @@ -1,7 +1,7 @@ use std::convert::TryInto; use std::fmt::{self, Debug}; use std::io; -use std::mem; +use std::mem::{self, MaybeUninit}; use std::ops::Deref; use ndarray::{Array, Array1, Array2, ArrayD, ArrayView, ArrayView1}; @@ -46,7 +46,17 @@ impl<'a> Reader<'a> { let mem_dtype = Datatype::from_type::()?; file_dtype.ensure_convertible(&mem_dtype, self.conv)?; let (obj_id, tp_id) = (self.obj.id(), mem_dtype.id()); - + let n_elements = match mspace { + Some(space) => space.size(), + None => self.obj.space()?.size(), + }; + let uninit_buf = buf.cast::<::std::mem::MaybeUninit>(); + for i in 0..n_elements { + unsafe { + let uninit_ref: &mut MaybeUninit = &mut *(uninit_buf.add(i)); + T::init_skipped_fields(uninit_ref); + } + } if self.obj.is_attr() { h5try!(H5Aread(obj_id, tp_id, buf.cast())); } else { diff --git a/hdf5/tests/test_dataset.rs b/hdf5/tests/test_dataset.rs index a775dadf..baf95479 100644 --- a/hdf5/tests/test_dataset.rs +++ b/hdf5/tests/test_dataset.rs @@ -480,3 +480,118 @@ fn remove_attr() { ds.delete_attr("bar").unwrap(); assert!(ds.attr("bar").is_err()); } +mod test_reading_and_written_skipped_structs { + use super::*; + #[derive(hdf5_derive::H5Type)] + #[repr(C)] + struct PlainStruct { + id: u32, + value: u8, + } + #[derive(hdf5_derive::H5Type)] + #[repr(C)] + struct NamedSkippedStruct { + id: u32, + #[hdf5(skip)] + skipped: String, + value: u8, + } + #[derive(hdf5_derive::H5Type)] + #[repr(C)] + struct TupleSkippedStruct(u32, #[hdf5(skip)] Vec, u8); + + #[test] + fn skipped_and_plain_structs_deserialize_the_same() { + let written = vec![ + NamedSkippedStruct { id: 1, skipped: String::from("Test1"), value: 10 }, + NamedSkippedStruct { id: 2, skipped: String::from("Test2"), value: 20 }, + ]; + + let dataset = new_in_memory_file() + .unwrap() + .new_dataset::() + .shape((written.len(),)) + .create("named") + .unwrap(); + + dataset.write_raw(&written).unwrap(); + + let plain = dataset.read_raw::().unwrap(); + let skipped = dataset.read_raw::().unwrap(); + + assert_eq!(plain.len(), skipped.len()); + for (p, s) in plain.iter().zip(skipped.iter()) { + assert_eq!(p.id, s.id); + assert_eq!(p.value, s.value); + assert_eq!(s.skipped, String::default()); + } + } + + #[test] + fn named_struct_defaults_skipped_field() { + let written = vec![ + NamedSkippedStruct { id: 1, skipped: String::from("first"), value: 10 }, + NamedSkippedStruct { id: 2, skipped: String::from("second"), value: 20 }, + ]; + + let dataset = new_in_memory_file() + .unwrap() + .new_dataset::() + .shape((written.len(),)) + .create("named") + .unwrap(); + + dataset.write_raw(&written).unwrap(); + + let read = dataset.read_raw::().unwrap(); + + assert_eq!(read.len(), 2); + assert_eq!(read[0].id, 1); + assert_eq!(read[0].value, 10); + assert_eq!(read[0].skipped, String::default()); + assert_eq!(read[1].id, 2); + assert_eq!(read[1].value, 20); + assert_eq!(read[1].skipped, String::default()); + } + + #[test] + fn tuple_struct_defaults_skipped_field() { + let written = vec![ + TupleSkippedStruct(11, vec![1, 2, 3], 9), + TupleSkippedStruct(21, vec![4, 5, 6], 19), + ]; + + let dataset = new_in_memory_file() + .unwrap() + .new_dataset::() + .shape((written.len(),)) + .create("tuple") + .unwrap(); + + dataset.write_raw(&written).unwrap(); + + let read = dataset.read_raw::().unwrap(); + + assert_eq!(read.len(), 2); + assert_eq!(read[0].0, 11); + assert_eq!(read[0].2, 9); + assert_eq!(read[0].1, Vec::::default()); + assert_eq!(read[1].0, 21); + assert_eq!(read[1].2, 19); + assert_eq!(read[1].1, Vec::::default()); + } + + #[test] + fn attribute_defaults_skipped_field() { + let written = NamedSkippedStruct { id: 99, skipped: String::from("meta"), value: 5 }; + + let file = new_in_memory_file().unwrap(); + let attr = file.new_attr::().shape(()).create("my_attr").unwrap(); + + attr.write_scalar(&written).unwrap(); + let read = attr.read_scalar::().unwrap(); + assert_eq!(read.id, 99); + assert_eq!(read.value, 5); + assert_eq!(read.skipped, String::default()); + } +}