[05/49] vec.h: add auto_delete_vec

This patch adds a class auto_delete_vec<T>, a subclass of auto_vec <T *>
that deletes all of its elements on destruction; it's used in many
places later in the kit.

This is a crude way for a vec to "own" the objects it points to
and clean up automatically (essentially a workaround for not being able
to use unique_ptr, due to C++98).

gcc/ChangeLog:
	* vec.c (class selftest::count_dtor): New class.
	(selftest::test_auto_delete_vec): New test.
	(selftest::vec_c_tests): Call it.
	* vec.h (class auto_delete_vec): New class template.
	(auto_delete_vec<T>::~auto_delete_vec): New dtor.
---
 gcc/vec.c | 27 +++++++++++++++++++++++++++
 gcc/vec.h | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

On 11/15/19 6:22 PM, David Malcolm wrote:
> This patch adds a class auto_delete_vec<T>, a subclass of auto_vec <T *>
> that deletes all of its elements on destruction; it's used in many
> places later in the kit.
> 
> This is a crude way for a vec to "own" the objects it points to
> and clean up automatically (essentially a workaround for not being able
> to use unique_ptr, due to C++98).
> 
> gcc/ChangeLog:
> 	* vec.c (class selftest::count_dtor): New class.
> 	(selftest::test_auto_delete_vec): New test.
> 	(selftest::vec_c_tests): Call it.
> 	* vec.h (class auto_delete_vec): New class template.
> 	(auto_delete_vec<T>::~auto_delete_vec): New dtor.

Because of slicing, unless preventing the elements from being
deleted in the class dtor is meant to be a feature, it seems that
using a wrapper class rather than public derivation from auto_vec
might be a safer solution.

It might be worth mentioning in a comment that the class isn't
safe to copy or assign (each copy would wind up delete the same
pointers), in addition to making its copy ctor and copy assignment
operator inaccessible or deleted.

Martin

> ---
>   gcc/vec.c | 27 +++++++++++++++++++++++++++
>   gcc/vec.h | 35 +++++++++++++++++++++++++++++++++++
>   2 files changed, 62 insertions(+)
> 
> diff --git a/gcc/vec.c b/gcc/vec.c
> index bac5eb7..a9c840d 100644
> --- a/gcc/vec.c
> +++ b/gcc/vec.c
> @@ -516,6 +516,32 @@ test_reverse ()
>     }
>   }
>   
> +/* A test class that increments a counter every time its dtor is called.  */
> +
> +class count_dtor
> +{
> + public:
> +  count_dtor (int *counter) : m_counter (counter) {}
> +  ~count_dtor () { (*m_counter)++; }
> +
> + private:
> +  int *m_counter;
> +};
> +
> +/* Verify that auto_delete_vec deletes the elements within it.  */
> +
> +static void
> +test_auto_delete_vec ()
> +{
> +  int dtor_count = 0;
> +  {
> +    auto_delete_vec <count_dtor> v;
> +    v.safe_push (new count_dtor (&dtor_count));
> +    v.safe_push (new count_dtor (&dtor_count));
> +  }
> +  ASSERT_EQ (dtor_count, 2);
> +}
> +
>   /* Run all of the selftests within this file.  */
>   
>   void
> @@ -533,6 +559,7 @@ vec_c_tests ()
>     test_block_remove ();
>     test_qsort ();
>     test_reverse ();
> +  test_auto_delete_vec ();
>   }
>   
>   } // namespace selftest
> diff --git a/gcc/vec.h b/gcc/vec.h
> index 091056b..1957739 100644
> --- a/gcc/vec.h
> +++ b/gcc/vec.h
> @@ -1539,6 +1539,28 @@ class auto_string_vec : public auto_vec <char *>
>     ~auto_string_vec ();
>   };
>   
> +/* A subclass of auto_vec <T *> that deletes all of its elements on
> +   destruction.
> +
> +   This is a crude way for a vec to "own" the objects it points to
> +   and clean up automatically.
> +
> +   For example, no attempt is made to delete elements when an item
> +   within the vec is overwritten.
> +
> +   We can't rely on gnu::unique_ptr within a container,
> +   since we can't rely on move semantics in C++98.  */
> +
> +template <typename T>
> +class auto_delete_vec : public auto_vec <T *>
> +{
> + public:
> +  auto_delete_vec () {}
> +  auto_delete_vec (size_t s) : auto_vec <T *> (s) {}
> +
> +  ~auto_delete_vec ();
> +};
> +
>   /* Conditionally allocate heap memory for VEC and its internal vector.  */
>   
>   template<typename T>
> @@ -1643,6 +1665,19 @@ auto_string_vec::~auto_string_vec ()
>       free (str);
>   }
>   
> +/* auto_delete_vec's dtor, deleting all contained items, automatically
> +   chaining up to ~auto_vec <T*>, which frees the internal buffer.  */
> +
> +template <typename T>
> +inline
> +auto_delete_vec<T>::~auto_delete_vec ()
> +{
> +  int i;
> +  T *item;
> +  FOR_EACH_VEC_ELT (*this, i, item)
> +    delete item;
> +}
> +
>   
>   /* Return a copy of this vector.  */
>   
>

