Skip to content

Commit 67d27f2

Browse files
committed
cleanup: noexcept code
The idea is to have make moving noexcept. The twist is that move assignment requires decrementing a reference count. Which in turn might require freeing the object, which could fail. Hence move assignment is not no-except for any objects that own a HID. It's still worth implementing since we can steal the reference of `other`. All other object should behave normally, i.e. both move construction and assignment are noexcept. This commit changes two addition occurences of `noexcept`: * Add `noexcept` to `Object(hid_t)`, because it semantically steals the HID and it can't check anything. Moreover, it's useful for move ctors/assignment. * Remove `noexcept` from `Chunking::getDimensions` for consistency and freedom to change the implementation later. Outdated occurences of `throw()` have been modernized. There's several occurences of `noexcept` in accessors of `ObjectInfo`. They're not changed, because we probably want to have different objects for different versions of `H5O_info_t` (since that struct dictates what's available).
1 parent a915da8 commit 67d27f2

File tree

9 files changed

+127
-8
lines changed

9 files changed

+127
-8
lines changed

include/highfive/H5DataType.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,9 @@ class CompoundType: public DataType {
212212
};
213213

214214
CompoundType(const CompoundType& other) = default;
215+
CompoundType(CompoundType&& other) noexcept = default;
216+
CompoundType& operator=(const CompoundType& other) = default;
217+
CompoundType& operator=(CompoundType&& other) = default;
215218

216219
///
217220
/// \brief Initializes a compound type from a vector of member definitions

