[PATCH] Implement unique_xmalloc_ptr<T[]> and add more selftests
David Malcolm
dmalcolm@redhat.com
Sat Oct 14 00:35:00 GMT 2017
On Fri, 2017-10-13 at 13:01 +0100, Pedro Alves wrote:
> On 10/13/2017 10:26 AM, Richard Biener wrote:
> > On Fri, Oct 13, 2017 at 2:40 AM, David Malcolm <dmalcolm@redhat.com
> > > wrote:
> > > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > >
> > > I had a go at updating Trevor's unique_ptr patch from July
> > > ( https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02084.html )
> > >
> > > One of the sticking points was what to call the namespace; there
> > > was
> > > wariness about using "gtl" as the name.
> > >
> > > Richi proposed (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg001
> > > 55.html):
> > > > If it should be short use g::. We can also use gnu:: I guess
> > > > and I
> > > > agree gnutools:: is a little long (similar to
> > > > libiberty::). Maybe
> > > > gt:: as a short-hand for gnutools.
> > >
> > > Pedro noted (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00157.
> > > html):
> > > > Exactly 3 letters has the nice property of making
> > > > s/gtl::foo/std::foo/
> > > > super trivial down the road; you don't have to care about
> > > > reindenting
> > > > stuff
> > >
> > > Hence this version of the patch uses "gnu::" - 3 letters, one of
> > > the
> > > ones Richi proposed, and *not* a match for ".tl" (e.g. "gtl");
> > > (FWIW personally "gnu::" is my favorite, followed by "gcc::").
> > >
> > > The include/unique-ptr.h in this patch is identical to that
> > > posted
> > > by Trevor in July, with the following changes (by me):
> > > - renaming of "gtl" to "gnu"
> > > - renaming of DEFINE_GDB_UNIQUE_PTR to DEFINE_GNU_UNIQUE_PTR
> > > - renaming of xfree_deleter to xmalloc_deleter, and making it
> > > use "free" rather than "xfree" (which doesn't exist)
> > >
> > > I also went and added a gcc/unique-ptr-tests.cc file containing
> > > selftests (my thinking here is that although std::unique_ptr
> > > ought
> > > to already be well-tested, we need to ensure that the fallback
> > > implementation is sane when building with C++ prior to C++11).
> > >
> > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
> > > using gcc 4.8 for the initial bootstrap (hence testing both
> > > gnu+03
> > > and then gnu++14 in the selftests, for stage 1 and stages 2 and 3
> > > respectively).
> > >
> > > I also manually tested selftests with both gcc 4.8 and trunk on
> > > the same hardware (again, to exercise both the with/without C++11
> > > behavior).
> > >
> > > Tested with "make selftest-valgrind" (no new issues).
> > >
> > > OK for trunk?
> >
> > Ok if the gdb folks approve
>
> The new tests looks good to me. [ The class too, obviously. :-) ]
>
> (and will change to this version).
>
> GDB used this emulation/shim in master for a period, but it
> no longer does, because GDB switched to requiring
> C++11 (gcc >= 4.8) instead meanwhile... I.e., GDB uses
> standard std::unique_ptr nowadays.
>
> GDB still uses gdb::unique_xmalloc_ptr though. Might make
> sense to switch to using gnu::unique_xmalloc_ptr instead.
>
> GDB does have an xfree function that we call instead
> of free throughout (that's where the reference David fixed
> comes from). We can most probably just switch to free too nowadays.
>
> Also, gdb nowadays also has a gdb::unique_xmalloc_ptr<T[]>
> specialization
> that this patch is missing (because the specialization was added
> after switching to C++11):
>
> [pushed] Allow gdb::unique_xmalloc_ptr<T[]>
> https://sourceware.org/ml/gdb-patches/2017-08/msg00212.html
>
> So we'd need that before switching. I don't recall off hand whether
> it's easy to make that work with the C++03 version.
>
> Thanks,
> Pedro Alves
Pedro: it appears based on Richard's email that you (or another
gdb hacker) are responsible for approving this gcc change.
(presumably in the hope of sharing this header between gcc and gdb?)
(though gdb's copy doesn't seem to have a lot left in it:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/common/gdb_unique_ptr.h;hb=HEAD
is only 52 lines, including comments).
As far as I can tell from your mail, the one issue that blocks that
is the lack of gdb::unique_xmalloc_ptr<T[]>.
So here's an patch on top of the previous one which adds the
xmalloc_deleter<T[]> (taken from gdb, but changing "xfree" to
"free", and adding support for pre-C++11 dialects), along with
selftests for unique_ptr<T[]>, unique_xmalloc_ptr<T> and
unique_xmalloc_ptr<T[]>.
As before, successfully bootstrapped & regrtested on x86_64-pc-linux-gnu,
using gcc 4.8 for the initial bootstrap (hence testing both gnu++03
then gnu++14 in the selftests, for stage 1 and stages 2 and 3
respectively).
Hand-tested with "make selftest-valgrind" with both gnu++03 and
gnu++14.
Also tested stage1 on powerpc-ibm-aix7.1.3.0 ("gcc111" in the
compile farm; gcc 4.8 i.e. gnu++03)
Is this OK?
Thanks
Dave
gcc/ChangeLog:
* unique-ptr-tests.cc (struct selftest::has_default_ctor): New
struct.
(struct selftest::dummy): New struct.
(selftest::test_array_new): New function.
(selftest::test_xmalloc): New function.
(selftest::test_xmalloc_array): New function.
(selftest::unique_ptr_tests_cc_tests): Call the new functions.
include/ChangeLog:
* unique-ptr.h (struct xmalloc_deleter<T[]>): New.
[!__cplusplus >= 201103] (class unique_xmalloc_ptr<T[]>): New.
---
gcc/unique-ptr-tests.cc | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
include/unique-ptr.h | 17 +++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc
index df18467..f051a77 100644
--- a/gcc/unique-ptr-tests.cc
+++ b/gcc/unique-ptr-tests.cc
@@ -57,6 +57,21 @@ private:
stats &m_s;
};
+/* A struct for testing unique_ptr<T[]>. */
+
+struct has_default_ctor
+{
+ has_default_ctor () : m_field (42) {}
+ int m_field;
+};
+
+/* A dummy struct for testing unique_xmalloc_ptr. */
+
+struct dummy
+{
+ int field;
+};
+
} // anonymous namespace
/* Verify that the default ctor inits ptrs to NULL. */
@@ -160,6 +175,45 @@ test_overloaded_ops ()
ASSERT_NE (f, g);
}
+/* Verify that the gnu::unique_ptr specialization for T[] works. */
+
+static void
+test_array_new ()
+{
+ const int num = 10;
+ gnu::unique_ptr<has_default_ctor[]> p (new has_default_ctor[num]);
+ ASSERT_NE (NULL, p.get ());
+ /* Verify that operator[] works, and that the default ctor was called
+ on each element. */
+ for (int i = 0; i < num; i++)
+ ASSERT_EQ (42, p[i].m_field);
+}
+
+/* Verify that gnu::unique_malloc_ptr works. */
+
+static void
+test_xmalloc ()
+{
+ gnu::unique_xmalloc_ptr<dummy> p (XNEW (dummy));
+ ASSERT_NE (NULL, p.get ());
+}
+
+/* Verify the gnu::unique_malloc_ptr specialization for T[]. */
+
+static void
+test_xmalloc_array ()
+{
+ const int num = 10;
+ gnu::unique_xmalloc_ptr<dummy[]> p (XNEWVEC (dummy, num));
+ ASSERT_NE (NULL, p.get ());
+
+ /* Verify that operator[] works. */
+ for (int i = 0; i < num; i++)
+ p[i].field = 42;
+ for (int i = 0; i < num; i++)
+ ASSERT_EQ (42, p[i].field);
+}
+
/* Run all of the selftests within this file. */
void
@@ -170,6 +224,9 @@ unique_ptr_tests_cc_tests ()
test_overwrite_of_null ();
test_overwrite_of_non_null ();
test_overloaded_ops ();
+ test_array_new ();
+ test_xmalloc ();
+ test_xmalloc_array ();
}
} // namespace selftest
diff --git a/include/unique-ptr.h b/include/unique-ptr.h
index eddb001..c5ca031 100644
--- a/include/unique-ptr.h
+++ b/include/unique-ptr.h
@@ -357,6 +357,13 @@ struct xmalloc_deleter
void operator() (T *ptr) const { free (ptr); }
};
+/* Same, for arrays. */
+template <typename T>
+struct xmalloc_deleter<T[]>
+{
+ void operator() (T *ptr) const { free (ptr); }
+};
+
#if __cplusplus >= 201103
/* In C++11, we just import the standard unique_ptr to our namespace
@@ -379,6 +386,16 @@ class unique_xmalloc_ptr : public unique_ptr<T, xmalloc_deleter<T> >
DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
};
+/* Define gnu::unique_xmalloc_ptr specialization for T[]. */
+
+template <typename T>
+class unique_xmalloc_ptr<T[]> : public unique_ptr<T[], xmalloc_deleter<T[]> >
+{
+ typedef unique_ptr<T[], xmalloc_deleter<T[]> > base_type;
+
+ DEFINE_GNU_UNIQUE_PTR (unique_xmalloc_ptr)
+};
+
#endif /* C++11 */
} /* namespace gnu */
--
1.8.5.3
More information about the Gcc-patches
mailing list