Re: [PATCH 1/2] add unique_ptr header

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

On Tue, Aug 01, 2017 at 10:40:53AM -0400, David Malcolm wrote:
> 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").

aiui C++03 is C++98 with a few additions to the stl.  Yes this doesn't
build with -std=c++98 I haven't tried any of the strange c++ compilers
out there, but its not very fancy so I expect its fine.

> 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").

Its possible, but I'm not sure it would provide much value.  I think
we're unlikely to change this code much since it already more or less
completely implements the API it is shimming.  It also is fairly
trivial, so I doubt anyone would want to run valgrind on just the
unique_ptr code.  Of course I may just be being lazy here.

thanks

Trev

> 
> I'm not a global reviewer, so treat these as suggestions.
> 
> Hope this is helpful
> Dave