Remove deprecated default ctor for DataSet (#947)

* Remove deprecated default ctor for DataSet

* Allow default ctors.

* Test that default constructed objects throw. (#975)

The only valid operations of default constructed DataSets and Groups is
to assign a valid object to them.

Therefore, calling any other methods should raise an exception.

---------

Co-authored-by: Luc Grosheintz <[email protected]>

include/highfive/H5DataSet.hpp

@@ -98,8 +98,6 @@ class DataSet: public Object,
         return details::get_plist<DataSetAccessProps>(*this, H5Dget_access_plist);
     }
 
-    /// \deprecated Default constructor creates unsafe uninitialized objects
-    H5_DEPRECATED("Default constructor creates unsafe uninitialized objects")
     DataSet() = default;
 
   protected:

include/highfive/bits/H5Attribute_misc.hpp

@@ -31,6 +31,10 @@ inline std::string Attribute::getName() const {
 }
 
 inline size_t Attribute::getStorageSize() const {
+    if (!this->isValid()) {
+        throw AttributeException("Invalid call to `DataSet::getFile` for invalid object");
+    }
+
     return static_cast<size_t>(detail::h5a_get_storage_size(_hid));
 }
 

include/highfive/bits/H5DataSet_misc.hpp

@@ -22,6 +22,10 @@
 namespace HighFive {
 
 inline uint64_t DataSet::getStorageSize() const {
+    if (!this->isValid()) {
+        throw DataSetException("Invalid call to `DataSet::getStorageSize` for invalid object");
+    }
+
     return detail::h5d_get_storage_size(_hid);
 }
 

include/highfive/bits/H5Path_traits.hpp

@@ -25,8 +25,7 @@ class PathTraits {
     ///
     /// \brief Return a reference to the File object this object belongs
     /// \return the File object ref
-    File& getFile() const noexcept;
-
+    File& getFile() const;
 
   protected:
     std::shared_ptr<File> _file_obj;  // keep a ref to file so we keep its ref count > 0

include/highfive/bits/H5Path_traits_misc.hpp

@@ -35,7 +35,11 @@ inline std::string PathTraits<Derivate>::getPath() const {
 }
 
 template <typename Derivate>
-inline File& PathTraits<Derivate>::getFile() const noexcept {
+inline File& PathTraits<Derivate>::getFile() const {
+    const auto& obj = static_cast<const Derivate&>(*this);
+    if (!obj.isValid()) {
+        throw ObjectException("Invalid call to `PathTraits::getFile` for invalid object");
+    }
     return *_file_obj;
 }
 

tests/unit/tests_high_five_base.cpp

@@ -6,6 +6,7 @@
  *          http://www.boost.org/LICENSE_1_0.txt)
  *
  */
+#include <H5Ipublic.h>
 #include <algorithm>
 #include <cstdio>
 #include <cstdlib>
@@ -321,27 +322,154 @@ TEST_CASE("Test allocation time") {
     CHECK(alloc_size == data.size() * sizeof(decltype(data)::value_type));
 }
 
-/*
- * Test to ensure legacy support: DataSet used to have a default constructor.
- * However, it is not useful to have a DataSet object that does not actually
- * refer to a dataset in a file. Hence, the the default constructor was
- * deprecated.
- * This test is to ensure that the constructor is not accidentally removed and
- * thereby break users' code.
- */
-TEST_CASE("Test default constructors") {
-    const std::string file_name("h5_default_ctors.h5");
-    const std::string dataset_name("dset");
-    File file(file_name, File::Truncate);
-    auto ds = file.createDataSet(dataset_name, std::vector<int>{1, 2, 3, 4, 5});
+template <class T>
+void check_invalid_hid_Object(T& obj) {
+    auto silence = SilenceHDF5();
+
+    CHECK(!obj.isValid());
+    CHECK(obj.getId() == H5I_INVALID_HID);
+
+    CHECK_THROWS(obj.getInfo());
+    CHECK_THROWS(obj.getType());
+}
 
-    DataSet d2;  // expect deprecation warning, as it constructs unsafe object
-    // d2.getFile();  // runtime error
-    CHECK(!d2.isValid());
-    d2 = ds;  // copy
-    CHECK(d2.isValid());
+template <class T, class U>
+void check_invalid_hid_NodeTraits(T& obj, const U& linkable) {
+    auto silence = SilenceHDF5();
+
+    auto data_space = DataSpace{2, 3};
+    auto data_type = HighFive::create_datatype<double>();
+    auto data = std::vector<double>{1.0, 2.0, 3.0};
+    auto gcpl = GroupCreateProps();
+
+    CHECK_THROWS(obj.createDataSet("foo", data_space, data_type));
+    CHECK_THROWS(obj.template createDataSet<double>("foo", data_space));
+    CHECK_THROWS(obj.createDataSet("foo", data));
+
+    CHECK_THROWS(obj.getDataSet("foo"));
+    CHECK_THROWS(obj.createGroup("foo"));
+    CHECK_THROWS(obj.createGroup("foo", gcpl));
+    CHECK_THROWS(obj.getGroup("foo"));
+    CHECK_THROWS(obj.getDataType("foo"));
+    CHECK_THROWS(obj.getNumberObjects());
+    CHECK_THROWS(obj.getObjectName(0));
+    CHECK_THROWS(obj.rename("foo", "bar"));
+    CHECK_THROWS(obj.listObjectNames());
+    CHECK_THROWS(obj.exist("foo"));
+    CHECK_THROWS(obj.unlink("foo"));
+    CHECK_THROWS(obj.getLinkType("foo"));
+    CHECK_THROWS(obj.getObjectType("foo"));
+    CHECK_THROWS(obj.createSoftLink("foo", linkable));
+    CHECK_THROWS(obj.createSoftLink("foo", "bar"));
+    CHECK_THROWS(obj.createExternalLink("foo", "bar", "baz"));
+    CHECK_THROWS(obj.createHardLink("foo", linkable));
 }
 
+template <class T>
+void check_invalid_hid_DataSet(T& obj) {
+    auto silence = SilenceHDF5();
+
+    CHECK_THROWS(obj.getStorageSize());
+    CHECK_THROWS(obj.getOffset());
+    CHECK_THROWS(obj.getMemSpace());
+    CHECK_THROWS(obj.resize({1, 2, 3}));
+    CHECK_THROWS(obj.getDimensions());
+    CHECK_THROWS(obj.getElementCount());
+    CHECK_THROWS(obj.getCreatePropertyList());
+    CHECK_THROWS(obj.getAccessPropertyList());
+}
+
+template <class T>
+void check_invalid_hid_SliceTraits(T& obj) {
+    auto silence = SilenceHDF5();
+
+    auto slab = HighFive::HyperSlab(RegularHyperSlab({0}));
+    auto space = DataSpace{3};
+    auto set = ElementSet{0, 1, 3};
+    auto data = std::vector<double>{1.0, 2.0, 3.0};
+    auto type = create_datatype<double>();
+    auto cols = std::vector<size_t>{0, 2, 3};
+
+    CHECK_THROWS(obj.select(slab));
+    CHECK_THROWS(obj.select(slab, space));
+    CHECK_THROWS(obj.select({0}, {3}));
+    CHECK_THROWS(obj.select(cols));
+    CHECK_THROWS(obj.select(set));
+
+    CHECK_THROWS(obj.template read<double>());
+    CHECK_THROWS(obj.read(data));
+    CHECK_THROWS(obj.read_raw(data.data(), type));
+    CHECK_THROWS(obj.template read_raw<double>(data.data()));
+
+    CHECK_THROWS(obj.write(data));
+    CHECK_THROWS(obj.write_raw(data.data(), type));
+    CHECK_THROWS(obj.template write_raw<double>(data.data()));
+}
+
+template <class T>
+void check_invalid_hid_PathTraits(T& obj) {
+    auto silence = SilenceHDF5();
+
+    CHECK_THROWS(obj.getPath());
+    CHECK_THROWS(obj.getFile());
+}
+
+template <class T>
+void check_invalid_hid_AnnotateTraits(T& obj) {
+    auto silence = SilenceHDF5();
+
+    auto space = DataSpace{3};
+    auto data = std::vector<double>{1.0, 2.0, 3.0};
+    auto type = create_datatype<double>();
+
+    CHECK_THROWS(obj.createAttribute("foo", space, type));
+    CHECK_THROWS(obj.template createAttribute<double>("foo", space));
+    CHECK_THROWS(obj.createAttribute("foo", data));
+
+    CHECK_THROWS(obj.deleteAttribute("foo"));
+    CHECK_THROWS(obj.getAttribute("foo"));
+    CHECK_THROWS(obj.getNumberAttributes());
+    CHECK_THROWS(obj.listAttributeNames());
+    CHECK_THROWS(obj.hasAttribute("foo"));
+}
+
+template <class T>
+void check_invalid_hid_Group(T& obj) {
+    auto silence = SilenceHDF5();
+
+    CHECK_THROWS(obj.getEstimatedLinkInfo());
+    CHECK_THROWS(obj.getCreatePropertyList());
+}
+
+TEST_CASE("Test default DataSet constructor") {
+    DataSet ds;
+    check_invalid_hid_Object(ds);
+    check_invalid_hid_DataSet(ds);
+    check_invalid_hid_SliceTraits(ds);
+    check_invalid_hid_AnnotateTraits(ds);
+    check_invalid_hid_PathTraits(ds);
+
+    File file("h5_default_dset_ctor.h5", File::Truncate);
+    ds = file.createDataSet("dset", std::vector<int>{1, 2, 3, 4, 5});
+    CHECK(ds.isValid());
+}
+
+TEST_CASE("Test default Group constructor") {
+    File file("h5_default_group_ctor.h5", File::Truncate);
+    Group linkable = file.createGroup("bar");
+
+    Group grp;
+    check_invalid_hid_Object(grp);
+    check_invalid_hid_NodeTraits(grp, linkable);
+    check_invalid_hid_AnnotateTraits(grp);
+    check_invalid_hid_PathTraits(grp);
+
+    grp = file.createGroup("grp");
+
+    CHECK(grp.isValid());
+}
+
+
 TEST_CASE("Test groups and datasets") {
     const std::string file_name("h5_group_test.h5");
     const std::string dataset_name("dset");