cleanup: explicit ctors. #58
Conversation
| using Object::Object; // bring DataSet(hid_t) | ||
|
|
||
| DataSet(Object&& o) noexcept | ||
| explicit DataSet(Object&& o) noexcept |
There was a problem hiding this comment.
OK, because we definitely don't want to silently convert any Object to DataSet.
| /// \brief Create a variable length string HDF5 datatype. | ||
| /// | ||
| VariableLengthStringType(CharacterSet character_set = CharacterSet::Ascii); | ||
| explicit VariableLengthStringType(CharacterSet character_set = CharacterSet::Ascii); |
There was a problem hiding this comment.
OK, because conversion seems unexpected.
| /// \brief Initializes a compound type from a DataType | ||
| /// \param type | ||
| inline CompoundType(DataType&& type) | ||
| inline explicit CompoundType(DataType&& type) |
There was a problem hiding this comment.
OK, because not every DataType is a CompoundType. Hence, no conversion.
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
| } | ||
|
|
||
| Group(Object&& o) noexcept | ||
| explicit Group(Object&& o) noexcept |
There was a problem hiding this comment.
OK, because silently converting any Object to Group is unexpected.
| class SilenceHDF5 { | ||
| public: | ||
| inline SilenceHDF5(bool enable = true) | ||
| inline explicit SilenceHDF5(bool enable = true) |
There was a problem hiding this comment.
OK, because bool isn't related to the concept of silencing HDF5 error output.
|
|
||
| struct HighFiveIterateData { | ||
| inline HighFiveIterateData(std::vector<std::string>& my_names) | ||
| explicit HighFiveIterateData(std::vector<std::string>& my_names) |
| /// \param list List of continuous coordinates (e.g.: in 2 dimensions space | ||
| /// `ElementSet{1, 2, 3 ,4}` creates points `(1, 2)` and `(3, 4)`). | ||
| explicit ElementSet(std::initializer_list<std::size_t> list); | ||
| ElementSet(std::initializer_list<std::size_t> list); |
There was a problem hiding this comment.
Probably OK, because more permissive, meaning it shouldn't break anything and:
void foo(ElementSet);
foo({1, 2, 3});
seems fair.
| const std::vector<size_t>& count_ = {}, | ||
| const std::vector<size_t>& stride_ = {}, | ||
| const std::vector<size_t>& block_ = {}) | ||
| explicit RegularHyperSlab(const std::vector<size_t>& offset_, |
There was a problem hiding this comment.
OK, because an std::vector<size_t> isn't tightly related to RegularHyperSlab.
| class HyperCube { | ||
| public: | ||
| HyperCube(size_t rank) | ||
| explicit HyperCube(size_t rank) |
| /// | ||
| /// \param list List of N-dim points. | ||
| explicit ElementSet(std::initializer_list<std::vector<std::size_t>> list); | ||
| ElementSet(std::initializer_list<std::vector<std::size_t>> list); |
There was a problem hiding this comment.
Probably OK, see above.
| class Exception: public std::exception { | ||
| public: | ||
| Exception(const std::string& err_msg) | ||
| explicit Exception(const std::string& err_msg) |
There was a problem hiding this comment.
Undecided: std::runtime_error allows conversion from std::string (and char*).
include/highfive/H5DataType.hpp
Outdated
| /// \param t_members | ||
| /// \param size | ||
| inline CompoundType(const std::vector<member_def>& t_members, size_t size = 0) | ||
| inline explicit CompoundType(const std::vector<member_def>& t_members, size_t size = 0) |
There was a problem hiding this comment.
Undecided: member_def is tightly linked to CompoundType. Hence, maybe not worth breaking user code over something that's unlikely to be very surprising.
There was a problem hiding this comment.
I'll revert, not worth breaking anyone's code over this.
include/highfive/H5DataType.hpp
Outdated
| EnumType(const EnumType& other) = default; | ||
|
|
||
| EnumType(const std::vector<member_def>& t_members) | ||
| explicit EnumType(const std::vector<member_def>& t_members) |
There was a problem hiding this comment.
Undecided: member_def is tightly linked to EnumType.
There was a problem hiding this comment.
I'll revert, same argument.
1uc
force-pushed
the
1uc/explicit-ctors
branch
2 times, most recently
from
8ae74e5 to
0bd4945
Compare
This commit follows applies the google-explicit-constructor rules as the
default:
* all ctors are explicit,
* except ctors using the initializer list, those are never
explicit.
Exceptions are made for cases where conversion has little risk of being
bugprone, e.g. because the type of the argument of the ctor is closely
linked to the object being constructed.
Apply clang-tidy fixes for google-explicit-constructor. This prevents implicit conversion and allows passing initializer list to function, to create a temporary argument from the initializer list.