On Wed, 2019-12-04 at 09:29 -0700, Martin Sebor wrote:
> On 11/15/19 6:22 PM, David Malcolm wrote:
> > This patch adds a class auto_delete_vec<T>, a subclass of auto_vec
> > <T *>
> > that deletes all of its elements on destruction; it's used in many
> > places later in the kit.
> > 
> > This is a crude way for a vec to "own" the objects it points to
> > and clean up automatically (essentially a workaround for not being
> > able
> > to use unique_ptr, due to C++98).
> > 
> > gcc/ChangeLog:
> > 	* vec.c (class selftest::count_dtor): New class.
> > 	(selftest::test_auto_delete_vec): New test.
> > 	(selftest::vec_c_tests): Call it.
> > 	* vec.h (class auto_delete_vec): New class template.
> > 	(auto_delete_vec<T>::~auto_delete_vec): New dtor.
> 
> Because of slicing, unless preventing the elements from being
> deleted in the class dtor is meant to be a feature, it seems that
> using a wrapper class rather than public derivation from auto_vec
> might be a safer solution.
> 
> It might be worth mentioning in a comment that the class isn't
> safe to copy or assign (each copy would wind up delete the same
> pointers), in addition to making its copy ctor and copy assignment
> operator inaccessible or deleted.
> 
> Martin

In the version of the patch in the v4 kit:
  https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01035.html
I added:
  private:
     DISABLE_COPY_AND_ASSIGN(auto_delete_vec<T>);
to the class.

Does that satisfy your concerns about slicing? (and, indeed, about
copying and assigning)

Thanks
Dave

On Wed, 2019-12-18 at 10:59 -0500, David Malcolm wrote:
> On Wed, 2019-12-04 at 09:29 -0700, Martin Sebor wrote:
> > On 11/15/19 6:22 PM, David Malcolm wrote:
> > > This patch adds a class auto_delete_vec<T>, a subclass of auto_vec
> > > <T *>
> > > that deletes all of its elements on destruction; it's used in many
> > > places later in the kit.
> > > 
> > > This is a crude way for a vec to "own" the objects it points to
> > > and clean up automatically (essentially a workaround for not being
> > > able
> > > to use unique_ptr, due to C++98).
> > > 
> > > gcc/ChangeLog:
> > > 	* vec.c (class selftest::count_dtor): New class.
> > > 	(selftest::test_auto_delete_vec): New test.
> > > 	(selftest::vec_c_tests): Call it.
> > > 	* vec.h (class auto_delete_vec): New class template.
> > > 	(auto_delete_vec<T>::~auto_delete_vec): New dtor.
> > 
> > Because of slicing, unless preventing the elements from being
> > deleted in the class dtor is meant to be a feature, it seems that
> > using a wrapper class rather than public derivation from auto_vec
> > might be a safer solution.
> > 
> > It might be worth mentioning in a comment that the class isn't
> > safe to copy or assign (each copy would wind up delete the same
> > pointers), in addition to making its copy ctor and copy assignment
> > operator inaccessible or deleted.
> > 
> > Martin
> 
> In the version of the patch in the v4 kit:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01035.html
> I added:
>   private:
>      DISABLE_COPY_AND_ASSIGN(auto_delete_vec<T>);
> to the class.
> 
> Does that satisfy your concerns about slicing? (and, indeed, about
> copying and assigning)
I think this can and should go forward at this point.

jeff