include/highfive/H5Exception.hpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,19 @@ class Exception: public std::exception {
2525
Exception(const std::string& err_msg)
2626
: _errmsg(err_msg) {}
2727

28-
~Exception() throw() override = default;
28+
Exception(const Exception& other) = default;
29+
Exception(Exception&& other) noexcept = default;
30+
31+
Exception& operator=(const Exception& other) = default;
32+
Exception& operator=(Exception&& other) noexcept = default;
33+
34+
~Exception() noexcept override {}
2935

3036
///
3137
/// \brief get the current exception error message
3238
/// \return
3339
///
34-
inline const char* what() const throw() override {
40+
inline const char* what() const noexcept override {
3541
return _errmsg.c_str();
3642
}
3743

include/highfive/H5File.hpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,23 @@ class File: public Object, public NodeTraits<File>, public AnnotateTraits<File>
7979
const FileCreateProps& fileCreateProps,
8080
const FileAccessProps& fileAccessProps = FileAccessProps::Default());
8181

82+
/// \brief Keeps reference count constant, and invalidates other.
83+
File(File&& other) noexcept = default;
84+
85+
/// \brief Keeps reference count constant, and invalidates other.
86+
File& operator=(File&& other) = default;
87+
88+
/// \brief Increments reference count, keeps other valid.
89+
File(const File& other) = default;
90+
91+
/// \brief Increments reference count, keeps other valid.
92+
File& operator=(const File& other) = default;
93+
8294
///
8395
/// \brief Return the name of the file
8496
///
8597
const std::string& getName() const;
8698

87-
8899
/// \brief Object path of a File is always "/"
89100
std::string getPath() const noexcept {
90101
return "/";

include/highfive/H5Object.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,14 @@ class Object {
7575
Object(const Object& other);
7676

7777
// Init with an low-level object id
78-
explicit Object(hid_t);
78+
explicit Object(hid_t) noexcept;
7979

8080
// decrease reference counter
8181
~Object();
8282

8383
// Copy-Assignment operator
8484
Object& operator=(const Object& other);
85+
Object& operator=(Object&& other);
8586

8687
hid_t _hid;
8788

include/highfive/H5PropertyList.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ class Chunking {
506506

507507
explicit Chunking(DataSetCreateProps& plist, size_t max_dims = 32);
508508

509-
const std::vector<hsize_t>& getDimensions() const noexcept;
509+
const std::vector<hsize_t>& getDimensions() const;
510510

511511
private:
512512
friend DataSetCreateProps;

include/highfive/bits/H5Object_misc.hpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace HighFive {
1919
inline Object::Object()
2020
: _hid(H5I_INVALID_HID) {}
2121

22-
inline Object::Object(hid_t hid)
22+
inline Object::Object(hid_t hid) noexcept
2323
: _hid(hid) {}
2424

2525
inline Object::Object(const Object& other)
@@ -34,6 +34,17 @@ inline Object::Object(Object&& other) noexcept
3434
other._hid = H5I_INVALID_HID;
3535
}
3636

37+
inline Object& Object::operator=(Object&& other) {
38+
if (this != &other) {
39+
if ((*this).isValid()) {
40+
detail::h5i_dec_ref(_hid);
41+
}
42+
_hid = other._hid;
43+
other._hid = H5I_INVALID_HID;
44+
}
45+
return *this;
46+
}
47+
3748
inline Object& Object::operator=(const Object& other) {
3849
if (this != &other) {
3950
if ((*this).isValid()) {

include/highfive/bits/H5PropertyList_misc.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ inline Chunking::Chunking(DataSetCreateProps& plist, size_t max_dims)
296296
}
297297
}
298298

299-
inline const std::vector<hsize_t>& Chunking::getDimensions() const noexcept {
299+
inline const std::vector<hsize_t>& Chunking::getDimensions() const {
300300
return _dims;
301301
}
302302

tests/unit/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ if(MSVC)
66
endif()
77

88
## Base tests
9-
foreach(test_name tests_high_five_base tests_high_five_easy test_all_types test_high_five_selection tests_high_five_data_type test_boost test_empty_arrays test_legacy test_opencv test_string test_stl test_xtensor)
9+
foreach(test_name tests_high_five_base tests_high_five_easy test_all_types test_high_five_selection tests_high_five_data_type test_boost test_empty_arrays test_legacy test_nothrow_movable test_opencv test_string test_stl test_xtensor)
1010
add_executable(${test_name} "${test_name}.cpp")
1111
target_link_libraries(${test_name} HighFive HighFiveWarnings HighFiveFlags Catch2::Catch2WithMain)
1212
target_link_libraries(${test_name} HighFiveOptionalDependencies)
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright (c), 2025, HighFive Developers
3+
*
4+
* Distributed under the Boost Software License, Version 1.0.
5+
* (See accompanying file LICENSE_1_0.txt or copy at
6+
* http://www.boost.org/LICENSE_1_0.txt)
7+
*
8+
*/
9+
10+
#include <catch2/catch_template_test_macros.hpp>
11+
12+
#include <highfive/highfive.hpp>
13+
14+
using namespace HighFive;
15+
16+
template<typename T, bool T1, bool T2, bool T3, bool T4>
17+
constexpr void check_constructible_assignable() {
18+
static_assert(std::is_copy_constructible_v<T> == T1);
19+
static_assert(std::is_copy_assignable_v<T> == T2);
20+
static_assert(std::is_move_constructible_v<T> == T3);
21+
static_assert(std::is_move_assignable_v<T> == T4);
22+
}
23+
24+
TEST_CASE("Enshrine constructible/assignable status", "[core]") {
25+
// Object isn't constructible at all, because its dtor has
26+
// been deleted.
27+
check_constructible_assignable<Object, false, false, false, false>();
28+
check_constructible_assignable<File, true, true, true, true>();
29+
check_constructible_assignable<CompoundType, true, true, true, true>();
30+
}
31+
32+
template<class T>
33+
constexpr void
34+
check_nothrow_movable() {
35+
// Check that if it's moveable, it's nothrow movable.
36+
static_assert(std::is_move_constructible_v<T> == std::is_nothrow_move_constructible_v<T>);
37+
static_assert(std::is_move_assignable_v<T> == std::is_nothrow_move_assignable_v<T>);
38+
}
39+
40+
TEST_CASE("Nothrow movable exceptions", "[core]") {
41+
check_nothrow_movable<Exception>();
42+
check_nothrow_movable<FileException>();
43+
check_nothrow_movable<ObjectException>();
44+
check_nothrow_movable<AttributeException>();
45+
check_nothrow_movable<DataSpaceException>();
46+
check_nothrow_movable<DataSetException>();
47+
check_nothrow_movable<GroupException>();
48+
check_nothrow_movable<PropertyException>();
49+
check_nothrow_movable<ReferenceException>();
50+
check_nothrow_movable<DataTypeException>();
51+
}
52+
53+
template<class T>
54+
constexpr void
55+
check_nothrow_object() {
56+
// HighFive objects are reference counted. During move assignment, the
57+
// reference count can drop to zero, triggering freeing of the object, which
58+
// can fail. Hence, move assignment can file, but move construction shouldn't.
59+
static_assert(std::is_move_constructible_v<T> == std::is_nothrow_move_constructible_v<T>);
60+
static_assert(!std::is_nothrow_move_assignable_v<T>);
61+
static_assert(!std::is_nothrow_copy_constructible_v<T>);
62+
static_assert(!std::is_nothrow_copy_assignable_v<T>);
63+
}
64+
65+
TEST_CASE("HighFive::Objects are nothrow move constructible", "[core]") {
66+
check_nothrow_object<Object>();
67+
check_nothrow_object<File>();
68+
check_nothrow_object<Attribute>();
69+
check_nothrow_object<DataSpace>();
70+
check_nothrow_object<DataSet>();
71+
check_nothrow_object<Group>();
72+
check_nothrow_object<DataType>();
73+
check_nothrow_object<Selection>();
74+
}
75+
76+
TEST_CASE("Regular HighFive objects are nothrow movable", "[core]") {
77+
check_nothrow_movable<RegularHyperSlab>();
78+
check_nothrow_movable<HyperSlab>();
79+
check_nothrow_movable<ElementSet>();
80+
check_nothrow_movable<ProductSet>();
81+
}
82+
83+
TEST_CASE("Nothrow movable data types", "[core]") {
84+
check_nothrow_object<AtomicType<double>>();
85+
check_nothrow_object<CompoundType>();
86+
check_nothrow_object<StringType>();
87+
}

0 commit comments

Comments
 (0)