Re: [PATCH] Add gnu::unique_ptr

[Pedro Alves]

On 10/13/2017 10:26 AM, Richard Biener wrote:
> On Fri, Oct 13, 2017 at 2:40 AM, David Malcolm  wrote:
>> From: Trevor Saunders 
>>
>> 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/msg00155.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 specialization
that this patch is missing (because the specialization was added
after switching to C++11):

  [pushed] Allow gdb::unique_xmalloc_ptr
  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

Re: [PATCH] Add gnu::unique_ptr

[Richard Biener]

On Fri, Oct 13, 2017 at 2:40 AM, David Malcolm  wrote:
> From: Trevor Saunders 
>
> 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/msg00155.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 (and will change to this version).

Thanks,
Richard.

> Motivation/use-cases:
> (a) I have an updated version of the
> name_hint/deferred_diagnostic patch kit, which uses this (rather
> than the refcounting used by
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html ); and
> (b) having this available ought to allow various other cleanups
> e.g. the example ones identified by Trevor here:
>   https://gcc.gnu.org/ml/gcc-patches/2017-07/msg02085.html
>
> Thanks
> Dave
>
>
> Blurb from Trevor:
>
> For most of the history of this see
>   https://sourceware.org/ml/gdb-patches/2016-10/msg00223.html
> The changes are mostly s/gdb/gtl/g
>
> include/ChangeLog:
>
> 2017-07-29  Trevor Saunders  
>
> * unique-ptr.h: New file.
>
> Combined ChangeLog follows:
>
> gcc/ChangeLog:
>
> David Malcolm 
>
> * Makefile.in (OBJS): Add unique-ptr-tests.o.
> * selftest-run-tests.c (selftest::run_tests): Call
> selftest::unique_ptr_tests_cc_tests.
> * selftest.h (selftest::unique_ptr_tests_cc_tests): New decl.
> * unique-ptr-tests.cc: New file.
>
> include/ChangeLog:
>
> Trevor Saunders  
> David Malcolm 
>
> * unique-ptr.h: New file.
> ---
>  gcc/Makefile.in  |   1 +
>  gcc/selftest-run-tests.c |   1 +
>  gcc/selftest.h   |   1 +
>  gcc/unique-ptr-tests.cc  | 177 ++
>  include/unique-ptr.h | 386 
> +++
>  5 files changed, 566 insertions(+)
>  create mode 100644 gcc/unique-ptr-tests.cc
>  create mode 100644 include/unique-ptr.h
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 878ce7b..2809619 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1568,6 +1568,7 @@ OBJS = \
> tree-vrp.o \
> tree.o \
> typed-splay-tree.o \
> +   unique-ptr-tests.o \
> valtrack.o \
> value-prof.o \
> var-tracking.o \
> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
> index 30e476d..b05e0fc 100644
> --- a/gcc/selftest-run-tests.c
> +++ b/gcc/selftest-run-tests.c
> @@ -66,6 +66,7 @@ selftest::run_tests ()
>sreal_c_tests ();
>fibonacci_heap_c_tests ();
>typed_splay_tree_c_tests ();
> +  unique_ptr_tests_cc_tests ();
>
>/* Mid-level data structures.  */
>input_c_tests ();
> diff --git a/gcc/selftest.h b/gcc/selftest.h
> index 96eccac..adc0b68 100644
> --- a/gcc/selftest.h
> +++ b/gcc/selftest.h
> @@ -194,6 +194,7 @@ extern void store_merging_c_tests ();
>  extern void typed_splay_tree_c_tests ();
>  extern void tree_c_tests ();
>  extern void tree_cfg_c_tests ();
> +extern void unique_ptr_tests_cc_tests ();
>  extern void vec_c_tests ();
>  extern void wide_int_cc_tests ();
>  extern void predict_c_tests ();
> diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc
> new file mode 100644
> index 000..df18467
> --- /