Skip to content

Commit

Permalink
Avoid templated friend classes. (#688)
Browse files Browse the repository at this point in the history
There seems to be a compiler bug in numerous version of GCC which
prevent code such as:

    class Foo {
      template<class Derived>
      friend class SomeCRTP<Derived>;
    };

This commit removed all such constructs from HighFive, by allowing a
function `detail::make_*` to create `Object`, `Selection` and `Group`
from an `hid_t`.
  • Loading branch information
1uc committed Feb 25, 2023
1 parent bc71590 commit 20be06f
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 32 deletions.
29 changes: 27 additions & 2 deletions include/highfive/H5Attribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,26 @@
namespace HighFive {
class DataSpace;

namespace detail {

/// \brief Internal hack to create an `Attribute` from an ID.
///
/// WARNING: Creating an Attribute from an ID has implications w.r.t. the lifetime of the object
/// that got passed via its ID. Using this method careless opens up the suite of issues
/// related to C-style resource management, including the analog of double free, dangling
/// pointers, etc.
///
/// NOTE: This is not part of the API and only serves to work around a compiler issue in GCC which
/// prevents us from using `friend`s instead. This function should only be used for internal
/// purposes. The problematic construct is:
///
/// template<class Derived>
/// friend class SomeCRTP<Derived>;
///
/// \private
Attribute make_attribute(hid_t hid);
} // namespace detail

///
/// \brief Class representing an attribute of a dataset or group
///
Expand Down Expand Up @@ -95,8 +115,13 @@ class Attribute: public Object, public PathTraits<Attribute> {
private:
using Object::Object;

template <typename Derivate>
friend class ::HighFive::AnnotateTraits;
friend Attribute detail::make_attribute(hid_t);
};

namespace detail {
inline Attribute make_attribute(hid_t hid) {
return Attribute(hid);
}
} // namespace detail

} // namespace HighFive
34 changes: 29 additions & 5 deletions include/highfive/H5Group.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,25 @@

namespace HighFive {

namespace detail {
/// \brief Internal hack to create an `Group` from an ID.
///
/// WARNING: Creating an Group from an ID has implications w.r.t. the lifetime of the object
/// that got passed via its ID. Using this method careless opens up the suite of issues
/// related to C-style resource management, including the analog of double free, dangling
/// pointers, etc.
///
/// NOTE: This is not part of the API and only serves to work around a compiler issue in GCC which
/// prevents us from using `friend`s instead. This function should only be used for internal
/// purposes. The problematic construct is:
///
/// template<class Derived>
/// friend class SomeCRTP<Derived>;
///
/// \private
Group make_group(hid_t);
} // namespace detail

///
/// \brief Represents an hdf5 group
class Group: public Object,
Expand All @@ -38,16 +57,15 @@ class Group: public Object,
return details::get_plist<GroupCreateProps>(*this, H5Gget_create_plist);
}

protected:
using Object::Object;

Group(Object&& o) noexcept
: Object(std::move(o)){};

protected:
using Object::Object;

friend Group detail::make_group(hid_t);
friend class File;
friend class Reference;
template <typename Derivate>
friend class ::HighFive::NodeTraits;
};

inline std::pair<unsigned int, unsigned int> Group::getEstimatedLinkInfo() const {
Expand All @@ -56,4 +74,10 @@ inline std::pair<unsigned int, unsigned int> Group::getEstimatedLinkInfo() const
return std::make_pair(eli.getEntries(), eli.getNameLength());
}

namespace detail {
inline Group make_group(hid_t hid) {
return Group(hid);
}
} // namespace detail

} // namespace HighFive
32 changes: 23 additions & 9 deletions include/highfive/H5Object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,31 @@ enum class ObjectType {
Other // Internal/custom object type
};

namespace detail {
/// \brief Internal hack to create an `Object` from an ID.
///
/// WARNING: Creating an Object from an ID has implications w.r.t. the lifetime of the object
/// that got passed via its ID. Using this method careless opens up the suite of issues
/// related to C-style resource management, including the analog of double free, dangling
/// pointers, etc.
///
/// NOTE: This is not part of the API and only serves to work around a compiler issue in GCC which
/// prevents us from using `friend`s instead. This function should only be used for internal
/// purposes. The problematic construct is:
///
/// template<class Derived>
/// friend class SomeCRTP<Derived>;
///
/// \private
Object make_object(hid_t hid);
} // namespace detail


