Elem::_children should be a unique_ptr to an array of Elem*

* The Elem class is no longer manually mangaging any memory
* Default the Elem destructor

Author: jwpeterson

include/geom/elem.h

@@ -110,23 +110,23 @@ class Elem : public ReferenceCountedObject<Elem>,
 public:
 
   /**
-   * Elems are responsible for allocating and deleting their children
-   * during refinement, so they cannot be (default) copied or
-   * assigned. We therefore explicitly delete these operations.  Note
-   * that because _children is a C-style array, an Elem cannot even be
-   * safely default move-constructed (we would have to maintain a
-   * custom move constructor that explicitly sets _children to nullptr
-   * to do this safely).
+   * Elems are responsible for allocating and deleting space for
+   * storing pointers to their children during refinement, so they
+   * cannot currently be (default) copy-constructed or copy-
+   * assigned. We therefore explicitly delete these operations. In
+   * addition, the DofObject base class currently has private copy
+   * construction and assignment operators, so that prevents us from
+   * copying Elems as well.
    */
   Elem (Elem &&) = delete;
   Elem (const Elem &) = delete;
   Elem & operator= (const Elem &) = delete;
   Elem & operator= (Elem &&) = delete;
 
   /**
-   * Destructor.  Frees all the memory associated with the element.
+   * Destructor.
    */
-  virtual ~Elem();
+  virtual ~Elem() = default;
 
   /**
    * \returns The \p Point associated with local \p Node \p i.
@@ -2002,9 +2002,13 @@ class Elem : public ReferenceCountedObject<Elem>,
 
 #ifdef LIBMESH_ENABLE_AMR
   /**
-   * Pointers to this element's children.
+   * unique_ptr to array of this element's children.
+   *
+   * A Mesh ultimately owns the child Elems so we are not responsible
+   * for deleting them, but we are responsible for cleaning up the
+   * array allocated to hold those Elems, hence the unique_ptr.
    */
-  Elem ** _children;
+  std::unique_ptr<Elem *[]> _children;
 #endif
 
   /**
@@ -2087,19 +2091,20 @@ Elem::ConstChildRefIter : public PointerToPointerIter<const Elem>
 };
 
 
+
 inline
 SimpleRange<Elem::ChildRefIter> Elem::child_ref_range()
 {
   libmesh_assert(_children);
-  return {_children, _children + this->n_children()};
+  return {_children.get(), _children.get() + this->n_children()};
 }
 
 
 inline
 SimpleRange<Elem::ConstChildRefIter> Elem::child_ref_range() const
 {
   libmesh_assert(_children);
-  return {_children, _children + this->n_children()};
+  return {_children.get(), _children.get() + this->n_children()};
 }
 #endif // LIBMESH_ENABLE_AMR
 
@@ -2127,9 +2132,6 @@ Elem::Elem(const unsigned int nn,
            Node ** nodelinkdata) :
   _nodes(nodelinkdata),
   _elemlinks(elemlinkdata),
-#ifdef LIBMESH_ENABLE_AMR
-  _children(nullptr),
-#endif
   _sbd_id(0),
 #ifdef LIBMESH_ENABLE_AMR
   _rflag(Elem::DO_NOTHING),
@@ -2179,30 +2181,6 @@ Elem::Elem(const unsigned int nn,
 
 
 
-inline
-Elem::~Elem()
-{
-  // Deleting my parent/neighbor/nodes storage isn't necessary since it's
-  // handled by the subclass
-
-  // if (_nodes != nullptr)
-  //   delete [] _nodes;
-  // _nodes = nullptr;
-
-  // delete [] _elemlinks;
-
-#ifdef LIBMESH_ENABLE_AMR
-
-  // Delete my children's storage
-  if (_children != nullptr)
-    delete [] _children;
-  _children = nullptr;
-
-#endif
-}
-
-
-
 inline
 const Point & Elem::point (const unsigned int i) const
 {
@@ -2746,7 +2724,7 @@ inline
 bool Elem::has_children() const
 {
 #ifdef LIBMESH_ENABLE_AMR
-  if (_children == nullptr)
+  if (!_children)
     return false;
   else
     return true;
@@ -2760,7 +2738,7 @@ inline
 bool Elem::has_ancestor_children() const
 {
 #ifdef LIBMESH_ENABLE_AMR
-  if (_children == nullptr)
+  if (!_children)
     return false;
   else
     for (auto & c : child_ref_range())

src/geom/elem.C

@@ -1574,9 +1574,9 @@ void Elem::add_child (Elem * elem)
 {
   const unsigned int nc = this->n_children();
 
-  if (_children == nullptr)
+  if (!_children)
     {
-      _children = new Elem *[nc];
+      _children = std::make_unique<Elem *[]>(nc);
 
       for (unsigned int c = 0; c != nc; c++)
         this->set_child(c, nullptr);
@@ -1602,7 +1602,7 @@ void Elem::add_child (Elem * elem, unsigned int c)
   if (!this->has_children())
     {
       const unsigned int nc = this->n_children();
-      _children = new Elem *[nc];
+      _children = std::make_unique<Elem *[]>(nc);
 
       for (unsigned int i = 0; i != nc; i++)
         this->set_child(i, nullptr);

src/geom/elem_refinement.C

@@ -94,7 +94,7 @@ void Elem::refine (MeshRefinement & mesh_refinement)
   // Create my children if necessary
   if (!_children)
     {
-      _children = new Elem *[nc];
+      _children = std::make_unique<Elem *[]>(nc);
 
       unsigned int parent_p_level = this->p_level();
       const unsigned int nei = this->n_extra_integers();
@@ -166,8 +166,6 @@ void Elem::coarsen()
   libmesh_assert (!this->active());
 
   // We no longer delete children until MeshRefinement::contract()
-  // delete [] _children;
-  // _children = nullptr;
 
   unsigned int parent_p_level = 0;
 
@@ -233,8 +231,7 @@ void Elem::contract()
   libmesh_assert (this->active());
 
   // Active contracted elements no longer can have children
-  delete [] _children;
-  _children = nullptr;
+  _children.reset(nullptr);
 
   if (this->refinement_flag() == Elem::JUST_COARSENED)
     this->set_refinement_flag(Elem::DO_NOTHING);