Re: [PATCH 1/2] add unique_ptr header

[Trevor Saunders]

On Wed, Oct 11, 2017 at 02:16:38PM -0400, David Malcolm wrote:
> On Sat, 2017-08-05 at 01:39 -0400, Trevor Saunders wrote:
> > On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> > > On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > > > aiui C++03 is C++98 with a few additions to the stl.
> > > 
> > > Again, STL != C++ Standard Library.
> > > 
> > > C++03 made some important changes to the core language,
> > > particularly
> > > the value-initialization rules.
> > > 
> > > Most of the library changes were small bug fixes, and most were to
> > > locales (which didn't originate in the STL and aren't even
> > > templates)
> > > and iostreams (which didn't originate in the STL).
> > > 
> > > There were also changes to std::auto_ptr (also not from the STL)
> > > which
> > > this might rely on (I haven't checked).
> > 
> > I doubt it, as Pedro said in his email it originally copied code from
> > std::auto_ptr, but it doesn't use the standard libraries definition
> > of
> > std::auto_ptr anywhere.  However please do feel free to look at the
> > implementation.
> > 
> > Trev
> > 
> 
> Trevor: did this go anywhere?

I'm sorry, I've been pretty busy starting a new job, and haven't yet
finished the paperwork to get permission to work on gcc stuff.  So I
haven't done anything since my last message in the thread.

> Do we have an unique_ptr class we can use within gcc's own
> implementation?

Not yet :(

> (I was hoping to use it for
>   https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html
>   "[PATCH 1/3] c-family: add name_hint/deferred_diagnostic";
> see the discussion at:
>   https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00123.html
> onwards)

I believe you should be good to take the last version of the patch I
submitted and sed the namespace name to whatever people can agree on.  I
think that was the only issue people had with the patch.  I'd really
like to see this get into gcc8, and will try to get stuff done on my
end, but I'd also be thankful if you felt like doing that work first.

Trev

> 
> Thanks
> Dave

Re: [PATCH 1/2] add unique_ptr header

[David Malcolm]

On Sat, 2017-08-05 at 01:39 -0400, Trevor Saunders wrote:
> On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> > On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > > aiui C++03 is C++98 with a few additions to the stl.
> > 
> > Again, STL != C++ Standard Library.
> > 
> > C++03 made some important changes to the core language,
> > particularly
> > the value-initialization rules.
> > 
> > Most of the library changes were small bug fixes, and most were to
> > locales (which didn't originate in the STL and aren't even
> > templates)
> > and iostreams (which didn't originate in the STL).
> > 
> > There were also changes to std::auto_ptr (also not from the STL)
> > which
> > this might rely on (I haven't checked).
> 
> I doubt it, as Pedro said in his email it originally copied code from
> std::auto_ptr, but it doesn't use the standard libraries definition
> of
> std::auto_ptr anywhere.  However please do feel free to look at the
> implementation.
> 
> Trev
> 

Trevor: did this go anywhere?

Do we have an unique_ptr class we can use within gcc's own
implementation?

(I was hoping to use it for
  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html
  "[PATCH 1/3] c-family: add name_hint/deferred_diagnostic";
see the discussion at:
  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00123.html
onwards)

Thanks
Dave

Re: [PATCH 1/2] add unique_ptr header

[Trevor Saunders]

On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > aiui C++03 is C++98 with a few additions to the stl.
> 
> Again, STL != C++ Standard Library.
> 
> C++03 made some important changes to the core language, particularly
> the value-initialization rules.
> 
> Most of the library changes were small bug fixes, and most were to
> locales (which didn't originate in the STL and aren't even templates)
> and iostreams (which didn't originate in the STL).
> 
> There were also changes to std::auto_ptr (also not from the STL) which
> this might rely on (I haven't checked).

I doubt it, as Pedro said in his email it originally copied code from
std::auto_ptr, but it doesn't use the standard libraries definition of
std::auto_ptr anywhere.  However please do feel free to look at the
implementation.

Trev

> 
> 

Re: [PATCH 1/2] add unique_ptr header

[Jonathan Wakely]

On 01/08/17 23:09 -0400, Trevor Saunders wrote:

aiui C++03 is C++98 with a few additions to the stl.


Again, STL != C++ Standard Library.

C++03 made some important changes to the core language, particularly
the value-initialization rules.

Most of the library changes were small bug fixes, and most were to
locales (which didn't originate in the STL and aren't even templates)
and iostreams (which didn't originate in the STL).

There were also changes to std::auto_ptr (also not from the STL) which
this might rely on (I haven't checked).

Re: [PATCH 1/2] add unique_ptr header

[Richard Biener]

On Tue, Aug 1, 2017 at 4:40 PM, David Malcolm  wrote
> On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+...@tbsaunde.org wrote:
>> From: Trevor Saunders 
>>
>> 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.
>> ---
>>  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 000..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 > 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.

It's actually C++04 but I don't remember the details.  It happens to work
with GCC 3.4 and thus C++98 though.

Richard.

> 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

Re: [PATCH 1/2] add unique_ptr header

[Trevor Saunders]

On Tue, Aug 01, 2017 at 10:40:53AM -0400, David Malcolm wrote:
> On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+...@tbsaunde.org wrote:
> > From: Trevor Saunders 
> > 
> > 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.
> > ---
> >  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 000..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  > 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

Re: [PATCH 1/2] add unique_ptr header

[David Malcolm]

On Mon, 2017-07-31 at 19:46 -0400, tbsaunde+...@tbsaunde.org wrote:
> From: Trevor Saunders 
> 
> 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.
> ---
>  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 000..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  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