- removed milestone
Get rid of Hierarchical::_self
Hierarchical::_self
is shared ptr constructed in
/// Constructor
Hierarchical(T& self) : _self(reference_to_no_delete_pointer(self)) {}
It is only needed for computing Hierarchical::root/leaf_node_shared_ptr()
. This can be defined as returning NULL
when it should return self. Additionally it can be made private and replaced by either
- template specialization
shared_ptr<Hierarchical<T>> shared_ptr<Hierarchical<T>>::root/leaf_node() const
- or just free function
shared_ptr<Hierarchical<T>> leaf/root_node(shared_ptr<Hierarchical<T>>)
Comments (5)
-
-
reporter Or maybe using
std::enable_shared_from_this
andshared_from_this()
. (Too bad thatshared_from_this()
on not-shared-ptr-managed object is undefined behaviour in C++11, being an exception in C++17.) -
The description of this issue doesn't mention that the current behaviour is dangerous, since e.g.
std::shared_ptr<T> root_node_shared_ptr() { std::shared_ptr<T> it = _self; for (; it->_parent; it = it->_parent); return it; }
will actually return
_self
if it has no parent, resulting in a user believing a correctshared_ptr
is obtained while it's actually a dysfunctional no-deleter variant.In fact, looking at how parent/child pointers are set in
adapt.cpp
:template <typename T> void set_parent_child(const T& parent, std::shared_ptr<T> child) { T& _parent = const_cast<T&>(parent); _parent.set_child(child); child->set_parent(reference_to_no_delete_pointer(_parent)); }
all
_parent
pointers will be no-delete variants. This is not documented inroot_node_shared_ptr
and can lead to rather unexpected behaviour in user programs. Also this is what you get if you call e.g.f.root_node()
in python, meaning the usual coupling of shared_ptr and python refcounting will probably be broken if that's used. -
reporter Exactly! What you wrote in the first paragraph is a motivation for this issue. The second paragraph is issue #642 which depends on this one.
-
reporter - removed responsible
- Log in to comment
Removing milestone: 1.7 (automated comment)