feat: added skip attribute to hdf5-derive#185
Conversation
|
I struggle in understanding how we can hdf5::read a type like this. The bytes returned will be uninit and unsafe |
|
You are absolutely right! I was so focused on the serialization side of things that I completely forgot the deserialization. I think the most ergonomic and safe behavior is to require that any field marked with |
|
The serialization should be fine in theory, I have not examined the details though. Default is not going to be enough since we start from mem::uninitialized, don't think a Zeroded trait will do either. If you want this feature then we could add new traits for serialization and deserialization and make H5Type be the combination of those two |
|
I think splitting this into something like By doing this, we avoid the undefined behavior issue and there is no need for any unnecessary zeroing or default initialization of the C HDF5 read buffer. It mimics Serde's asymmetric model but without the intermediate data representation. I'd be happy to help if you provide me some direction, i used the HDF5 library in C before but i'm getting familiar with the code base |
|
Such a change would likely involve breaking changes in most of the crate(s) and I can't see any other usecases than the one you suggest. I am not confident that it will be worth the extra work in maintaining such a solution, unfortunately |
|
I understand, since you'll be maintaining this crate long-term, it's not a great idea to introduce massive breaking changes. What if we could achieve this without breaking the current API? Looking quickly at the Let me know if a non-breaking approach like that sounds great, otherwise I am happy to leave it as is and thank you for your time |
|
I have some great news! After a few tweaks, I've added tests for reading and writing skipped structs, and is working exactly as expected. The skipped fields are now safely initialized using the To not break backward compatibility, I added a new method to the Because this has an empty default implementation, existing types incur zero runtime overhead and require absolutely no code changes. I search for all instances of Finally, when the derive macro finds quote! {
for i in 0..size {
let ptr = ptr.add(i);
#(
::std::ptr::write_unaligned(
::std::ptr::addr_of_mut!((*ptr).#fields),
::core::default::Default::default(),
);
)*
}
} |
|
I updated the function signature in the unsafe fn initialize_skipped_fields(_ptr: *mut Self, _size: usize) {}to: unsafe fn init_skipped_fields(element: &mut MaybeUninit<Self>)I believe these semantics are clearer, it explicitly says that it partially initializes the skipped fields in a block of memory that may be uninitialized. The read_buf function now calls this N times across the entire buffer. I was initially concerned that the loop in read_buf would introduce overhead for structs without skipped fields. However, after verifying with cargo-show-asm, I confirmed that the loop it is optimized away in the LLVM IR. Playing around with Compiler Explorer i think when the loop has no observable side effects, it's simple dead code elimination https://godbolt.org/z/WhnrYojh1 I also added a test to assert that when a struct with skipped fields is serialized, it can successfully be deserialized into a struct without the skipped fields (which i think it's the most common usage). |
Description
First, thank you for maintaining this amazing crate! I used
hdf5-metnofor my final graduation project, and it was incredibly helpful. While working on my project, one feature I found myself missing was the ability to explicitly skip fields of the producedH5Type, similar to how#[serde(skip)]works in Serde.This PR introduces the
#[hdf5(skip)]attribute to thehdf5-derivecrate. This allows users to ignore specific fields (like large vectors, strings, or irrelevant internal state) when writing a struct to an HDF5 file.Usage Example
Implementation Details
The implementation itself is relatively straightforward. I added a new
is_hdf5_skiphelper function in the derive macro that parses the field attributes (using parse_nested_meta). If#[hdf5(skip)]is found, the field is simply filtered out before the HDF5 TypeDescriptor is generated. In the code, it works exactly like the is_phantom_data function.Tests added
As I described before, the behavior should be the same as a PhantomData field, but to actually assert that, I added the
test_hdf5_skip_attributemodule.The test suite uses a custom check_fields! macro. For every test case, it allocates an uninitialized struct via std::mem::MaybeUninit, uses raw pointers to calculate the exact physical byte offset of each field in memory, and asserts that the Rust compiler's memory layout perfectly matches the offset registered in the generated HDF5 Compound descriptor.
The tests explicitly verify: