Get rid of Hierarchical::_self

[Jan Blechta]

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>>)

[Anders Logg (Chalmers)]

Removing milestone: 1.7 (automated comment)

[Jan Blechta]

Or maybe using std::enable_shared_from_this and shared_from_this(). (Too bad that shared_from_this() on not-shared-ptr-managed object is undefined behaviour in C++11, being an exception in C++17.)

[Martin Sandve Alnæs]

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 correct shared_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 in root_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.

[Jan Blechta]

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.