class Object {
public:
// move constructor, reuse hid
Object(Object&& other) noexcept;

// decrease reference counter
~Object();

Expand Down Expand Up @@ -73,9 +95,6 @@ class Object {
// copy constructor, increase reference counter
Object(const Object& other);

// move constructor, reuse hid
Object(Object&& other) noexcept;

// Init with an low-level object id
explicit Object(hid_t);

Expand All @@ -85,14 +104,9 @@ class Object {
hid_t _hid;

private:
template <typename Derivate>
friend class NodeTraits;
template <typename Derivate>
friend class AnnotateTraits;
friend Object detail::make_object(hid_t);
friend class Reference;
friend class CompoundType;
template <typename Derivate>
friend class PathTraits;
};


Expand Down
8 changes: 5 additions & 3 deletions include/highfive/H5Selection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

namespace HighFive {

namespace detail {
Selection make_selection(const DataSpace&, const DataSpace&, const DataSet&);
}

///
/// \brief Selection: represent a view on a slice/part of a dataset
///
Expand Down Expand Up @@ -52,9 +56,7 @@ class Selection: public SliceTraits<Selection> {
DataSpace _mem_space, _file_space;
DataSet _set;

template <typename Derivate>
friend class ::HighFive::SliceTraits;
// absolute namespace naming due to GCC bug 52625
friend Selection detail::make_selection(const DataSpace&, const DataSpace&, const DataSet&);
};

} // namespace HighFive
8 changes: 4 additions & 4 deletions include/highfive/bits/H5Annotate_traits_misc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ inline Attribute AnnotateTraits<Derivate>::createAttribute(const std::string& at
const DataType& dtype) {
auto attr_id = H5Acreate2(static_cast<Derivate*>(this)->getId(),
attribute_name.c_str(),
dtype._hid,
space._hid,
dtype.getId(),
space.getId(),
H5P_DEFAULT,
H5P_DEFAULT);
if (attr_id < 0) {
HDF5ErrMapper::ToException<AttributeException>(
std::string("Unable to create the attribute \"") + attribute_name + "\":");
}
return Attribute(attr_id);
return detail::make_attribute(attr_id);
}

template <typename Derivate>
Expand Down Expand Up @@ -71,7 +71,7 @@ inline Attribute AnnotateTraits<Derivate>::getAttribute(const std::string& attri
HDF5ErrMapper::ToException<AttributeException>(
std::string("Unable to open the attribute \"") + attribute_name + "\":");
}
return Attribute(attr_id);
return detail::make_attribute(attr_id);
}

template <typename Derivate>
Expand Down
12 changes: 6 additions & 6 deletions include/highfive/bits/H5Node_traits_misc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ inline DataSet NodeTraits<Derivate>::createDataSet(const std::string& dataset_na
lcpl.add(CreateIntermediateGroup(parents));
const auto hid = H5Dcreate2(static_cast<Derivate*>(this)->getId(),
dataset_name.c_str(),
dtype._hid,
space._hid,
dtype.getId(),
space.getId(),
lcpl.getId(),
createProps.getId(),
accessProps.getId());
Expand Down Expand Up @@ -142,7 +142,7 @@ inline Group NodeTraits<Derivate>::createGroup(const std::string& group_name, bo
HDF5ErrMapper::ToException<GroupException>(std::string("Unable to create the group \"") +
group_name + "\":");
}
return Group(hid);
return detail::make_group(hid);
}

template <typename Derivate>
Expand All @@ -160,7 +160,7 @@ inline Group NodeTraits<Derivate>::createGroup(const std::string& group_name,
HDF5ErrMapper::ToException<GroupException>(std::string("Unable to create the group \"") +
group_name + "\":");
}
return Group(hid);
return detail::make_group(hid);
}

template <typename Derivate>
Expand All @@ -171,7 +171,7 @@ inline Group NodeTraits<Derivate>::getGroup(const std::string& group_name) const
HDF5ErrMapper::ToException<GroupException>(std::string("Unable to open the group \"") +
group_name + "\":");
}
return Group(hid);
return detail::make_group(hid);
}

template <typename Derivate>
Expand Down Expand Up @@ -369,7 +369,7 @@ inline Object NodeTraits<Derivate>::_open(const std::string& node_name,
HDF5ErrMapper::ToException<GroupException>(std::string("Unable to open \"") + node_name +
"\":");
}
return Object(id);
return detail::make_object(id);
}


Expand Down
6 changes: 6 additions & 0 deletions include/highfive/bits/H5Object_misc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
#include "highfive/H5Utility.hpp"

namespace HighFive {
namespace detail {
inline Object make_object(hid_t hid) {
return Object(hid);
}
} // namespace detail


inline Object::Object()
: _hid(H5I_INVALID_HID) {}
Expand Down
8 changes: 8 additions & 0 deletions include/highfive/bits/H5Selection_misc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,12 @@ inline const DataType Selection::getDataType() const {
return _set.getDataType();
}

namespace detail {
inline Selection make_selection(const DataSpace& mem_space,
const DataSpace& file_space,
const DataSet& set) {
return Selection(mem_space, file_space, set);
}
} // namespace detail

} // namespace HighFive
6 changes: 3 additions & 3 deletions include/highfive/bits/H5Slice_traits_misc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ inline Selection SliceTraits<Derivate>::select_impl(const HyperSlab& hyperslab,
const auto& slice = static_cast<const Derivate&>(*this);
auto filespace = hyperslab.apply(slice.getSpace());

return Selection(memspace, filespace, details::get_dataset(slice));
return detail::make_selection(memspace, filespace, details::get_dataset(slice));
}

template <typename Derivate>
Expand All @@ -87,7 +87,7 @@ inline Selection SliceTraits<Derivate>::select(const HyperSlab& hyper_slab) cons
auto n_elements = H5Sget_select_npoints(filespace.getId());
auto memspace = DataSpace(std::array<size_t, 1>{size_t(n_elements)});

return Selection(memspace, filespace, details::get_dataset(slice));
return detail::make_selection(memspace, filespace, details::get_dataset(slice));
}


Expand Down Expand Up @@ -153,7 +153,7 @@ inline Selection SliceTraits<Derivate>::select(const ElementSet& elements) const
HDF5ErrMapper::ToException<DataSpaceException>("Unable to select elements");
}

return Selection(DataSpace(num_elements), space, details::get_dataset(slice));
return detail::make_selection(DataSpace(num_elements), space, details::get_dataset(slice));
}


Expand Down

0 comments on commit 20be06f

Please sign in to comment.