Re: [PATCH 1/2] add unique_ptr header

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> 
> 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  <tbsaunde+gcc@tbsaunde.org>
> 
> 	* unique-ptr.h: New file.
> ---
>  include/unique-ptr.h | 386
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 386 insertions(+)
>  create mode 100644 include/unique-ptr.h
> 
> diff --git a/include/unique-ptr.h b/include/unique-ptr.h
> new file mode 100644
> index 00000000000..7903a5abefe
> --- /dev/null
> +++ b/include/unique-ptr.h
> @@ -0,0 +1,386 @@
> +/* gtl::unique_ptr, a simple std::unique_ptr replacement for C++03.
> +
> +   Copyright (C) 2007-2016 Free Software Foundation, Inc.
> +
> +   This file is part of GCC.
> +
> +   This program is free software; you can redistribute it and/or
> modify
> +   it under the terms of the GNU General Public License as published
> by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/license
> s/>.  */
> +
> +/* gtl::unique_ptr defines a C++ owning smart pointer that exposes a
> +   subset of the std::unique_ptr API.
> +
> +   In fact, when compiled with a C++11 compiler, gtl::unique_ptr
> +   actually _is_ std::unique_ptr.  When compiled with a C++03
> compiler
> +   OTOH, it's an hand coded std::unique_ptr emulation that assumes
> +   code is correct and doesn't try to be too smart.

The comments talk about C++03 (presumably due to the gdb heritage of
this code), but gcc's minimum requirement is for C++98.

Does this code work with C++98?  If so, presumably this comment should
state that (presumably by just writing "C++98 or C++03" wherever you
say "C++03").

[...snip...]

Could the patch add a set of selftests for the new code?  (e.g. assert
the number of times that a destructor is called, for various test
classes in various situations).  This would also give a handy way to
run the pertinent code relatively quickly under valgrind (via "make
selftest-valgrind").

I'm not a global reviewer, so treat these as suggestions.

Hope this is helpful
Dave