From ff695a61a7e4df082769547f527fb3552b45a76f Mon Sep 17 00:00:00 2001 From: Magnus Ulimoen Date: Wed, 3 Apr 2024 22:09:19 +0200 Subject: [PATCH 01/17] checkpoint --- hdf5-types/src/dyn_value.rs | 1 + hdf5-types/src/h5type.rs | 16 ++++++++ hdf5-types/src/lib.rs | 3 +- hdf5/src/hl/datatype.rs | 6 +++ hdf5/src/hl/location.rs | 77 +++++++++++++++++++++++++++++++++++++ hdf5/src/lib.rs | 11 +++--- 6 files changed, 108 insertions(+), 6 deletions(-) diff --git a/hdf5-types/src/dyn_value.rs b/hdf5-types/src/dyn_value.rs index 1ef50ef3..0bd8fd87 100644 --- a/hdf5-types/src/dyn_value.rs +++ b/hdf5-types/src/dyn_value.rs @@ -703,6 +703,7 @@ impl<'a> DynValue<'a> { FixedUnicode(_) => DynFixedString::new(buf, true).into(), VarLenAscii => DynVarLenString::new(buf, false).into(), VarLenUnicode => DynVarLenString::new(buf, true).into(), + Reference(x) => todo!(), } } } diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index f8dbf6dd..e5057880 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -160,6 +160,7 @@ pub enum TypeDescriptor { VarLenArray(Box), VarLenAscii, VarLenUnicode, + Reference(Reference), } impl Display for TypeDescriptor { @@ -186,6 +187,8 @@ impl Display for TypeDescriptor { TypeDescriptor::VarLenArray(ref tp) => write!(f, "[{}] (var len)", tp), TypeDescriptor::VarLenAscii => write!(f, "string (var len)"), TypeDescriptor::VarLenUnicode => write!(f, "unicode (var len)"), + TypeDescriptor::Reference(Reference::Object) => write!(f, "reference (object)"), + TypeDescriptor::Reference(Reference::Region) => write!(f, "reference (region)"), } } } @@ -202,6 +205,7 @@ impl TypeDescriptor { Self::FixedAscii(len) | Self::FixedUnicode(len) => len, Self::VarLenArray(_) => mem::size_of::(), Self::VarLenAscii | Self::VarLenUnicode => mem::size_of::<*const u8>(), + Self::Reference(reftyp) => reftyp.size(), } } @@ -375,6 +379,18 @@ unsafe impl H5Type for VarLenUnicode { } } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum Reference { + Object, + Region, +} + +impl Reference { + fn size(self) -> usize { + mem::size_of::() + } +} + #[cfg(test)] pub mod tests { use super::TypeDescriptor as TD; diff --git a/hdf5-types/src/lib.rs b/hdf5-types/src/lib.rs index c5de9ac1..71cd07fd 100644 --- a/hdf5-types/src/lib.rs +++ b/hdf5-types/src/lib.rs @@ -28,7 +28,8 @@ mod complex; pub use self::array::VarLenArray; pub use self::dyn_value::{DynValue, OwnedDynValue}; pub use self::h5type::{ - CompoundField, CompoundType, EnumMember, EnumType, FloatSize, H5Type, IntSize, TypeDescriptor, + CompoundField, CompoundType, EnumMember, EnumType, FloatSize, H5Type, IntSize, Reference, + TypeDescriptor, }; pub use self::string::{FixedAscii, FixedUnicode, StringError, VarLenAscii, VarLenUnicode}; diff --git a/hdf5/src/hl/datatype.rs b/hdf5/src/hl/datatype.rs index 9c9ebe58..c281f38c 100644 --- a/hdf5/src/hl/datatype.rs +++ b/hdf5/src/hl/datatype.rs @@ -410,6 +410,12 @@ impl Datatype { } TD::VarLenAscii => string_type(None, H5T_cset_t::H5T_CSET_ASCII), TD::VarLenUnicode => string_type(None, H5T_cset_t::H5T_CSET_UTF8), + TD::Reference(hdf5_types::Reference::Object) => { + Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF_OBJ))) + } + TD::Reference(hdf5_types::Reference::Region) => { + Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF_DSETREG))) + } } }); diff --git a/hdf5/src/hl/location.rs b/hdf5/src/hl/location.rs index 82adea4f..5bd61b97 100644 --- a/hdf5/src/hl/location.rs +++ b/hdf5/src/hl/location.rs @@ -22,6 +22,9 @@ use hdf5_sys::{ h5f::H5Fget_name, h5i::{H5Iget_file_id, H5Iget_name}, h5o::{H5O_type_t, H5Oget_comment}, + h5r::{ + H5R_ref_t, H5Rcreate_object, H5Rdereference2, H5Rdestroy, H5Rget_obj_type3, H5R_OBJECT1, + }, }; use crate::internal_prelude::*; @@ -146,6 +149,64 @@ impl Location { pub fn open_by_token(&self, token: LocationToken) -> Result { H5O_open_by_token(self.id(), token) } + + pub fn reference(&self, name: &str) -> Result { + let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); + let name = to_cstring(name)?; + h5call!(H5Rcreate_object(self.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; + Ok(ObjectReference { inner: unsafe { out.assume_init() } }) + } + + pub fn dereference(&self, reference: &ObjectReference) -> Result { + let mut objtype = std::mem::MaybeUninit::uninit(); + h5call!(H5Rget_obj_type3( + std::ptr::addr_of!(reference.inner), + H5P_DEFAULT, + objtype.as_mut_ptr() + ))?; + let objtype = unsafe { objtype.assume_init() }; + + let objid = h5call!(H5Rdereference2( + self.id(), + H5P_DEFAULT, + H5R_OBJECT1, + std::ptr::addr_of!(reference.inner).cast(), + ))?; + use hdf5_sys::h5o::H5O_type_t::*; + Ok(match objtype { + H5O_TYPE_GROUP => ReferencedObject::Group(Group::from_id(objid)?), + H5O_TYPE_DATASET => ReferencedObject::Dataset(Dataset::from_id(objid)?), + H5O_TYPE_NAMED_DATATYPE => ReferencedObject::Datatype(Datatype::from_id(objid)?), + #[cfg(feature = "1.12.0")] + H5O_TYPE_MAP => fail!("Can not create object from a map"), + H5O_TYPE_UNKNOWN => fail!("Unknown datatype"), + H5O_TYPE_NTYPES => fail!("hdf5 should not produce this type"), + }) + } +} + +#[derive(Clone, Debug)] +pub enum ReferencedObject { + Group(Group), + Dataset(Dataset), + Datatype(Datatype), +} + +#[repr(transparent)] +pub struct ObjectReference { + inner: H5R_ref_t, +} + +unsafe impl H5Type for ObjectReference { + fn type_descriptor() -> hdf5_types::TypeDescriptor { + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Object) + } +} + +impl Drop for ObjectReference { + fn drop(&mut self) { + let _e = h5call!(H5Rdestroy(&mut self.inner)); + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -417,4 +478,20 @@ pub mod tests { assert!(file.loc_info_by_name("gibberish").is_err()); }) } + + #[test] + pub fn test_references() { + use super::ReferencedObject; + with_tmp_file(|file| { + file.create_group("g").unwrap(); + let gref = file.reference("g").unwrap(); + let group = file.dereference(&gref).unwrap(); + assert!(matches!(group, ReferencedObject::Group(_))); + + file.new_dataset::().create("ds").unwrap(); + let dsref = file.reference("ds").unwrap(); + let ds = file.dereference(&dsref).unwrap(); + assert!(matches!(ds, ReferencedObject::Dataset(_))); + }) + } } diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index c8d70426..84d3a8a1 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -58,11 +58,12 @@ mod export { hl::extents::{Extent, Extents, SimpleExtents}, hl::selection::{Hyperslab, Selection, SliceOrIndex}, hl::{ - Attribute, AttributeBuilder, AttributeBuilderData, AttributeBuilderEmpty, - AttributeBuilderEmptyShape, ByteReader, Container, Conversion, Dataset, DatasetBuilder, - DatasetBuilderData, DatasetBuilderEmpty, DatasetBuilderEmptyShape, Dataspace, Datatype, - File, FileBuilder, Group, LinkInfo, LinkType, Location, LocationInfo, LocationToken, - LocationType, Object, OpenMode, PropertyList, Reader, Writer, + location::ObjectReference, Attribute, AttributeBuilder, AttributeBuilderData, + AttributeBuilderEmpty, AttributeBuilderEmptyShape, ByteReader, Container, Conversion, + Dataset, DatasetBuilder, DatasetBuilderData, DatasetBuilderEmpty, + DatasetBuilderEmptyShape, Dataspace, Datatype, File, FileBuilder, Group, LinkInfo, + LinkType, Location, LocationInfo, LocationToken, LocationType, Object, OpenMode, + PropertyList, Reader, Writer, }, }; From c9065db1a212dbd429220de287559a55c695be74 Mon Sep 17 00:00:00 2001 From: Magnus Ulimoen Date: Wed, 3 Apr 2024 22:20:53 +0200 Subject: [PATCH 02/17] Working? --- hdf5/examples/stuff.rs | 17 +++++++++++++++++ hdf5/src/globals.rs | 1 + hdf5/src/hl/datatype.rs | 2 +- 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 hdf5/examples/stuff.rs diff --git a/hdf5/examples/stuff.rs b/hdf5/examples/stuff.rs new file mode 100644 index 00000000..c7a552a5 --- /dev/null +++ b/hdf5/examples/stuff.rs @@ -0,0 +1,17 @@ +use hdf5::*; + +fn main() { + let file = File::create("file.h5").unwrap(); + let _g = file.create_group("g").unwrap(); + let _gg = file.create_group("gg").unwrap(); + let refs = + [file.reference("g").unwrap(), file.reference("gg").unwrap(), file.reference("g").unwrap()]; + + let ds = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); + ds.write_slice(&refs[1..2], 1..2).unwrap(); + ds.write_slice(&refs[2..3], 2..3).unwrap(); + + let refs: ndarray::Array1 = ds.read().unwrap(); + let g = file.dereference(&refs[1]); + println!("{g:?}"); +} diff --git a/hdf5/src/globals.rs b/hdf5/src/globals.rs index e1b9461f..dc7d8148 100644 --- a/hdf5/src/globals.rs +++ b/hdf5/src/globals.rs @@ -76,6 +76,7 @@ link_hid!(H5T_STD_B64BE, h5t::H5T_STD_B64BE); link_hid!(H5T_STD_B64LE, h5t::H5T_STD_B64LE); link_hid!(H5T_STD_REF_OBJ, h5t::H5T_STD_REF_OBJ); link_hid!(H5T_STD_REF_DSETREG, h5t::H5T_STD_REF_DSETREG); +link_hid!(H5T_STD_REF, h5t::H5T_STD_REF); link_hid!(H5T_UNIX_D32BE, h5t::H5T_UNIX_D32BE); link_hid!(H5T_UNIX_D32LE, h5t::H5T_UNIX_D32LE); link_hid!(H5T_UNIX_D64BE, h5t::H5T_UNIX_D64BE); diff --git a/hdf5/src/hl/datatype.rs b/hdf5/src/hl/datatype.rs index c281f38c..a80a760e 100644 --- a/hdf5/src/hl/datatype.rs +++ b/hdf5/src/hl/datatype.rs @@ -411,7 +411,7 @@ impl Datatype { TD::VarLenAscii => string_type(None, H5T_cset_t::H5T_CSET_ASCII), TD::VarLenUnicode => string_type(None, H5T_cset_t::H5T_CSET_UTF8), TD::Reference(hdf5_types::Reference::Object) => { - Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF_OBJ))) + Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF))) } TD::Reference(hdf5_types::Reference::Region) => { Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF_DSETREG))) From 4737ddc1a997189333db5da016ea7686cfe160e4 Mon Sep 17 00:00:00 2001 From: Magnus Ulimoen Date: Wed, 3 Apr 2024 22:42:24 +0200 Subject: [PATCH 03/17] checkpoint --- hdf5-types/src/array.rs | 4 ++-- hdf5-types/src/h5type.rs | 8 +++++++- hdf5-types/src/lib.rs | 1 + hdf5-types/src/references.rs | 1 + hdf5/examples/stuff.py | 11 +++++++++++ hdf5/examples/stuff.rs | 22 ++++++++++++++++++++-- hdf5/src/hl/datatype.rs | 5 ++++- hdf5/src/hl/location.rs | 32 +++++++++++++++++++++++++------- hdf5/src/lib.rs | 12 ++++++------ 9 files changed, 77 insertions(+), 19 deletions(-) create mode 100644 hdf5-types/src/references.rs create mode 100644 hdf5/examples/stuff.py diff --git a/hdf5-types/src/array.rs b/hdf5-types/src/array.rs index cf090924..f3dfa978 100644 --- a/hdf5-types/src/array.rs +++ b/hdf5-types/src/array.rs @@ -6,7 +6,7 @@ use std::ptr; use std::slice; #[repr(C)] -pub struct VarLenArray { +pub struct VarLenArray { len: usize, ptr: *const T, tag: PhantomData, @@ -50,7 +50,7 @@ impl VarLenArray { } } -impl Drop for VarLenArray { +impl Drop for VarLenArray { fn drop(&mut self) { if !self.ptr.is_null() { unsafe { diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index e5057880..49b7abdc 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -189,6 +189,7 @@ impl Display for TypeDescriptor { TypeDescriptor::VarLenUnicode => write!(f, "unicode (var len)"), TypeDescriptor::Reference(Reference::Object) => write!(f, "reference (object)"), TypeDescriptor::Reference(Reference::Region) => write!(f, "reference (region)"), + TypeDescriptor::Reference(Reference::Std) => write!(f, "reference"), } } } @@ -383,11 +384,16 @@ unsafe impl H5Type for VarLenUnicode { pub enum Reference { Object, Region, + Std, } impl Reference { fn size(self) -> usize { - mem::size_of::() + match self { + Self::Object => mem::size_of::(), + Self::Region => mem::size_of::(), + Self::Std => mem::size_of::(), + } } } diff --git a/hdf5-types/src/lib.rs b/hdf5-types/src/lib.rs index 71cd07fd..48176f4a 100644 --- a/hdf5-types/src/lib.rs +++ b/hdf5-types/src/lib.rs @@ -20,6 +20,7 @@ extern crate quickcheck; mod array; pub mod dyn_value; mod h5type; +pub mod references; mod string; #[cfg(feature = "complex")] diff --git a/hdf5-types/src/references.rs b/hdf5-types/src/references.rs new file mode 100644 index 00000000..ba768139 --- /dev/null +++ b/hdf5-types/src/references.rs @@ -0,0 +1 @@ +// TODO: move references to this location diff --git a/hdf5/examples/stuff.py b/hdf5/examples/stuff.py new file mode 100644 index 00000000..a48f7c28 --- /dev/null +++ b/hdf5/examples/stuff.py @@ -0,0 +1,11 @@ +from h5py import File +import numpy as np + +# http://docs.h5py.org/en/stable/high/dims.html + +f = File('dims_1d.h5', 'w') + +f['x1'] = [1, 2] +f['x1'].make_scale('x1 name') +f['data'] = np.ones((2,), 'f') +f['data'].dims[0].attach_scale(f['x1']) diff --git a/hdf5/examples/stuff.rs b/hdf5/examples/stuff.rs index c7a552a5..6770e01c 100644 --- a/hdf5/examples/stuff.rs +++ b/hdf5/examples/stuff.rs @@ -11,7 +11,25 @@ fn main() { ds.write_slice(&refs[1..2], 1..2).unwrap(); ds.write_slice(&refs[2..3], 2..3).unwrap(); - let refs: ndarray::Array1 = ds.read().unwrap(); + let refs: ndarray::Array1 = ds.read().unwrap(); let g = file.dereference(&refs[1]); - println!("{g:?}"); + // println!("{g:?}"); + + #[derive(H5Type, Debug)] + #[repr(C)] + struct RefList { + dataset: ObjectReference, + dimension: u32, + } + + let file = File::open("dims_1d.h5").unwrap(); + let ds = file.dataset("x1").unwrap(); + let attr = ds.attr("REFERENCE_LIST").unwrap(); + let reflist = attr.read_1d::().unwrap(); + assert_eq!(reflist.len(), 1); + + let ds = file.dataset("data").unwrap(); + let attr = ds.attr("DIMENSION_LIST").unwrap(); + let dimlist = attr.read_1d::>().unwrap(); + println!("{dimlist:?}"); } diff --git a/hdf5/src/hl/datatype.rs b/hdf5/src/hl/datatype.rs index a80a760e..447f9072 100644 --- a/hdf5/src/hl/datatype.rs +++ b/hdf5/src/hl/datatype.rs @@ -410,9 +410,12 @@ impl Datatype { } TD::VarLenAscii => string_type(None, H5T_cset_t::H5T_CSET_ASCII), TD::VarLenUnicode => string_type(None, H5T_cset_t::H5T_CSET_UTF8), - TD::Reference(hdf5_types::Reference::Object) => { + TD::Reference(hdf5_types::Reference::Std) => { Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF))) } + TD::Reference(hdf5_types::Reference::Object) => { + Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF_OBJ))) + } TD::Reference(hdf5_types::Reference::Region) => { Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF_DSETREG))) } diff --git a/hdf5/src/hl/location.rs b/hdf5/src/hl/location.rs index 5bd61b97..5fa1849f 100644 --- a/hdf5/src/hl/location.rs +++ b/hdf5/src/hl/location.rs @@ -150,14 +150,14 @@ impl Location { H5O_open_by_token(self.id(), token) } - pub fn reference(&self, name: &str) -> Result { + pub fn reference(&self, name: &str) -> Result { let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); let name = to_cstring(name)?; h5call!(H5Rcreate_object(self.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; - Ok(ObjectReference { inner: unsafe { out.assume_init() } }) + Ok(StdReference { inner: unsafe { out.assume_init() } }) } - pub fn dereference(&self, reference: &ObjectReference) -> Result { + pub fn dereference(&self, reference: &StdReference) -> Result { let mut objtype = std::mem::MaybeUninit::uninit(); h5call!(H5Rget_obj_type3( std::ptr::addr_of!(reference.inner), @@ -193,22 +193,40 @@ pub enum ReferencedObject { } #[repr(transparent)] -pub struct ObjectReference { +pub struct StdReference { inner: H5R_ref_t, } -unsafe impl H5Type for ObjectReference { +unsafe impl H5Type for StdReference { fn type_descriptor() -> hdf5_types::TypeDescriptor { - hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Object) + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Std) } } -impl Drop for ObjectReference { +impl Drop for StdReference { fn drop(&mut self) { let _e = h5call!(H5Rdestroy(&mut self.inner)); } } +#[repr(transparent)] +#[derive(Debug, Copy, Clone)] +pub struct ObjectReference { + inner: hdf5_sys::h5r::hobj_ref_t, +} + +unsafe impl H5Type for ObjectReference { + fn type_descriptor() -> hdf5_types::TypeDescriptor { + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Object) + } +} + +// impl Drop for ObjectReference { +// fn drop(&mut self) { +// // let _e = h5call!(H5Rdestroy(&mut self.inner)); +// } +// } + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct LocationToken( #[cfg(not(feature = "1.12.0"))] haddr_t, diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index 84d3a8a1..2cff068f 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -58,12 +58,12 @@ mod export { hl::extents::{Extent, Extents, SimpleExtents}, hl::selection::{Hyperslab, Selection, SliceOrIndex}, hl::{ - location::ObjectReference, Attribute, AttributeBuilder, AttributeBuilderData, - AttributeBuilderEmpty, AttributeBuilderEmptyShape, ByteReader, Container, Conversion, - Dataset, DatasetBuilder, DatasetBuilderData, DatasetBuilderEmpty, - DatasetBuilderEmptyShape, Dataspace, Datatype, File, FileBuilder, Group, LinkInfo, - LinkType, Location, LocationInfo, LocationToken, LocationType, Object, OpenMode, - PropertyList, Reader, Writer, + location::{ObjectReference, StdReference}, + Attribute, AttributeBuilder, AttributeBuilderData, AttributeBuilderEmpty, + AttributeBuilderEmptyShape, ByteReader, Container, Conversion, Dataset, DatasetBuilder, + DatasetBuilderData, DatasetBuilderEmpty, DatasetBuilderEmptyShape, Dataspace, Datatype, + File, FileBuilder, Group, LinkInfo, LinkType, Location, LocationInfo, LocationToken, + LocationType, Object, OpenMode, PropertyList, Reader, Writer, }, }; From c01e92df29bfe0002654872a7bffbeba7067b039 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 7 Apr 2024 09:02:16 +0100 Subject: [PATCH 04/17] Add Integration Tests for Current Reference Implementations --- hdf5/src/hl/location.rs | 10 ++ hdf5/src/lib.rs | 2 +- hdf5/tests/test_references.rs | 199 ++++++++++++++++++++++++++++++++++ 3 files changed, 210 insertions(+), 1 deletion(-) create mode 100644 hdf5/tests/test_references.rs diff --git a/hdf5/src/hl/location.rs b/hdf5/src/hl/location.rs index 5fa1849f..b28b532c 100644 --- a/hdf5/src/hl/location.rs +++ b/hdf5/src/hl/location.rs @@ -150,6 +150,9 @@ impl Location { H5O_open_by_token(self.id(), token) } + /// Generate a [standard reference](StdReference) to the object for a reference storage. + /// + /// This can be a group, dataset or datatype. Other objects are not supported. pub fn reference(&self, name: &str) -> Result { let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); let name = to_cstring(name)?; @@ -157,6 +160,9 @@ impl Location { Ok(StdReference { inner: unsafe { out.assume_init() } }) } + /// Get a reference back to the referenced object from a standard reference. + /// + /// This can be called against any object in the same file as the referenced object. pub fn dereference(&self, reference: &StdReference) -> Result { let mut objtype = std::mem::MaybeUninit::uninit(); h5call!(H5Rget_obj_type3( @@ -185,6 +191,9 @@ impl Location { } } +/// The result of dereferencing a [standard reference](StdReference). +/// +/// Each variant represents a different type of object that can be referenced by a [StdReference]. #[derive(Clone, Debug)] pub enum ReferencedObject { Group(Group), @@ -192,6 +201,7 @@ pub enum ReferencedObject { Datatype(Datatype), } +/// A reference to a HDF5 item that can be stored in attributes or datasets. #[repr(transparent)] pub struct StdReference { inner: H5R_ref_t, diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index 2cff068f..a98234d2 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -58,7 +58,7 @@ mod export { hl::extents::{Extent, Extents, SimpleExtents}, hl::selection::{Hyperslab, Selection, SliceOrIndex}, hl::{ - location::{ObjectReference, StdReference}, + location::{ObjectReference, ReferencedObject, StdReference}, Attribute, AttributeBuilder, AttributeBuilderData, AttributeBuilderEmpty, AttributeBuilderEmptyShape, ByteReader, Container, Conversion, Dataset, DatasetBuilder, DatasetBuilderData, DatasetBuilderEmpty, DatasetBuilderEmptyShape, Dataspace, Datatype, diff --git a/hdf5/tests/test_references.rs b/hdf5/tests/test_references.rs new file mode 100644 index 00000000..47c290ca --- /dev/null +++ b/hdf5/tests/test_references.rs @@ -0,0 +1,199 @@ +//! Tests for the reference type storage and retrieval. +//! + +mod common; + +use common::util::new_in_memory_file; +use hdf5::{file, Group, H5Type, ObjectReference, ReferencedObject, StdReference}; +use hdf5_types::VarLenArray; + +#[test] +fn test_group_references() { + let file = new_in_memory_file().unwrap(); + let g1 = file.create_group("g1").unwrap(); + let _g1_1 = g1.create_group("g1_1").unwrap(); + + let refs = [file.reference("g1").unwrap(), g1.reference("g1_1").unwrap()]; + + let ds = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); + + let read_references = ds.read_1d::().unwrap(); + + match file.dereference(&read_references[0]).unwrap() { + ReferencedObject::Group(g) => { + assert_eq!(g.name(), "/g1"); + } + _ => { + panic!("Expected a group reference"); + } + } + + match file.dereference(&read_references[1]).unwrap() { + ReferencedObject::Group(g) => { + assert_eq!(g.name(), "/g1/g1_1"); + } + _ => { + panic!("Expected a group reference"); + } + } + + match g1.dereference(&read_references[1]).expect("Dereference against the group.") { + ReferencedObject::Group(g) => { + assert_eq!(g.name(), "/g1/g1_1"); + } + _ => { + panic!("Expected a group reference"); + } + } +} + +#[test] +fn test_dataset_references() { + let dummy_data = [0, 1, 2, 3]; + + let file = new_in_memory_file().unwrap(); + let ds1 = file.new_dataset_builder().with_data(&dummy_data).create("ds1").unwrap(); + let g = file.create_group("g").unwrap(); + let ds2 = g.new_dataset_builder().with_data(&dummy_data).create("ds2").unwrap(); + let refs = [file.reference("ds1").unwrap(), g.reference("ds2").unwrap()]; + + let ds_refs = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); + let read_references = ds_refs.read_1d::().unwrap(); + + match file.dereference(&read_references[0]).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/ds1"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &dummy_data); + } + _ => { + panic!("Expected a dataset reference"); + } + } + + match file.dereference(&read_references[1]).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/g/ds2"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &dummy_data); + } + _ => { + panic!("Expected a dataset reference"); + } + } +} + +#[test] +fn test_reference_in_attribute() { + let file = new_in_memory_file().unwrap(); + let _ds1 = file.new_dataset_builder().with_data(&[1, 2, 3]).create("ds1").unwrap(); + let ref1 = file.reference("ds1").unwrap(); + + file.new_attr::().create("ref_attr").unwrap().write_scalar(&ref1).unwrap(); + + let ref_read = file.attr("ref_attr").unwrap().read_scalar::().unwrap(); + + match file.dereference(&ref_read).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/ds1"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &[1, 2, 3]); + } + _ => { + panic!("Expected a dataset reference"); + } + } +} + +#[test] +fn test_reference_errors_on_attribute() { + let file = new_in_memory_file().unwrap(); + let attr = file.new_attr::().create("ref_attr").unwrap(); + // Attempt to create reference to attribute should fail. + let result = file.reference("ref_attr"); + assert!(result.is_err()); +} + +#[test] +fn test_reference_in_datatype() { + let dummy_data = [1, 2, 3, 4]; + let file = new_in_memory_file().unwrap(); + let _ds1 = file.new_dataset_builder().with_data(&dummy_data).create("ds1").unwrap(); + let ref1 = file.reference("ds1").unwrap(); + let _ds2 = file.new_dataset_builder().with_data(&dummy_data).create("ds2").unwrap(); + let ref2 = file.reference("ds2").unwrap(); + + #[derive(H5Type)] + #[repr(C)] + struct RefData { + dataset: StdReference, + value: i32, + } + + let ds3 = file + .new_dataset_builder() + .with_data(&[RefData { dataset: ref1, value: 42 }, RefData { dataset: ref2, value: 43 }]) + .create("ds3") + .unwrap(); + + let read_data = ds3.read_1d::().unwrap(); + assert_eq!(read_data[0].value, 42); + assert_eq!(read_data[1].value, 43); + match file.dereference(&read_data[0].dataset).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/ds1"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &dummy_data); + } + _ => { + panic!("Expected a dataset reference"); + } + } + match file.dereference(&read_data[1].dataset).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/ds2"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &dummy_data); + } + _ => { + panic!("Expected a dataset reference"); + } + } +} + +/* TODO: Should this be possible? Reference not implementing Copy blocks this in a few places. +#[test] +fn test_references_in_array_types() { + let file = new_in_memory_file().unwrap(); + let _ds1 = file.new_dataset_builder().with_data(&[1, 2, 3]).create("ds1").unwrap(); + let _ds2 = file.new_dataset_builder().with_data(&[4, 5, 6]).create("ds2").unwrap(); + let refs = [file.reference("ds1").unwrap(), file.reference("ds2").unwrap()]; + let refs_array = VarLenArray::from_slice(&refs); + + file.new_attr::>() + .create("var_array") + .unwrap() + .write_scalar(&refs) + .unwrap(); + + let read_array = + file.attr("var_array").unwrap().read_scalar::>().unwrap(); + + let read_refs = read_array.as_slice(); + + assert_eq!(read_refs.len(), 2); + match file.dereference(&read_refs[0]).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/ds1"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &[1, 2, 3]); + } + _ => { + panic!("Expected a dataset reference"); + } + } + match file.dereference(&read_refs[1]).unwrap() { + ReferencedObject::Dataset(ds) => { + assert_eq!(ds.name(), "/ds2"); + assert_eq!(ds.read_1d::().unwrap().as_slice().unwrap(), &[4, 5, 6]); + } + _ => { + panic!("Expected a dataset reference"); + } + } +} +*/ From 67fe6897f6cf1f8c382d5327bbf41591fde3eef1 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 7 Apr 2024 17:42:44 +0100 Subject: [PATCH 05/17] Factor references out to their own module --- hdf5-types/src/array.rs | 46 +++--- hdf5-types/src/h5type.rs | 2 +- hdf5/src/hl.rs | 1 + hdf5/src/hl/location.rs | 95 +----------- hdf5/src/hl/references/legacy.rs | 22 +++ hdf5/src/hl/references/mod.rs | 5 + hdf5/src/hl/references/modern.rs | 146 ++++++++++++++++++ hdf5/src/lib.rs | 2 +- ...eferences.rs => test_modern_references.rs} | 1 + 9 files changed, 204 insertions(+), 116 deletions(-) create mode 100644 hdf5/src/hl/references/legacy.rs create mode 100644 hdf5/src/hl/references/mod.rs create mode 100644 hdf5/src/hl/references/modern.rs rename hdf5/tests/{test_references.rs => test_modern_references.rs} (99%) diff --git a/hdf5-types/src/array.rs b/hdf5-types/src/array.rs index f3dfa978..2bca73b2 100644 --- a/hdf5-types/src/array.rs +++ b/hdf5-types/src/array.rs @@ -12,23 +12,7 @@ pub struct VarLenArray { tag: PhantomData, } -impl VarLenArray { - pub unsafe fn from_parts(p: *const T, len: usize) -> Self { - let (len, ptr) = if !p.is_null() && len != 0 { - let dst = crate::malloc(len * mem::size_of::()); - ptr::copy_nonoverlapping(p, dst.cast(), len); - (len, dst) - } else { - (0, ptr::null_mut()) - }; - Self { len, ptr: ptr as *const _, tag: PhantomData } - } - - #[inline] - pub fn from_slice(arr: &[T]) -> Self { - unsafe { Self::from_parts(arr.as_ptr(), arr.len()) } - } - +impl VarLenArray { #[inline] pub fn as_ptr(&self) -> *const T { self.ptr @@ -46,7 +30,27 @@ impl VarLenArray { #[inline] pub fn as_slice(&self) -> &[T] { - self + unsafe { slice::from_raw_parts(self.as_ptr(), self.len()) } + } +} + +impl VarLenArray { + /// Creates a new `VarLenArray` from a slice by copying the source data. + #[inline] + pub fn from_slice(arr: &[T]) -> Self { + unsafe { Self::from_parts(arr.as_ptr(), arr.len()) } + } + + /// Create a new `VarLenArray` from a ptr/len combo by copying the source data. + pub unsafe fn from_parts(p: *const T, len: usize) -> Self { + let (len, ptr) = if !p.is_null() && len != 0 { + let dst = crate::malloc(len * mem::size_of::()); + ptr::copy_nonoverlapping(p, dst.cast(), len); + (len, dst) + } else { + (0, ptr::null_mut()) + }; + Self { len, ptr: ptr as *const _, tag: PhantomData } } } @@ -71,7 +75,7 @@ impl Clone for VarLenArray { } } -impl Deref for VarLenArray { +impl Deref for VarLenArray { type Target = [T]; #[inline] @@ -79,7 +83,7 @@ impl Deref for VarLenArray { if self.len == 0 || self.ptr.is_null() { &[] } else { - unsafe { slice::from_raw_parts(self.as_ptr(), self.len()) } + self.as_slice() } } } @@ -135,7 +139,7 @@ impl PartialEq<[T; N]> for VarLenArray { } } -impl fmt::Debug for VarLenArray { +impl fmt::Debug for VarLenArray { #[inline] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.as_slice().fmt(f) diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index 49b7abdc..30e00cbc 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -345,7 +345,7 @@ unsafe impl H5Type for [T; N] { } } -unsafe impl H5Type for VarLenArray { +unsafe impl H5Type for VarLenArray { #[inline] fn type_descriptor() -> TypeDescriptor { TypeDescriptor::VarLenArray(Box::new(::type_descriptor())) diff --git a/hdf5/src/hl.rs b/hdf5/src/hl.rs index ac813036..5cae2f0b 100644 --- a/hdf5/src/hl.rs +++ b/hdf5/src/hl.rs @@ -11,6 +11,7 @@ pub mod group; pub mod location; pub mod object; pub mod plist; +pub mod references; pub mod selection; pub use self::{ diff --git a/hdf5/src/hl/location.rs b/hdf5/src/hl/location.rs index b28b532c..4d90a3fe 100644 --- a/hdf5/src/hl/location.rs +++ b/hdf5/src/hl/location.rs @@ -22,9 +22,6 @@ use hdf5_sys::{ h5f::H5Fget_name, h5i::{H5Iget_file_id, H5Iget_name}, h5o::{H5O_type_t, H5Oget_comment}, - h5r::{ - H5R_ref_t, H5Rcreate_object, H5Rdereference2, H5Rdestroy, H5Rget_obj_type3, H5R_OBJECT1, - }, }; use crate::internal_prelude::*; @@ -154,89 +151,17 @@ impl Location { /// /// This can be a group, dataset or datatype. Other objects are not supported. pub fn reference(&self, name: &str) -> Result { - let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); - let name = to_cstring(name)?; - h5call!(H5Rcreate_object(self.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; - Ok(StdReference { inner: unsafe { out.assume_init() } }) + StdReference::create(self, name) } /// Get a reference back to the referenced object from a standard reference. /// /// This can be called against any object in the same file as the referenced object. pub fn dereference(&self, reference: &StdReference) -> Result { - let mut objtype = std::mem::MaybeUninit::uninit(); - h5call!(H5Rget_obj_type3( - std::ptr::addr_of!(reference.inner), - H5P_DEFAULT, - objtype.as_mut_ptr() - ))?; - let objtype = unsafe { objtype.assume_init() }; - - let objid = h5call!(H5Rdereference2( - self.id(), - H5P_DEFAULT, - H5R_OBJECT1, - std::ptr::addr_of!(reference.inner).cast(), - ))?; - use hdf5_sys::h5o::H5O_type_t::*; - Ok(match objtype { - H5O_TYPE_GROUP => ReferencedObject::Group(Group::from_id(objid)?), - H5O_TYPE_DATASET => ReferencedObject::Dataset(Dataset::from_id(objid)?), - H5O_TYPE_NAMED_DATATYPE => ReferencedObject::Datatype(Datatype::from_id(objid)?), - #[cfg(feature = "1.12.0")] - H5O_TYPE_MAP => fail!("Can not create object from a map"), - H5O_TYPE_UNKNOWN => fail!("Unknown datatype"), - H5O_TYPE_NTYPES => fail!("hdf5 should not produce this type"), - }) - } -} - -/// The result of dereferencing a [standard reference](StdReference). -/// -/// Each variant represents a different type of object that can be referenced by a [StdReference]. -#[derive(Clone, Debug)] -pub enum ReferencedObject { - Group(Group), - Dataset(Dataset), - Datatype(Datatype), -} - -/// A reference to a HDF5 item that can be stored in attributes or datasets. -#[repr(transparent)] -pub struct StdReference { - inner: H5R_ref_t, -} - -unsafe impl H5Type for StdReference { - fn type_descriptor() -> hdf5_types::TypeDescriptor { - hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Std) + reference.dereference(self) } } -impl Drop for StdReference { - fn drop(&mut self) { - let _e = h5call!(H5Rdestroy(&mut self.inner)); - } -} - -#[repr(transparent)] -#[derive(Debug, Copy, Clone)] -pub struct ObjectReference { - inner: hdf5_sys::h5r::hobj_ref_t, -} - -unsafe impl H5Type for ObjectReference { - fn type_descriptor() -> hdf5_types::TypeDescriptor { - hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Object) - } -} - -// impl Drop for ObjectReference { -// fn drop(&mut self) { -// // let _e = h5call!(H5Rdestroy(&mut self.inner)); -// } -// } - #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct LocationToken( #[cfg(not(feature = "1.12.0"))] haddr_t, @@ -506,20 +431,4 @@ pub mod tests { assert!(file.loc_info_by_name("gibberish").is_err()); }) } - - #[test] - pub fn test_references() { - use super::ReferencedObject; - with_tmp_file(|file| { - file.create_group("g").unwrap(); - let gref = file.reference("g").unwrap(); - let group = file.dereference(&gref).unwrap(); - assert!(matches!(group, ReferencedObject::Group(_))); - - file.new_dataset::().create("ds").unwrap(); - let dsref = file.reference("ds").unwrap(); - let ds = file.dereference(&dsref).unwrap(); - assert!(matches!(ds, ReferencedObject::Dataset(_))); - }) - } } diff --git a/hdf5/src/hl/references/legacy.rs b/hdf5/src/hl/references/legacy.rs new file mode 100644 index 00000000..94038523 --- /dev/null +++ b/hdf5/src/hl/references/legacy.rs @@ -0,0 +1,22 @@ +//! The pre v1.12.0 reference types. + +use hdf5_sys::h5r::hobj_ref_t; +use hdf5_types::H5Type; + +#[repr(transparent)] +#[derive(Debug, Copy, Clone)] +pub struct ObjectReference { + inner: hobj_ref_t, +} + +unsafe impl H5Type for ObjectReference { + fn type_descriptor() -> hdf5_types::TypeDescriptor { + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Object) + } +} + +// impl Drop for ObjectReference { +// fn drop(&mut self) { +// // let _e = h5call!(H5Rdestroy(&mut self.inner)); +// } +// } diff --git a/hdf5/src/hl/references/mod.rs b/hdf5/src/hl/references/mod.rs new file mode 100644 index 00000000..7fe4c190 --- /dev/null +++ b/hdf5/src/hl/references/mod.rs @@ -0,0 +1,5 @@ +mod legacy; +mod modern; + +pub use legacy::ObjectReference; +pub use modern::{ReferencedObject, StdReference}; diff --git a/hdf5/src/hl/references/modern.rs b/hdf5/src/hl/references/modern.rs new file mode 100644 index 00000000..62f9f49d --- /dev/null +++ b/hdf5/src/hl/references/modern.rs @@ -0,0 +1,146 @@ +use hdf5_sys::h5r::{ + H5R_ref_t, H5Rcopy, H5Rcreate_object, H5Rdereference2, H5Rdestroy, H5Requal, H5Rget_obj_type3, + H5R_OBJECT1, +}; + +use crate::internal_prelude::*; +use crate::{Dataset, Datatype, Group, Location}; + +/// The result of dereferencing a [standard reference](StdReference). +/// +/// Each variant represents a different type of object that can be referenced by a [StdReference]. +#[derive(Clone, Debug)] +pub enum ReferencedObject { + Group(Group), + Dataset(Dataset), + Datatype(Datatype), +} + +/// A reference to a HDF5 item that can be stored in attributes or datasets. +#[repr(transparent)] +pub struct StdReference { + inner: H5R_ref_t, +} + +impl StdReference { + pub fn create(source: &Location, name: &str) -> Result { + let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); + let name = to_cstring(name)?; + h5call!(H5Rcreate_object(source.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; + Ok(StdReference { inner: unsafe { out.assume_init() } }) + } + + fn ptr(&self) -> *const H5R_ref_t { + std::ptr::addr_of!(self.inner) + } + + pub fn try_clone(&self) -> Result { + unsafe { + let mut dst = std::mem::MaybeUninit::uninit(); + h5call!(H5Rcopy(self.ptr(), dst.as_mut_ptr()))?; + Ok(Self { inner: dst.assume_init() }) + } + } + + pub fn dereference(&self, location: &Location) -> Result { + let mut objtype = std::mem::MaybeUninit::uninit(); + h5call!(H5Rget_obj_type3( + std::ptr::addr_of!(self.inner), + H5P_DEFAULT, + objtype.as_mut_ptr() + ))?; + let objtype = unsafe { objtype.assume_init() }; + + let objid = h5call!(H5Rdereference2( + location.id(), + H5P_DEFAULT, + H5R_OBJECT1, + std::ptr::addr_of!(self.inner).cast(), + ))?; + use hdf5_sys::h5o::H5O_type_t::*; + Ok(match objtype { + H5O_TYPE_GROUP => ReferencedObject::Group(Group::from_id(objid)?), + H5O_TYPE_DATASET => ReferencedObject::Dataset(Dataset::from_id(objid)?), + H5O_TYPE_NAMED_DATATYPE => ReferencedObject::Datatype(Datatype::from_id(objid)?), + #[cfg(feature = "1.12.0")] + H5O_TYPE_MAP => fail!("Can not create object from a map"), + H5O_TYPE_UNKNOWN => fail!("Unknown datatype"), + H5O_TYPE_NTYPES => fail!("hdf5 should not produce this type"), + }) + } +} + +impl PartialEq for StdReference { + fn eq(&self, other: &Self) -> bool { + let result = unsafe { H5Requal(self.ptr(), other.ptr()) }; + println!("Result of H5Requal: {}", result); + match result { + 0 => false, + 1.. => true, + // Less than 0 indicates an error but not clear on what the error conditions could be. Fail the equality right now. + _ => false, + } + } +} + +impl Eq for StdReference {} + +//todo: could we query some actual object parameters to make this more useful? +impl std::fmt::Debug for StdReference { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("StdReference") + } +} + +unsafe impl H5Type for StdReference { + fn type_descriptor() -> hdf5_types::TypeDescriptor { + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Std) + } +} + +impl Drop for StdReference { + fn drop(&mut self) { + let _e = h5call!(H5Rdestroy(&mut self.inner)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + pub fn test_references() { + use super::ReferencedObject; + with_tmp_file(|file| { + file.create_group("g").unwrap(); + let gref = file.reference("g").unwrap(); + let group = file.dereference(&gref).unwrap(); + assert!(matches!(group, ReferencedObject::Group(_))); + + file.new_dataset::().create("ds").unwrap(); + let dsref = file.reference("ds").unwrap(); + let ds = file.dereference(&dsref).unwrap(); + assert!(matches!(ds, ReferencedObject::Dataset(_))); + }) + } + + #[test] + fn test_reference_equality() { + with_tmp_file(|file| { + file.create_group("g").unwrap(); + let gref1 = file.reference("g").unwrap(); + let gref2 = file.reference("g").unwrap(); + assert_eq!(gref1, gref2); + + file.new_dataset::().create("ds").unwrap(); + file.new_dataset::().create("ds2").unwrap(); + let dsref1 = file.reference("ds").unwrap(); + let dsref2 = file.reference("ds").unwrap(); + assert_eq!(dsref1, dsref2); + + println!("{}", gref1 == dsref1); + assert_ne!(gref1, dsref1); + assert_ne!(dsref1, file.reference("ds2").unwrap()); + }) + } +} diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index a98234d2..a4e19e55 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -58,7 +58,7 @@ mod export { hl::extents::{Extent, Extents, SimpleExtents}, hl::selection::{Hyperslab, Selection, SliceOrIndex}, hl::{ - location::{ObjectReference, ReferencedObject, StdReference}, + references::{ObjectReference, ReferencedObject, StdReference}, Attribute, AttributeBuilder, AttributeBuilderData, AttributeBuilderEmpty, AttributeBuilderEmptyShape, ByteReader, Container, Conversion, Dataset, DatasetBuilder, DatasetBuilderData, DatasetBuilderEmpty, DatasetBuilderEmptyShape, Dataspace, Datatype, diff --git a/hdf5/tests/test_references.rs b/hdf5/tests/test_modern_references.rs similarity index 99% rename from hdf5/tests/test_references.rs rename to hdf5/tests/test_modern_references.rs index 47c290ca..224639fc 100644 --- a/hdf5/tests/test_references.rs +++ b/hdf5/tests/test_modern_references.rs @@ -1,5 +1,6 @@ //! Tests for the reference type storage and retrieval. //! +#![cfg(feature = "1.12.0")] mod common; From b06368a15895c5774c330cd01d32b6b2d5074a00 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 7 Apr 2024 17:46:33 +0100 Subject: [PATCH 06/17] Add version flags to HDF5-Types References --- hdf5-types/src/h5type.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index 30e00cbc..d1c97ea8 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -384,6 +384,7 @@ unsafe impl H5Type for VarLenUnicode { pub enum Reference { Object, Region, + #[cfg(feature = "1.12.0")] Std, } @@ -392,6 +393,7 @@ impl Reference { match self { Self::Object => mem::size_of::(), Self::Region => mem::size_of::(), + #[cfg(feature = "1.12.0")] Self::Std => mem::size_of::(), } } From 98a8e2b7cff740cbdc79ef41fd7713dedfa49605 Mon Sep 17 00:00:00 2001 From: James McNally Date: Mon, 8 Apr 2024 20:27:07 +0100 Subject: [PATCH 07/17] refactor: ObjectReference Trait Added a trait for object references and refactored the StdReference as a newtype (ObjectReference2) and behind the trait so we can support it side by side with the older object references. --- hdf5-types/build.rs | 12 +++- hdf5/examples/stuff.rs | 10 +-- hdf5/src/hl/location.rs | 9 +-- hdf5/src/hl/references/legacy.rs | 4 +- hdf5/src/hl/references/mod.rs | 23 +++++- .../hl/references/{modern.rs => standard.rs} | 69 +++++++++--------- hdf5/src/lib.rs | 5 +- hdf5/tests/test_modern_references.rs | 70 ++++++++++++------- 8 files changed, 129 insertions(+), 73 deletions(-) rename hdf5/src/hl/references/{modern.rs => standard.rs} (77%) diff --git a/hdf5-types/build.rs b/hdf5-types/build.rs index 7419b5eb..79a60bd9 100644 --- a/hdf5-types/build.rs +++ b/hdf5-types/build.rs @@ -1,6 +1,14 @@ fn main() { + let print_feature = |key: &str| println!("cargo:rustc-cfg=feature=\"{}\"", key); + let print_cfg = |key: &str| println!("cargo:rustc-cfg={}", key); println!("cargo:rerun-if-changed=build.rs"); - if std::env::var_os("DEP_HDF5_MSVC_DLL_INDIRECTION").is_some() { - println!("cargo:rustc-cfg=windows_dll"); + for (key, _) in std::env::vars() { + match key.as_str() { + "DEP_HDF5_MSVC_DLL_INDIRECTION" => print_cfg("windows_dll"), + key if key.starts_with("DEP_HDF5_VERSION_") => { + print_feature(&key.trim_start_matches("DEP_HDF5_VERSION_").replace('_', ".")); + } + _ => continue, + } } } diff --git a/hdf5/examples/stuff.rs b/hdf5/examples/stuff.rs index 6770e01c..b235adc7 100644 --- a/hdf5/examples/stuff.rs +++ b/hdf5/examples/stuff.rs @@ -4,21 +4,21 @@ fn main() { let file = File::create("file.h5").unwrap(); let _g = file.create_group("g").unwrap(); let _gg = file.create_group("gg").unwrap(); - let refs = + let refs: [ObjectReference2; 3] = [file.reference("g").unwrap(), file.reference("gg").unwrap(), file.reference("g").unwrap()]; let ds = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); ds.write_slice(&refs[1..2], 1..2).unwrap(); ds.write_slice(&refs[2..3], 2..3).unwrap(); - let refs: ndarray::Array1 = ds.read().unwrap(); + let refs: ndarray::Array1 = ds.read().unwrap(); let g = file.dereference(&refs[1]); // println!("{g:?}"); - #[derive(H5Type, Debug)] + #[derive(H5Type)] #[repr(C)] struct RefList { - dataset: ObjectReference, + dataset: ObjectReference2, dimension: u32, } @@ -30,6 +30,6 @@ fn main() { let ds = file.dataset("data").unwrap(); let attr = ds.attr("DIMENSION_LIST").unwrap(); - let dimlist = attr.read_1d::>().unwrap(); + let dimlist = attr.read_1d::>().unwrap(); println!("{dimlist:?}"); } diff --git a/hdf5/src/hl/location.rs b/hdf5/src/hl/location.rs index 4d90a3fe..dd1f0755 100644 --- a/hdf5/src/hl/location.rs +++ b/hdf5/src/hl/location.rs @@ -27,6 +27,7 @@ use hdf5_sys::{ use crate::internal_prelude::*; use super::attribute::AttributeBuilderEmpty; +use super::references::ObjectReference2; /// Named location (file, group, dataset, named datatype). #[repr(transparent)] @@ -147,17 +148,17 @@ impl Location { H5O_open_by_token(self.id(), token) } - /// Generate a [standard reference](StdReference) to the object for a reference storage. + /// Generate a [object reference](ObjectReference) to the object for a reference storage. /// /// This can be a group, dataset or datatype. Other objects are not supported. - pub fn reference(&self, name: &str) -> Result { - StdReference::create(self, name) + pub fn reference(&self, name: &str) -> Result { + R::create(self, name) } /// Get a reference back to the referenced object from a standard reference. /// /// This can be called against any object in the same file as the referenced object. - pub fn dereference(&self, reference: &StdReference) -> Result { + pub fn dereference(&self, reference: &R) -> Result { reference.dereference(self) } } diff --git a/hdf5/src/hl/references/legacy.rs b/hdf5/src/hl/references/legacy.rs index 94038523..e5d0fe86 100644 --- a/hdf5/src/hl/references/legacy.rs +++ b/hdf5/src/hl/references/legacy.rs @@ -5,11 +5,11 @@ use hdf5_types::H5Type; #[repr(transparent)] #[derive(Debug, Copy, Clone)] -pub struct ObjectReference { +pub struct ObjectReference1 { inner: hobj_ref_t, } -unsafe impl H5Type for ObjectReference { +unsafe impl H5Type for ObjectReference1 { fn type_descriptor() -> hdf5_types::TypeDescriptor { hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Object) } diff --git a/hdf5/src/hl/references/mod.rs b/hdf5/src/hl/references/mod.rs index 7fe4c190..c08e0d08 100644 --- a/hdf5/src/hl/references/mod.rs +++ b/hdf5/src/hl/references/mod.rs @@ -1,5 +1,22 @@ +use crate::internal_prelude::*; +use crate::Location; + mod legacy; -mod modern; +mod standard; + +pub use legacy::ObjectReference1; +pub use standard::ObjectReference2; -pub use legacy::ObjectReference; -pub use modern::{ReferencedObject, StdReference}; +pub trait ObjectReference: Sized + H5Type { + fn create(source: &Location, name: &str) -> Result; + fn dereference(&self, location: &Location) -> Result; +} +/// The result of dereferencing an [object reference](ObjectReference). +/// +/// Each variant represents a different type of object that can be referenced by a [ObjectReference]. +#[derive(Clone, Debug)] +pub enum ReferencedObject { + Group(Group), + Dataset(Dataset), + Datatype(Datatype), +} diff --git a/hdf5/src/hl/references/modern.rs b/hdf5/src/hl/references/standard.rs similarity index 77% rename from hdf5/src/hl/references/modern.rs rename to hdf5/src/hl/references/standard.rs index 62f9f49d..f8d94e68 100644 --- a/hdf5/src/hl/references/modern.rs +++ b/hdf5/src/hl/references/standard.rs @@ -1,21 +1,14 @@ +use std::ops::Deref; + use hdf5_sys::h5r::{ H5R_ref_t, H5Rcopy, H5Rcreate_object, H5Rdereference2, H5Rdestroy, H5Requal, H5Rget_obj_type3, H5R_OBJECT1, }; +use super::ObjectReference; use crate::internal_prelude::*; use crate::{Dataset, Datatype, Group, Location}; -/// The result of dereferencing a [standard reference](StdReference). -/// -/// Each variant represents a different type of object that can be referenced by a [StdReference]. -#[derive(Clone, Debug)] -pub enum ReferencedObject { - Group(Group), - Dataset(Dataset), - Datatype(Datatype), -} - /// A reference to a HDF5 item that can be stored in attributes or datasets. #[repr(transparent)] pub struct StdReference { @@ -23,13 +16,6 @@ pub struct StdReference { } impl StdReference { - pub fn create(source: &Location, name: &str) -> Result { - let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); - let name = to_cstring(name)?; - h5call!(H5Rcreate_object(source.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; - Ok(StdReference { inner: unsafe { out.assume_init() } }) - } - fn ptr(&self) -> *const H5R_ref_t { std::ptr::addr_of!(self.inner) } @@ -41,22 +27,35 @@ impl StdReference { Ok(Self { inner: dst.assume_init() }) } } +} + +#[repr(transparent)] +#[derive(Debug, PartialEq, Eq)] +pub struct ObjectReference2(StdReference); + +impl Deref for ObjectReference2 { + type Target = StdReference; - pub fn dereference(&self, location: &Location) -> Result { + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl ObjectReference for ObjectReference2 { + fn create(source: &Location, name: &str) -> Result { + let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); + let name = to_cstring(name)?; + h5call!(H5Rcreate_object(source.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; + Ok(Self(StdReference { inner: unsafe { out.assume_init() } })) + } + + fn dereference(&self, location: &Location) -> Result { let mut objtype = std::mem::MaybeUninit::uninit(); - h5call!(H5Rget_obj_type3( - std::ptr::addr_of!(self.inner), - H5P_DEFAULT, - objtype.as_mut_ptr() - ))?; + h5call!(H5Rget_obj_type3(self.ptr(), H5P_DEFAULT, objtype.as_mut_ptr()))?; let objtype = unsafe { objtype.assume_init() }; - let objid = h5call!(H5Rdereference2( - location.id(), - H5P_DEFAULT, - H5R_OBJECT1, - std::ptr::addr_of!(self.inner).cast(), - ))?; + let objid = + h5call!(H5Rdereference2(location.id(), H5P_DEFAULT, H5R_OBJECT1, self.ptr().cast(),))?; use hdf5_sys::h5o::H5O_type_t::*; Ok(match objtype { H5O_TYPE_GROUP => ReferencedObject::Group(Group::from_id(objid)?), @@ -70,6 +69,12 @@ impl StdReference { } } +unsafe impl H5Type for ObjectReference2 { + fn type_descriptor() -> hdf5_types::TypeDescriptor { + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Std) + } +} + impl PartialEq for StdReference { fn eq(&self, other: &Self) -> bool { let result = unsafe { H5Requal(self.ptr(), other.ptr()) }; @@ -113,12 +118,12 @@ mod tests { use super::ReferencedObject; with_tmp_file(|file| { file.create_group("g").unwrap(); - let gref = file.reference("g").unwrap(); + let gref = file.reference::("g").unwrap(); let group = file.dereference(&gref).unwrap(); assert!(matches!(group, ReferencedObject::Group(_))); file.new_dataset::().create("ds").unwrap(); - let dsref = file.reference("ds").unwrap(); + let dsref = file.reference::("ds").unwrap(); let ds = file.dereference(&dsref).unwrap(); assert!(matches!(ds, ReferencedObject::Dataset(_))); }) @@ -128,7 +133,7 @@ mod tests { fn test_reference_equality() { with_tmp_file(|file| { file.create_group("g").unwrap(); - let gref1 = file.reference("g").unwrap(); + let gref1 = file.reference::("g").unwrap(); let gref2 = file.reference("g").unwrap(); assert_eq!(gref1, gref2); diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index a4e19e55..545bf26e 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -58,7 +58,7 @@ mod export { hl::extents::{Extent, Extents, SimpleExtents}, hl::selection::{Hyperslab, Selection, SliceOrIndex}, hl::{ - references::{ObjectReference, ReferencedObject, StdReference}, + references::{ObjectReference, ReferencedObject}, Attribute, AttributeBuilder, AttributeBuilderData, AttributeBuilderEmpty, AttributeBuilderEmptyShape, ByteReader, Container, Conversion, Dataset, DatasetBuilder, DatasetBuilderData, DatasetBuilderEmpty, DatasetBuilderEmptyShape, Dataspace, Datatype, @@ -67,6 +67,9 @@ mod export { }, }; + #[cfg(feature = "1.12.0")] + pub use crate::hl::references::ObjectReference2; + #[doc(hidden)] pub use crate::error::h5check; diff --git a/hdf5/tests/test_modern_references.rs b/hdf5/tests/test_modern_references.rs index 224639fc..313a3290 100644 --- a/hdf5/tests/test_modern_references.rs +++ b/hdf5/tests/test_modern_references.rs @@ -5,20 +5,19 @@ mod common; use common::util::new_in_memory_file; -use hdf5::{file, Group, H5Type, ObjectReference, ReferencedObject, StdReference}; +use hdf5::{file, Group, H5Type, ObjectReference, ObjectReference2, ReferencedObject}; use hdf5_types::VarLenArray; -#[test] -fn test_group_references() { +fn test_group_references() { let file = new_in_memory_file().unwrap(); let g1 = file.create_group("g1").unwrap(); let _g1_1 = g1.create_group("g1_1").unwrap(); - let refs = [file.reference("g1").unwrap(), g1.reference("g1_1").unwrap()]; + let refs: [R; 2] = [file.reference("g1").unwrap(), g1.reference("g1_1").unwrap()]; let ds = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); - let read_references = ds.read_1d::().unwrap(); + let read_references = ds.read_1d::().unwrap(); match file.dereference(&read_references[0]).unwrap() { ReferencedObject::Group(g) => { @@ -48,18 +47,17 @@ fn test_group_references() { } } -#[test] -fn test_dataset_references() { +fn test_dataset_references() { let dummy_data = [0, 1, 2, 3]; let file = new_in_memory_file().unwrap(); let ds1 = file.new_dataset_builder().with_data(&dummy_data).create("ds1").unwrap(); let g = file.create_group("g").unwrap(); let ds2 = g.new_dataset_builder().with_data(&dummy_data).create("ds2").unwrap(); - let refs = [file.reference("ds1").unwrap(), g.reference("ds2").unwrap()]; + let refs: [R; 2] = [file.reference("ds1").unwrap(), g.reference("ds2").unwrap()]; let ds_refs = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); - let read_references = ds_refs.read_1d::().unwrap(); + let read_references = ds_refs.read_1d::().unwrap(); match file.dereference(&read_references[0]).unwrap() { ReferencedObject::Dataset(ds) => { @@ -82,15 +80,14 @@ fn test_dataset_references() { } } -#[test] -fn test_reference_in_attribute() { +fn test_reference_in_attribute() { let file = new_in_memory_file().unwrap(); let _ds1 = file.new_dataset_builder().with_data(&[1, 2, 3]).create("ds1").unwrap(); - let ref1 = file.reference("ds1").unwrap(); + let ref1: R = file.reference("ds1").unwrap(); - file.new_attr::().create("ref_attr").unwrap().write_scalar(&ref1).unwrap(); + file.new_attr::().create("ref_attr").unwrap().write_scalar(&ref1).unwrap(); - let ref_read = file.attr("ref_attr").unwrap().read_scalar::().unwrap(); + let ref_read = file.attr("ref_attr").unwrap().read_scalar::().unwrap(); match file.dereference(&ref_read).unwrap() { ReferencedObject::Dataset(ds) => { @@ -103,28 +100,26 @@ fn test_reference_in_attribute() { } } -#[test] -fn test_reference_errors_on_attribute() { +fn test_reference_errors_on_attribute() { let file = new_in_memory_file().unwrap(); let attr = file.new_attr::().create("ref_attr").unwrap(); // Attempt to create reference to attribute should fail. - let result = file.reference("ref_attr"); + let result = file.reference::("ref_attr"); assert!(result.is_err()); } -#[test] -fn test_reference_in_datatype() { +fn test_reference_in_datatype() { let dummy_data = [1, 2, 3, 4]; let file = new_in_memory_file().unwrap(); let _ds1 = file.new_dataset_builder().with_data(&dummy_data).create("ds1").unwrap(); - let ref1 = file.reference("ds1").unwrap(); + let ref1 = file.reference::("ds1").unwrap(); let _ds2 = file.new_dataset_builder().with_data(&dummy_data).create("ds2").unwrap(); - let ref2 = file.reference("ds2").unwrap(); + let ref2 = file.reference::("ds2").unwrap(); #[derive(H5Type)] #[repr(C)] - struct RefData { - dataset: StdReference, + struct RefData { + dataset: R, value: i32, } @@ -134,7 +129,7 @@ fn test_reference_in_datatype() { .create("ds3") .unwrap(); - let read_data = ds3.read_1d::().unwrap(); + let read_data = ds3.read_1d::>().unwrap(); assert_eq!(read_data[0].value, 42); assert_eq!(read_data[1].value, 43); match file.dereference(&read_data[0].dataset).unwrap() { @@ -198,3 +193,30 @@ fn test_references_in_array_types() { } } */ + +mod object2tests { + use super::*; + #[test] + fn test_group_references_with_objectreference2() { + test_group_references::(); + } + + #[test] + fn test_dataset_references_with_object_reference2() { + test_dataset_references::(); + } + #[test] + fn test_reference_in_attribute_object_reference2() { + test_reference_in_attribute::(); + } + + #[test] + fn test_reference_errors_on_attribute_object_reference2() { + test_reference_errors_on_attribute::(); + } + + #[test] + fn test_reference_in_datatype_object_reference2() { + test_reference_in_datatype::(); + } +} From c41b4748eab29caffc93af975954e9dfe17b982d Mon Sep 17 00:00:00 2001 From: James McNally Date: Mon, 8 Apr 2024 20:31:26 +0100 Subject: [PATCH 08/17] fix: Have types work against pre-1.12 --- hdf5-types/src/h5type.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index d1c97ea8..88f3bc9d 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -189,6 +189,7 @@ impl Display for TypeDescriptor { TypeDescriptor::VarLenUnicode => write!(f, "unicode (var len)"), TypeDescriptor::Reference(Reference::Object) => write!(f, "reference (object)"), TypeDescriptor::Reference(Reference::Region) => write!(f, "reference (region)"), + #[cfg(feature = "1.12.0")] TypeDescriptor::Reference(Reference::Std) => write!(f, "reference"), } } From 3688feaca79405b832fa6cec922f7be3bac1884b Mon Sep 17 00:00:00 2001 From: James McNally Date: Sat, 13 Apr 2024 13:46:39 +0100 Subject: [PATCH 09/17] wip: Support for Object1 and Object2 References This commit represents an MVP for object references. This includes the implementation of Object1 for 1.8 and later, and Object2 for 1.12 and later. --- hdf5/examples/stuff.py | 11 -- hdf5/examples/stuff.rs | 35 ------ hdf5/src/globals.rs | 1 + hdf5/src/hl/datatype.rs | 1 + hdf5/src/hl/location.rs | 1 - hdf5/src/hl/references/legacy.rs | 56 ++++++++-- hdf5/src/hl/references/mod.rs | 33 +++++- hdf5/src/hl/references/standard.rs | 102 +++++++++--------- hdf5/src/lib.rs | 2 +- ...eferences.rs => test_object_references.rs} | 81 +++++++++----- 10 files changed, 186 insertions(+), 137 deletions(-) delete mode 100644 hdf5/examples/stuff.py delete mode 100644 hdf5/examples/stuff.rs rename hdf5/tests/{test_modern_references.rs => test_object_references.rs} (78%) diff --git a/hdf5/examples/stuff.py b/hdf5/examples/stuff.py deleted file mode 100644 index a48f7c28..00000000 --- a/hdf5/examples/stuff.py +++ /dev/null @@ -1,11 +0,0 @@ -from h5py import File -import numpy as np - -# http://docs.h5py.org/en/stable/high/dims.html - -f = File('dims_1d.h5', 'w') - -f['x1'] = [1, 2] -f['x1'].make_scale('x1 name') -f['data'] = np.ones((2,), 'f') -f['data'].dims[0].attach_scale(f['x1']) diff --git a/hdf5/examples/stuff.rs b/hdf5/examples/stuff.rs deleted file mode 100644 index b235adc7..00000000 --- a/hdf5/examples/stuff.rs +++ /dev/null @@ -1,35 +0,0 @@ -use hdf5::*; - -fn main() { - let file = File::create("file.h5").unwrap(); - let _g = file.create_group("g").unwrap(); - let _gg = file.create_group("gg").unwrap(); - let refs: [ObjectReference2; 3] = - [file.reference("g").unwrap(), file.reference("gg").unwrap(), file.reference("g").unwrap()]; - - let ds = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); - ds.write_slice(&refs[1..2], 1..2).unwrap(); - ds.write_slice(&refs[2..3], 2..3).unwrap(); - - let refs: ndarray::Array1 = ds.read().unwrap(); - let g = file.dereference(&refs[1]); - // println!("{g:?}"); - - #[derive(H5Type)] - #[repr(C)] - struct RefList { - dataset: ObjectReference2, - dimension: u32, - } - - let file = File::open("dims_1d.h5").unwrap(); - let ds = file.dataset("x1").unwrap(); - let attr = ds.attr("REFERENCE_LIST").unwrap(); - let reflist = attr.read_1d::().unwrap(); - assert_eq!(reflist.len(), 1); - - let ds = file.dataset("data").unwrap(); - let attr = ds.attr("DIMENSION_LIST").unwrap(); - let dimlist = attr.read_1d::>().unwrap(); - println!("{dimlist:?}"); -} diff --git a/hdf5/src/globals.rs b/hdf5/src/globals.rs index dc7d8148..d6a12e8a 100644 --- a/hdf5/src/globals.rs +++ b/hdf5/src/globals.rs @@ -76,6 +76,7 @@ link_hid!(H5T_STD_B64BE, h5t::H5T_STD_B64BE); link_hid!(H5T_STD_B64LE, h5t::H5T_STD_B64LE); link_hid!(H5T_STD_REF_OBJ, h5t::H5T_STD_REF_OBJ); link_hid!(H5T_STD_REF_DSETREG, h5t::H5T_STD_REF_DSETREG); +#[cfg(feature = "1.12.0")] link_hid!(H5T_STD_REF, h5t::H5T_STD_REF); link_hid!(H5T_UNIX_D32BE, h5t::H5T_UNIX_D32BE); link_hid!(H5T_UNIX_D32LE, h5t::H5T_UNIX_D32LE); diff --git a/hdf5/src/hl/datatype.rs b/hdf5/src/hl/datatype.rs index 447f9072..1226e812 100644 --- a/hdf5/src/hl/datatype.rs +++ b/hdf5/src/hl/datatype.rs @@ -410,6 +410,7 @@ impl Datatype { } TD::VarLenAscii => string_type(None, H5T_cset_t::H5T_CSET_ASCII), TD::VarLenUnicode => string_type(None, H5T_cset_t::H5T_CSET_UTF8), + #[cfg(feature = "1.12.0")] TD::Reference(hdf5_types::Reference::Std) => { Ok(h5try!(H5Tcopy(*crate::globals::H5T_STD_REF))) } diff --git a/hdf5/src/hl/location.rs b/hdf5/src/hl/location.rs index dd1f0755..35205b68 100644 --- a/hdf5/src/hl/location.rs +++ b/hdf5/src/hl/location.rs @@ -27,7 +27,6 @@ use hdf5_sys::{ use crate::internal_prelude::*; use super::attribute::AttributeBuilderEmpty; -use super::references::ObjectReference2; /// Named location (file, group, dataset, named datatype). #[repr(transparent)] diff --git a/hdf5/src/hl/references/legacy.rs b/hdf5/src/hl/references/legacy.rs index e5d0fe86..685b5747 100644 --- a/hdf5/src/hl/references/legacy.rs +++ b/hdf5/src/hl/references/legacy.rs @@ -1,8 +1,21 @@ //! The pre v1.12.0 reference types. -use hdf5_sys::h5r::hobj_ref_t; +use crate::internal_prelude::*; + +use hdf5_sys::{ + h5o::H5O_type_t, + h5p::H5P_DEFAULT, + h5r::{hobj_ref_t, H5Rcreate, H5Rdereference, H5Rget_obj_type2}, +}; use hdf5_types::H5Type; +#[cfg(not(feature = "1.12.0"))] +use hdf5_sys::h5r::H5R_OBJECT as H5R_OBJECT1; +#[cfg(feature = "1.12.0")] +use hdf5_sys::h5r::H5R_OBJECT1; + +use crate::{Location, ObjectReference}; + #[repr(transparent)] #[derive(Debug, Copy, Clone)] pub struct ObjectReference1 { @@ -15,8 +28,39 @@ unsafe impl H5Type for ObjectReference1 { } } -// impl Drop for ObjectReference { -// fn drop(&mut self) { -// // let _e = h5call!(H5Rdestroy(&mut self.inner)); -// } -// } +impl ObjectReference for ObjectReference1 { + const REF_TYPE: hdf5_sys::h5r::H5R_type_t = H5R_OBJECT1; + + fn ptr(&self) -> *const c_void { + let pointer = std::ptr::addr_of!(self.inner); + pointer.cast() + } + + fn create(dataspace: &Location, name: &str) -> Result { + let mut ref_out: std::mem::MaybeUninit = std::mem::MaybeUninit::uninit(); + let name = to_cstring(name)?; + h5call!(H5Rcreate( + ref_out.as_mut_ptr().cast(), + dataspace.id(), + name.as_ptr(), + Self::REF_TYPE, + -1 + ))?; + let reference = unsafe { ref_out.assume_init() }; + Ok(Self { inner: reference }) + } + + fn get_object_type(&self, dataset: &Location) -> Result { + let mut objtype = std::mem::MaybeUninit::::uninit(); + h5call!(H5Rget_obj_type2(dataset.id(), H5R_OBJECT1, self.ptr(), objtype.as_mut_ptr()))?; + let objtype = unsafe { objtype.assume_init() }; + Ok(objtype) + } + + fn dereference(&self, dataset: &Location) -> Result { + let object_type = self.get_object_type(dataset)?; + let object_id = + h5call!(H5Rdereference(dataset.id(), H5P_DEFAULT, H5R_OBJECT1, self.ptr()))?; + ReferencedObject::from_type_and_id(object_type, object_id) + } +} diff --git a/hdf5/src/hl/references/mod.rs b/hdf5/src/hl/references/mod.rs index c08e0d08..52592070 100644 --- a/hdf5/src/hl/references/mod.rs +++ b/hdf5/src/hl/references/mod.rs @@ -2,13 +2,28 @@ use crate::internal_prelude::*; use crate::Location; mod legacy; + +#[cfg(feature = "1.12.0")] mod standard; +use hdf5_sys::h5o::H5O_type_t; +use hdf5_sys::h5r::H5R_type_t; + pub use legacy::ObjectReference1; +#[cfg(feature = "1.12.0")] pub use standard::ObjectReference2; pub trait ObjectReference: Sized + H5Type { - fn create(source: &Location, name: &str) -> Result; + const REF_TYPE: H5R_type_t; + fn ptr(&self) -> *const c_void; + + /// Get the type of the object that the reference points in the same space as the provided location. + fn get_object_type(&self, location: &Location) -> Result; + + /// Create a new reference in the same structure as the location provided. + fn create(location: &Location, name: &str) -> Result; + + /// Dereference the object reference. fn dereference(&self, location: &Location) -> Result; } /// The result of dereferencing an [object reference](ObjectReference). @@ -20,3 +35,19 @@ pub enum ReferencedObject { Dataset(Dataset), Datatype(Datatype), } + +impl ReferencedObject { + pub fn from_type_and_id(object_type: H5O_type_t, object_id: hid_t) -> Result { + use hdf5_sys::h5o::H5O_type_t::*; + let referenced_object = match object_type { + H5O_TYPE_GROUP => ReferencedObject::Group(Group::from_id(object_id)?), + H5O_TYPE_DATASET => ReferencedObject::Dataset(Dataset::from_id(object_id)?), + H5O_TYPE_NAMED_DATATYPE => ReferencedObject::Datatype(Datatype::from_id(object_id)?), + #[cfg(feature = "1.12.0")] + H5O_TYPE_MAP => fail!("Can not create object from a map"), + H5O_TYPE_UNKNOWN => fail!("Unknown datatype"), + H5O_TYPE_NTYPES => fail!("hdf5 should not produce this type"), + }; + Ok(referenced_object) + } +} diff --git a/hdf5/src/hl/references/standard.rs b/hdf5/src/hl/references/standard.rs index f8d94e68..5a19a0f7 100644 --- a/hdf5/src/hl/references/standard.rs +++ b/hdf5/src/hl/references/standard.rs @@ -1,13 +1,14 @@ use std::ops::Deref; +use hdf5_sys::h5o::H5O_type_t; +use hdf5_sys::h5r::H5R_type_t::H5R_OBJECT2; use hdf5_sys::h5r::{ - H5R_ref_t, H5Rcopy, H5Rcreate_object, H5Rdereference2, H5Rdestroy, H5Requal, H5Rget_obj_type3, - H5R_OBJECT1, + H5R_ref_t, H5Rcopy, H5Rcreate_object, H5Rdestroy, H5Rget_obj_type3, H5Ropen_object, }; use super::ObjectReference; use crate::internal_prelude::*; -use crate::{Dataset, Datatype, Group, Location}; +use crate::Location; /// A reference to a HDF5 item that can be stored in attributes or datasets. #[repr(transparent)] @@ -30,9 +31,15 @@ impl StdReference { } #[repr(transparent)] -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct ObjectReference2(StdReference); +impl ObjectReference2 { + pub fn try_clone(&self) -> Result { + self.0.try_clone().map(|std_ref| Self(std_ref)) + } +} + impl Deref for ObjectReference2 { type Target = StdReference; @@ -42,30 +49,28 @@ impl Deref for ObjectReference2 { } impl ObjectReference for ObjectReference2 { - fn create(source: &Location, name: &str) -> Result { - let mut out: std::mem::MaybeUninit<_> = std::mem::MaybeUninit::uninit(); - let name = to_cstring(name)?; - h5call!(H5Rcreate_object(source.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr()))?; - Ok(Self(StdReference { inner: unsafe { out.assume_init() } })) + const REF_TYPE: hdf5_sys::h5r::H5R_type_t = H5R_OBJECT2; + + fn ptr(&self) -> *const c_void { + self.0.ptr().cast() } - fn dereference(&self, location: &Location) -> Result { - let mut objtype = std::mem::MaybeUninit::uninit(); - h5call!(H5Rget_obj_type3(self.ptr(), H5P_DEFAULT, objtype.as_mut_ptr()))?; + fn create(location: &Location, name: &str) -> Result { + let reference: H5R_ref_t = create_object_reference(location, name)?; + Ok(Self(StdReference { inner: reference })) + } + + fn get_object_type(&self, _location: &Location) -> Result { + let mut objtype = std::mem::MaybeUninit::::uninit(); + h5call!(H5Rget_obj_type3(self.0.ptr(), H5P_DEFAULT, objtype.as_mut_ptr()))?; let objtype = unsafe { objtype.assume_init() }; + Ok(objtype) + } - let objid = - h5call!(H5Rdereference2(location.id(), H5P_DEFAULT, H5R_OBJECT1, self.ptr().cast(),))?; - use hdf5_sys::h5o::H5O_type_t::*; - Ok(match objtype { - H5O_TYPE_GROUP => ReferencedObject::Group(Group::from_id(objid)?), - H5O_TYPE_DATASET => ReferencedObject::Dataset(Dataset::from_id(objid)?), - H5O_TYPE_NAMED_DATATYPE => ReferencedObject::Datatype(Datatype::from_id(objid)?), - #[cfg(feature = "1.12.0")] - H5O_TYPE_MAP => fail!("Can not create object from a map"), - H5O_TYPE_UNKNOWN => fail!("Unknown datatype"), - H5O_TYPE_NTYPES => fail!("hdf5 should not produce this type"), - }) + fn dereference(&self, location: &Location) -> Result { + let object_type = self.get_object_type(location)?; + let object_id = h5call!(H5Ropen_object(self.0.ptr(), H5P_DEFAULT, H5P_DEFAULT))?; + ReferencedObject::from_type_and_id(object_type, object_id) } } @@ -75,21 +80,6 @@ unsafe impl H5Type for ObjectReference2 { } } -impl PartialEq for StdReference { - fn eq(&self, other: &Self) -> bool { - let result = unsafe { H5Requal(self.ptr(), other.ptr()) }; - println!("Result of H5Requal: {}", result); - match result { - 0 => false, - 1.. => true, - // Less than 0 indicates an error but not clear on what the error conditions could be. Fail the equality right now. - _ => false, - } - } -} - -impl Eq for StdReference {} - //todo: could we query some actual object parameters to make this more useful? impl std::fmt::Debug for StdReference { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -109,6 +99,13 @@ impl Drop for StdReference { } } +fn create_object_reference(dataset: &Location, name: &str) -> Result { + let mut out: std::mem::MaybeUninit = std::mem::MaybeUninit::uninit(); + let name = to_cstring(name)?; + h5call!(H5Rcreate_object(dataset.id(), name.as_ptr(), H5P_DEFAULT, out.as_mut_ptr().cast(),))?; + unsafe { Ok(out.assume_init()) } +} + #[cfg(test)] mod tests { use super::*; @@ -130,22 +127,19 @@ mod tests { } #[test] - fn test_reference_equality() { + fn test_standard_reference_clone() { with_tmp_file(|file| { - file.create_group("g").unwrap(); - let gref1 = file.reference::("g").unwrap(); - let gref2 = file.reference("g").unwrap(); - assert_eq!(gref1, gref2); - - file.new_dataset::().create("ds").unwrap(); - file.new_dataset::().create("ds2").unwrap(); - let dsref1 = file.reference("ds").unwrap(); - let dsref2 = file.reference("ds").unwrap(); - assert_eq!(dsref1, dsref2); - - println!("{}", gref1 == dsref1); - assert_ne!(gref1, dsref1); - assert_ne!(dsref1, file.reference("ds2").unwrap()); + let orig_group = file.create_group("g").unwrap(); + let ref1 = file.reference::("g").unwrap(); + let ref2 = ref1.try_clone().unwrap(); + + let ref2_group = ref2.dereference(&file).unwrap(); + match ref2_group { + crate::ReferencedObject::Group(g) => { + assert_eq!(g.name(), orig_group.name()) + } + _ => panic!("Wrong reference type."), + } }) } } diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index 545bf26e..7dd4b5d6 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -58,7 +58,7 @@ mod export { hl::extents::{Extent, Extents, SimpleExtents}, hl::selection::{Hyperslab, Selection, SliceOrIndex}, hl::{ - references::{ObjectReference, ReferencedObject}, + references::{ObjectReference, ObjectReference1, ReferencedObject}, Attribute, AttributeBuilder, AttributeBuilderData, AttributeBuilderEmpty, AttributeBuilderEmptyShape, ByteReader, Container, Conversion, Dataset, DatasetBuilder, DatasetBuilderData, DatasetBuilderEmpty, DatasetBuilderEmptyShape, Dataspace, Datatype, diff --git a/hdf5/tests/test_modern_references.rs b/hdf5/tests/test_object_references.rs similarity index 78% rename from hdf5/tests/test_modern_references.rs rename to hdf5/tests/test_object_references.rs index 313a3290..2b4c74ad 100644 --- a/hdf5/tests/test_modern_references.rs +++ b/hdf5/tests/test_object_references.rs @@ -1,12 +1,12 @@ //! Tests for the reference type storage and retrieval. //! -#![cfg(feature = "1.12.0")] mod common; use common::util::new_in_memory_file; -use hdf5::{file, Group, H5Type, ObjectReference, ObjectReference2, ReferencedObject}; -use hdf5_types::VarLenArray; +#[cfg(feature = "1.12.0")] +use hdf5::ObjectReference2; +use hdf5::{H5Type, ObjectReference, ObjectReference1, ReferencedObject}; fn test_group_references() { let file = new_in_memory_file().unwrap(); @@ -51,9 +51,9 @@ fn test_dataset_references() { let dummy_data = [0, 1, 2, 3]; let file = new_in_memory_file().unwrap(); - let ds1 = file.new_dataset_builder().with_data(&dummy_data).create("ds1").unwrap(); + let _ds1 = file.new_dataset_builder().with_data(&dummy_data).create("ds1").unwrap(); let g = file.create_group("g").unwrap(); - let ds2 = g.new_dataset_builder().with_data(&dummy_data).create("ds2").unwrap(); + let _ds2 = g.new_dataset_builder().with_data(&dummy_data).create("ds2").unwrap(); let refs: [R; 2] = [file.reference("ds1").unwrap(), g.reference("ds2").unwrap()]; let ds_refs = file.new_dataset_builder().with_data(&refs).create("refs").unwrap(); @@ -102,7 +102,7 @@ fn test_reference_in_attribute() { fn test_reference_errors_on_attribute() { let file = new_in_memory_file().unwrap(); - let attr = file.new_attr::().create("ref_attr").unwrap(); + let _attr = file.new_attr::().create("ref_attr").unwrap(); // Attempt to create reference to attribute should fail. let result = file.reference::("ref_attr"); assert!(result.is_err()); @@ -193,30 +193,55 @@ fn test_references_in_array_types() { } } */ +#[test] +fn test_group_references_with_objectreference1() { + test_group_references::(); +} -mod object2tests { - use super::*; - #[test] - fn test_group_references_with_objectreference2() { - test_group_references::(); - } +#[test] +fn test_dataset_references_with_object_reference1() { + test_dataset_references::(); +} +#[test] +fn test_reference_in_attribute_object_reference1() { + test_reference_in_attribute::(); +} - #[test] - fn test_dataset_references_with_object_reference2() { - test_dataset_references::(); - } - #[test] - fn test_reference_in_attribute_object_reference2() { - test_reference_in_attribute::(); - } +#[test] +fn test_reference_errors_on_attribute_object_reference1() { + test_reference_errors_on_attribute::(); +} - #[test] - fn test_reference_errors_on_attribute_object_reference2() { - test_reference_errors_on_attribute::(); - } +#[test] +fn test_reference_in_datatype_object_reference1() { + test_reference_in_datatype::(); +} - #[test] - fn test_reference_in_datatype_object_reference2() { - test_reference_in_datatype::(); - } +#[cfg(feature = "1.12.0")] +#[test] +fn test_group_references_with_objectreference2() { + test_group_references::(); +} + +#[cfg(feature = "1.12.0")] +#[test] +fn test_dataset_references_with_object_reference2() { + test_dataset_references::(); +} +#[cfg(feature = "1.12.0")] +#[test] +fn test_reference_in_attribute_object_reference2() { + test_reference_in_attribute::(); +} + +#[cfg(feature = "1.12.0")] +#[test] +fn test_reference_errors_on_attribute_object_reference2() { + test_reference_errors_on_attribute::(); +} + +#[cfg(feature = "1.12.0")] +#[test] +fn test_reference_in_datatype_object_reference2() { + test_reference_in_datatype::(); } From f6c0564aa8de3d83b5b4fc8310036ff0269a450b Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 08:02:05 +0100 Subject: [PATCH 10/17] fix: Mark DynValue variable as unused --- hdf5-types/src/dyn_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hdf5-types/src/dyn_value.rs b/hdf5-types/src/dyn_value.rs index 0bd8fd87..23a09357 100644 --- a/hdf5-types/src/dyn_value.rs +++ b/hdf5-types/src/dyn_value.rs @@ -703,7 +703,7 @@ impl<'a> DynValue<'a> { FixedUnicode(_) => DynFixedString::new(buf, true).into(), VarLenAscii => DynVarLenString::new(buf, false).into(), VarLenUnicode => DynVarLenString::new(buf, true).into(), - Reference(x) => todo!(), + Reference(_x) => todo!(), } } } From 36fb0265084cbc0f24c5d0a1e252171c5b5787d3 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 08:05:23 +0100 Subject: [PATCH 11/17] fix: Remove StdReference try_clone The wrapper to H5RCopy is causing issues in some enviroments. This commit removes it for now as it isn't critical to the basic object implementation. --- hdf5/src/hl/references/standard.rs | 35 +----------------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/hdf5/src/hl/references/standard.rs b/hdf5/src/hl/references/standard.rs index 5a19a0f7..823c38ec 100644 --- a/hdf5/src/hl/references/standard.rs +++ b/hdf5/src/hl/references/standard.rs @@ -2,9 +2,7 @@ use std::ops::Deref; use hdf5_sys::h5o::H5O_type_t; use hdf5_sys::h5r::H5R_type_t::H5R_OBJECT2; -use hdf5_sys::h5r::{ - H5R_ref_t, H5Rcopy, H5Rcreate_object, H5Rdestroy, H5Rget_obj_type3, H5Ropen_object, -}; +use hdf5_sys::h5r::{H5R_ref_t, H5Rcreate_object, H5Rdestroy, H5Rget_obj_type3, H5Ropen_object}; use super::ObjectReference; use crate::internal_prelude::*; @@ -20,26 +18,12 @@ impl StdReference { fn ptr(&self) -> *const H5R_ref_t { std::ptr::addr_of!(self.inner) } - - pub fn try_clone(&self) -> Result { - unsafe { - let mut dst = std::mem::MaybeUninit::uninit(); - h5call!(H5Rcopy(self.ptr(), dst.as_mut_ptr()))?; - Ok(Self { inner: dst.assume_init() }) - } - } } #[repr(transparent)] #[derive(Debug)] pub struct ObjectReference2(StdReference); -impl ObjectReference2 { - pub fn try_clone(&self) -> Result { - self.0.try_clone().map(|std_ref| Self(std_ref)) - } -} - impl Deref for ObjectReference2 { type Target = StdReference; @@ -125,21 +109,4 @@ mod tests { assert!(matches!(ds, ReferencedObject::Dataset(_))); }) } - - #[test] - fn test_standard_reference_clone() { - with_tmp_file(|file| { - let orig_group = file.create_group("g").unwrap(); - let ref1 = file.reference::("g").unwrap(); - let ref2 = ref1.try_clone().unwrap(); - - let ref2_group = ref2.dereference(&file).unwrap(); - match ref2_group { - crate::ReferencedObject::Group(g) => { - assert_eq!(g.name(), orig_group.name()) - } - _ => panic!("Wrong reference type."), - } - }) - } } From 4cb6c13bc741a2f86c9e503658a613db929947d3 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 08:42:48 +0100 Subject: [PATCH 12/17] refactor: some renames --- hdf5/src/hl/references/legacy.rs | 14 +++++++------- hdf5/src/hl/references/mod.rs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hdf5/src/hl/references/legacy.rs b/hdf5/src/hl/references/legacy.rs index 685b5747..3d92bd24 100644 --- a/hdf5/src/hl/references/legacy.rs +++ b/hdf5/src/hl/references/legacy.rs @@ -36,12 +36,12 @@ impl ObjectReference for ObjectReference1 { pointer.cast() } - fn create(dataspace: &Location, name: &str) -> Result { + fn create(location: &Location, name: &str) -> Result { let mut ref_out: std::mem::MaybeUninit = std::mem::MaybeUninit::uninit(); let name = to_cstring(name)?; h5call!(H5Rcreate( ref_out.as_mut_ptr().cast(), - dataspace.id(), + location.id(), name.as_ptr(), Self::REF_TYPE, -1 @@ -50,17 +50,17 @@ impl ObjectReference for ObjectReference1 { Ok(Self { inner: reference }) } - fn get_object_type(&self, dataset: &Location) -> Result { + fn get_object_type(&self, location: &Location) -> Result { let mut objtype = std::mem::MaybeUninit::::uninit(); - h5call!(H5Rget_obj_type2(dataset.id(), H5R_OBJECT1, self.ptr(), objtype.as_mut_ptr()))?; + h5call!(H5Rget_obj_type2(location.id(), H5R_OBJECT1, self.ptr(), objtype.as_mut_ptr()))?; let objtype = unsafe { objtype.assume_init() }; Ok(objtype) } - fn dereference(&self, dataset: &Location) -> Result { - let object_type = self.get_object_type(dataset)?; + fn dereference(&self, location: &Location) -> Result { + let object_type = self.get_object_type(location)?; let object_id = - h5call!(H5Rdereference(dataset.id(), H5P_DEFAULT, H5R_OBJECT1, self.ptr()))?; + h5call!(H5Rdereference(location.id(), H5P_DEFAULT, H5R_OBJECT1, self.ptr()))?; ReferencedObject::from_type_and_id(object_type, object_id) } } diff --git a/hdf5/src/hl/references/mod.rs b/hdf5/src/hl/references/mod.rs index 52592070..7f80f9ca 100644 --- a/hdf5/src/hl/references/mod.rs +++ b/hdf5/src/hl/references/mod.rs @@ -23,7 +23,7 @@ pub trait ObjectReference: Sized + H5Type { /// Create a new reference in the same structure as the location provided. fn create(location: &Location, name: &str) -> Result; - /// Dereference the object reference. + /// Dereference the object reference in the space provided. fn dereference(&self, location: &Location) -> Result; } /// The result of dereferencing an [object reference](ObjectReference). From 2d70d84837b405132b8fdb252bf12b543e1720b3 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 08:47:24 +0100 Subject: [PATCH 13/17] refactor: Move References to Module in H5Types --- hdf5-types/src/h5type.rs | 20 +------------------- hdf5-types/src/lib.rs | 4 ++-- hdf5-types/src/references.rs | 23 ++++++++++++++++++++++- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index 88f3bc9d..3cb50539 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -4,6 +4,7 @@ use std::os::raw::c_void; use std::ptr; use crate::array::VarLenArray; +use crate::references::Reference; use crate::string::{FixedAscii, FixedUnicode, VarLenAscii, VarLenUnicode}; #[allow(non_camel_case_types)] @@ -381,25 +382,6 @@ unsafe impl H5Type for VarLenUnicode { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub enum Reference { - Object, - Region, - #[cfg(feature = "1.12.0")] - Std, -} - -impl Reference { - fn size(self) -> usize { - match self { - Self::Object => mem::size_of::(), - Self::Region => mem::size_of::(), - #[cfg(feature = "1.12.0")] - Self::Std => mem::size_of::(), - } - } -} - #[cfg(test)] pub mod tests { use super::TypeDescriptor as TD; diff --git a/hdf5-types/src/lib.rs b/hdf5-types/src/lib.rs index 48176f4a..a1daa833 100644 --- a/hdf5-types/src/lib.rs +++ b/hdf5-types/src/lib.rs @@ -29,9 +29,9 @@ mod complex; pub use self::array::VarLenArray; pub use self::dyn_value::{DynValue, OwnedDynValue}; pub use self::h5type::{ - CompoundField, CompoundType, EnumMember, EnumType, FloatSize, H5Type, IntSize, Reference, - TypeDescriptor, + CompoundField, CompoundType, EnumMember, EnumType, FloatSize, H5Type, IntSize, TypeDescriptor, }; +pub use self::references::Reference; pub use self::string::{FixedAscii, FixedUnicode, StringError, VarLenAscii, VarLenUnicode}; pub(crate) unsafe fn malloc(n: usize) -> *mut core::ffi::c_void { diff --git a/hdf5-types/src/references.rs b/hdf5-types/src/references.rs index ba768139..a9649958 100644 --- a/hdf5-types/src/references.rs +++ b/hdf5-types/src/references.rs @@ -1 +1,22 @@ -// TODO: move references to this location +//! Types for references. + +use std::mem; + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum Reference { + Object, + Region, + #[cfg(feature = "1.12.0")] + Std, +} + +impl Reference { + pub fn size(self) -> usize { + match self { + Self::Object => mem::size_of::(), + Self::Region => mem::size_of::(), + #[cfg(feature = "1.12.0")] + Self::Std => mem::size_of::(), + } + } +} From 425f1874cd3dca5b4ed899749012ca40865b185b Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 09:20:53 +0100 Subject: [PATCH 14/17] fix: Gate ObjectReference2 on v1.12.1 There appear to be bugs in the v1.12.0 version which certainly break ObjectReference2 is an element in compound type. Other bugs were seen previously as well so this appears to be a more stable option. --- hdf5/src/hl/references/mod.rs | 4 +- hdf5/src/hl/references/standard.rs | 58 ++++++++++++---------------- hdf5/src/lib.rs | 2 +- hdf5/tests/test_object_references.rs | 12 +++--- 4 files changed, 34 insertions(+), 42 deletions(-) diff --git a/hdf5/src/hl/references/mod.rs b/hdf5/src/hl/references/mod.rs index 7f80f9ca..d0ac25be 100644 --- a/hdf5/src/hl/references/mod.rs +++ b/hdf5/src/hl/references/mod.rs @@ -3,14 +3,14 @@ use crate::Location; mod legacy; -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] mod standard; use hdf5_sys::h5o::H5O_type_t; use hdf5_sys::h5r::H5R_type_t; pub use legacy::ObjectReference1; -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] pub use standard::ObjectReference2; pub trait ObjectReference: Sized + H5Type { diff --git a/hdf5/src/hl/references/standard.rs b/hdf5/src/hl/references/standard.rs index 823c38ec..a7e045d8 100644 --- a/hdf5/src/hl/references/standard.rs +++ b/hdf5/src/hl/references/standard.rs @@ -1,5 +1,7 @@ -use std::ops::Deref; - +//! New standard reference types introduced in v1.12.0. +//! +//! These are gated on v1.12.1 since there appear to be multiple bugs in v1.12.0. +//! use hdf5_sys::h5o::H5O_type_t; use hdf5_sys::h5r::H5R_type_t::H5R_OBJECT2; use hdf5_sys::h5r::{H5R_ref_t, H5Rcreate_object, H5Rdestroy, H5Rget_obj_type3, H5Ropen_object}; @@ -10,28 +12,37 @@ use crate::Location; /// A reference to a HDF5 item that can be stored in attributes or datasets. #[repr(transparent)] -pub struct StdReference { - inner: H5R_ref_t, -} +pub struct StdReference(H5R_ref_t); impl StdReference { fn ptr(&self) -> *const H5R_ref_t { - std::ptr::addr_of!(self.inner) + std::ptr::addr_of!(self.0) } } -#[repr(transparent)] -#[derive(Debug)] -pub struct ObjectReference2(StdReference); +//todo: could we query some actual object parameters to make this more useful? +impl std::fmt::Debug for StdReference { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("StdReference") + } +} -impl Deref for ObjectReference2 { - type Target = StdReference; +unsafe impl H5Type for StdReference { + fn type_descriptor() -> hdf5_types::TypeDescriptor { + hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Std) + } +} - fn deref(&self) -> &Self::Target { - &self.0 +impl Drop for StdReference { + fn drop(&mut self) { + let _e = h5call!(H5Rdestroy(&mut self.0)); } } +#[repr(transparent)] +#[derive(Debug)] +pub struct ObjectReference2(StdReference); + impl ObjectReference for ObjectReference2 { const REF_TYPE: hdf5_sys::h5r::H5R_type_t = H5R_OBJECT2; @@ -41,7 +52,7 @@ impl ObjectReference for ObjectReference2 { fn create(location: &Location, name: &str) -> Result { let reference: H5R_ref_t = create_object_reference(location, name)?; - Ok(Self(StdReference { inner: reference })) + Ok(Self(StdReference(reference))) } fn get_object_type(&self, _location: &Location) -> Result { @@ -64,25 +75,6 @@ unsafe impl H5Type for ObjectReference2 { } } -//todo: could we query some actual object parameters to make this more useful? -impl std::fmt::Debug for StdReference { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("StdReference") - } -} - -unsafe impl H5Type for StdReference { - fn type_descriptor() -> hdf5_types::TypeDescriptor { - hdf5_types::TypeDescriptor::Reference(hdf5_types::Reference::Std) - } -} - -impl Drop for StdReference { - fn drop(&mut self) { - let _e = h5call!(H5Rdestroy(&mut self.inner)); - } -} - fn create_object_reference(dataset: &Location, name: &str) -> Result { let mut out: std::mem::MaybeUninit = std::mem::MaybeUninit::uninit(); let name = to_cstring(name)?; diff --git a/hdf5/src/lib.rs b/hdf5/src/lib.rs index 7dd4b5d6..12a1192b 100644 --- a/hdf5/src/lib.rs +++ b/hdf5/src/lib.rs @@ -67,7 +67,7 @@ mod export { }, }; - #[cfg(feature = "1.12.0")] + #[cfg(feature = "1.12.1")] pub use crate::hl::references::ObjectReference2; #[doc(hidden)] diff --git a/hdf5/tests/test_object_references.rs b/hdf5/tests/test_object_references.rs index 2b4c74ad..4b74c87e 100644 --- a/hdf5/tests/test_object_references.rs +++ b/hdf5/tests/test_object_references.rs @@ -4,7 +4,7 @@ mod common; use common::util::new_in_memory_file; -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] use hdf5::ObjectReference2; use hdf5::{H5Type, ObjectReference, ObjectReference1, ReferencedObject}; @@ -217,30 +217,30 @@ fn test_reference_in_datatype_object_reference1() { test_reference_in_datatype::(); } -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] #[test] fn test_group_references_with_objectreference2() { test_group_references::(); } -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] #[test] fn test_dataset_references_with_object_reference2() { test_dataset_references::(); } -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] #[test] fn test_reference_in_attribute_object_reference2() { test_reference_in_attribute::(); } -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] #[test] fn test_reference_errors_on_attribute_object_reference2() { test_reference_errors_on_attribute::(); } -#[cfg(feature = "1.12.0")] +#[cfg(feature = "1.12.1")] #[test] fn test_reference_in_datatype_object_reference2() { test_reference_in_datatype::(); From 24ab51df6f01f1ee300b808faa3743bdd88ad54e Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 10:14:04 +0100 Subject: [PATCH 15/17] backout VarArray non-copy changes --- hdf5-types/src/array.rs | 50 ++++++++++++++++++---------------------- hdf5-types/src/h5type.rs | 2 +- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/hdf5-types/src/array.rs b/hdf5-types/src/array.rs index 2bca73b2..cf090924 100644 --- a/hdf5-types/src/array.rs +++ b/hdf5-types/src/array.rs @@ -6,13 +6,29 @@ use std::ptr; use std::slice; #[repr(C)] -pub struct VarLenArray { +pub struct VarLenArray { len: usize, ptr: *const T, tag: PhantomData, } -impl VarLenArray { +impl VarLenArray { + pub unsafe fn from_parts(p: *const T, len: usize) -> Self { + let (len, ptr) = if !p.is_null() && len != 0 { + let dst = crate::malloc(len * mem::size_of::()); + ptr::copy_nonoverlapping(p, dst.cast(), len); + (len, dst) + } else { + (0, ptr::null_mut()) + }; + Self { len, ptr: ptr as *const _, tag: PhantomData } + } + + #[inline] + pub fn from_slice(arr: &[T]) -> Self { + unsafe { Self::from_parts(arr.as_ptr(), arr.len()) } + } + #[inline] pub fn as_ptr(&self) -> *const T { self.ptr @@ -30,31 +46,11 @@ impl VarLenArray { #[inline] pub fn as_slice(&self) -> &[T] { - unsafe { slice::from_raw_parts(self.as_ptr(), self.len()) } - } -} - -impl VarLenArray { - /// Creates a new `VarLenArray` from a slice by copying the source data. - #[inline] - pub fn from_slice(arr: &[T]) -> Self { - unsafe { Self::from_parts(arr.as_ptr(), arr.len()) } - } - - /// Create a new `VarLenArray` from a ptr/len combo by copying the source data. - pub unsafe fn from_parts(p: *const T, len: usize) -> Self { - let (len, ptr) = if !p.is_null() && len != 0 { - let dst = crate::malloc(len * mem::size_of::()); - ptr::copy_nonoverlapping(p, dst.cast(), len); - (len, dst) - } else { - (0, ptr::null_mut()) - }; - Self { len, ptr: ptr as *const _, tag: PhantomData } + self } } -impl Drop for VarLenArray { +impl Drop for VarLenArray { fn drop(&mut self) { if !self.ptr.is_null() { unsafe { @@ -75,7 +71,7 @@ impl Clone for VarLenArray { } } -impl Deref for VarLenArray { +impl Deref for VarLenArray { type Target = [T]; #[inline] @@ -83,7 +79,7 @@ impl Deref for VarLenArray { if self.len == 0 || self.ptr.is_null() { &[] } else { - self.as_slice() + unsafe { slice::from_raw_parts(self.as_ptr(), self.len()) } } } } @@ -139,7 +135,7 @@ impl PartialEq<[T; N]> for VarLenArray { } } -impl fmt::Debug for VarLenArray { +impl fmt::Debug for VarLenArray { #[inline] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.as_slice().fmt(f) diff --git a/hdf5-types/src/h5type.rs b/hdf5-types/src/h5type.rs index 3cb50539..251be5b2 100644 --- a/hdf5-types/src/h5type.rs +++ b/hdf5-types/src/h5type.rs @@ -347,7 +347,7 @@ unsafe impl H5Type for [T; N] { } } -unsafe impl H5Type for VarLenArray { +unsafe impl H5Type for VarLenArray { #[inline] fn type_descriptor() -> TypeDescriptor { TypeDescriptor::VarLenArray(Box::new(::type_descriptor())) From b975a21c7e60121fa763350ac8b58326ea8801b4 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 14 Apr 2024 10:21:33 +0100 Subject: [PATCH 16/17] fix: Fix API compatibility for ObjectReference1 --- hdf5/src/hl/references/legacy.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hdf5/src/hl/references/legacy.rs b/hdf5/src/hl/references/legacy.rs index 3d92bd24..be5d638c 100644 --- a/hdf5/src/hl/references/legacy.rs +++ b/hdf5/src/hl/references/legacy.rs @@ -59,8 +59,11 @@ impl ObjectReference for ObjectReference1 { fn dereference(&self, location: &Location) -> Result { let object_type = self.get_object_type(location)?; + #[cfg(feature = "1.10.0")] let object_id = h5call!(H5Rdereference(location.id(), H5P_DEFAULT, H5R_OBJECT1, self.ptr()))?; + #[cfg(not(feature = "1.10.0"))] + let object_id = h5call!(H5Rdereference(location.id(), H5R_OBJECT1, self.ptr()))?; ReferencedObject::from_type_and_id(object_type, object_id) } } From fdb5edc7deaaab7212e247ed2e880841de520953 Mon Sep 17 00:00:00 2001 From: James McNally Date: Sun, 21 Apr 2024 09:58:46 +0100 Subject: [PATCH 17/17] Update hdf5-types/src/dyn_value.rs Co-authored-by: Magnus Ulimoen Signed-off-by: James McNally --- hdf5-types/src/dyn_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hdf5-types/src/dyn_value.rs b/hdf5-types/src/dyn_value.rs index 23a09357..cc94a7d1 100644 --- a/hdf5-types/src/dyn_value.rs +++ b/hdf5-types/src/dyn_value.rs @@ -703,7 +703,7 @@ impl<'a> DynValue<'a> { FixedUnicode(_) => DynFixedString::new(buf, true).into(), VarLenAscii => DynVarLenString::new(buf, false).into(), VarLenUnicode => DynVarLenString::new(buf, true).into(), - Reference(_x) => todo!(), + Reference(_x) => todo!("References are not yet supported as fill value"), } } }