Re: [PATCH 1/2] Add gcc/make-unique.h

[Jonathan Wakely <jwakely.gcc@gmail.com>]

On Tue, 12 Jul 2022, 01:25 David Malcolm, <dmalcolm@redhat.com> wrote:

> On Fri, 2022-07-08 at 22:16 +0100, Jonathan Wakely wrote:
> > On Fri, 8 Jul 2022 at 21:47, David Malcolm via Gcc <gcc@gcc.gnu.org>
> > wrote:
> > >
> > > std::unique_ptr is C++11, and I'd like to use it in the
> > > gcc/analyzer
> > > subdirectory, at least.  The following patch eliminates a bunch of
> > > "takes ownership" comments and manual "delete" invocations in favor
> > > of simply using std::unique_ptr.
> > >
> > > The problem is that the patch makes use of std::make_unique, but
> > > that
> > > was added in C++14.
> > >
> > > I've heard that it's reasonably easy to reimplement
> > > std::make_unique,
> > > but I'm not sure that my C++11 skills are up to it.
> >
> > You know we have an implementation of std::make_unique in GCC, with a
> > GCC-compatible licence that you can look at, right? :-)
> >
> > But it's not really necessary. There are only two reasons to prefer
> > make_unique over just allocating an object with new and constructing
> > a
> > unique_ptr from it:
> >
> > 1) avoid a "naked" new in your code (some coding styles like this,
> > but
> > it's not really important as long as the 'delete' is managed
> > automatically by unique_ptr).
> >
> > 2) exception-safety when allocating multiple objects as args to a
> > function, see https://herbsutter.com/gotw/_102/ for details.
> > Irrelevant for GCC, because we build without exceptions.
>
> [moving from gcc to gcc-patches mailing list]
>
> Also, I *think* it's a lot less typing, since I can write just:
>
>   std::make_unique<name_of_type_which_could_be_long> (args)
>
> rather than
>
>   std::unique_ptr<name_of_type_which_could_be_long> (new
> name_of_type_which_could_be_long (args));
>
> >
> >
> >
> > > Is there:
> > > (a) an easy way to implement a std::make_unique replacement
> > >     (e.g. in system.h? what to call it?), or
> >
> > If you don't care about using it to create unique_ptr<T[]> arrays,
> > it's trivial:
> >
> >   template<typename T, typename... Args>
> >     inline typename std::enable_if<!std::is_array<T>::value,
> > std::unique_ptr<T>>::type
> >     make_unique(Args&&... args)
> >     { return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
> > }
> >
> > To add the overload that works for arrays is a little trickier.
>
> Thanks!
>
> I tried adding it to gcc/system.h, but anything that uses it needs to
> have std::unique_ptr declared, which meant forcibly including <memory>
> from gcc/system.h
>
> So instead, here's a patch that adds a new gcc/make-unique.h header,
> containing just the template decl above (in the root namespace, rather
> than std::, which saves a bit more typing).
>

Adding things to std isn't allowed anyway, so that's correct.


> I've successfully bootstrapped&regression-tested a version of my earlier
> analyzer patch that uses this patch (see patch 2 of the kit, which has
> lots of usage examples).
>
> OK for trunk?
>
> Dave
>
>
>
> This patch adds gcc/make-unique.h, containing a minimal C++11
> implementation of make_unique (std::make_unique is C++14).
>
> gcc/ChangeLog:
>         * make-unique.h: New file.
>
> Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> ---
>  gcc/make-unique.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 gcc/make-unique.h
>
> diff --git a/gcc/make-unique.h b/gcc/make-unique.h
> new file mode 100644
> index 00000000000..c99c5328545
> --- /dev/null
> +++ b/gcc/make-unique.h
> @@ -0,0 +1,41 @@
> +/* Minimal implementation of make_unique for C++11 compatibility.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC 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, or (at your option) any later
> +version.
> +
> +GCC 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 GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_MAKE_UNIQUE
> +#define GCC_MAKE_UNIQUE
> +
> +/* This header uses std::unique_ptr, but <memory> can't be directly
> +   included due to issues with macros.  Hence it must be included from
> +   system.h by defining INCLUDE_MEMORY in any source file using it.  */
> +
> +#ifndef INCLUDE_MEMORY
> +# error "You must define INCLUDE_MEMORY before including system.h to use
> make-unique.h"
> +#endif
>

You also need <type_traits> for the enable_if and is_array traits. With
libstdc++ that gets included by <memory> but that's guaranteed for other
library implementations.

I don't know if that had the same kind of issues as other system headers or
if it can just be included here.


+
> +/* Minimal implementation of make_unique for C++11 compatibility
> +   (std::make_unique is C++14).  */
> +
> +template<typename T, typename... Args>
> +inline typename std::enable_if<!std::is_array<T>::value,
> std::unique_ptr<T>>::type
> +make_unique(Args&&... args)
> +{
> +  return std::unique_ptr<T> (new T (std::forward<Args> (args)...));
> +}
> +
> +#endif /* ! GCC_MAKE_UNIQUE */
> --
> 2.26.3
>
>