| Message ID | 20220712002527.417444-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] Add gcc/make-unique.h | expand |
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®ression-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 > >
On Tue, 12 Jul 2022 at 07:48, Jonathan Wakely wrote:
> 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.
Sorry, I was replying from my phone and missed a word. That's **not**
guaranteed for other library implementations.
(It's not formally guaranteed for libstdc++ either, but in practice
nearly every libstdc++ header includes <type_traits> because nearly
every header needs part of it, and that's unlikely to change).
On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote: > 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 Did you consider making gcc/system.h include gcc/make-unique.h itself if INCLUDE_MEMORY is defined? Something like: #ifdef INCLUDE_MEMORY # include <memory> + #include "make-unique.h" #endif This is because std::make_unique is defined in <memory> in C++14. This would mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated. > (in the root namespace, rather than std::, which saves a bit more typing). It's less typing now, but it will be more churn once GCC requires C++14 (or later), at which point you'll naturally want to get rid of the custom make_unique. More churn since make_unique -> std::make_unique may require re-indentation of arguments, etc. For that reason, I would suggest instead to put the function (and any other straight standard library backport) in a 3-letter namespace already, like, gcc::make_unique or gnu::make_unique. That way, when the time comes that GCC requires C++14, the patch to replace gcc::make_unique won't have to worry about reindenting code, it'll just replace gcc -> std.
On Tue, 12 Jul 2022 at 14:24, Pedro Alves wrote: > > On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote: > > > 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 > > Did you consider making gcc/system.h include gcc/make-unique.h itself > if INCLUDE_MEMORY is defined? Something like: > > #ifdef INCLUDE_MEMORY > # include <memory> > + #include "make-unique.h" > #endif > > This is because std::make_unique is defined in <memory> in C++14. This would > mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated. That's a good idea. > > (in the root namespace, rather than std::, which saves a bit more typing). > > It's less typing now, but it will be more churn once GCC requires C++14 (or later), at > which point you'll naturally want to get rid of the custom make_unique. More churn > since make_unique -> std::make_unique may require re-indentation of arguments, etc. > For that reason, I would suggest instead to put the function (and any other straight > standard library backport) in a 3-letter namespace already, like, gcc::make_unique > or gnu::make_unique. That way, when the time comes that GCC requires C++14, > the patch to replace gcc::make_unique won't have to worry about reindenting code, > it'll just replace gcc -> std. Or (when the time comes) don't change gcc->std and do: namespace gcc { using std::make_unique; } or just leave it in the global namespace as in your current patch, and at a later date add a using-declaration to the global namespace: using std::make_unique;
On 2022-07-12 2:45 p.m., Jonathan Wakely wrote: > On Tue, 12 Jul 2022 at 14:24, Pedro Alves wrote: >> >> On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote: >> >>> 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 >> >> Did you consider making gcc/system.h include gcc/make-unique.h itself >> if INCLUDE_MEMORY is defined? Something like: >> >> #ifdef INCLUDE_MEMORY >> # include <memory> >> + #include "make-unique.h" >> #endif >> >> This is because std::make_unique is defined in <memory> in C++14. This would >> mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated. > > That's a good idea. > >>> (in the root namespace, rather than std::, which saves a bit more typing). >> >> It's less typing now, but it will be more churn once GCC requires C++14 (or later), at >> which point you'll naturally want to get rid of the custom make_unique. More churn >> since make_unique -> std::make_unique may require re-indentation of arguments, etc. >> For that reason, I would suggest instead to put the function (and any other straight >> standard library backport) in a 3-letter namespace already, like, gcc::make_unique >> or gnu::make_unique. That way, when the time comes that GCC requires C++14, >> the patch to replace gcc::make_unique won't have to worry about reindenting code, >> it'll just replace gcc -> std. > > Or (when the time comes) don't change gcc->std and do: > > namespace gcc { > using std::make_unique; > } It will seem like a pointless indirection then, IMO. > > or just leave it in the global namespace as in your current patch, and > at a later date add a using-declaration to the global namespace: > > using std::make_unique; > That's not very idiomatic, though. Let me turn this into a reverse question: If GCC required C++14 today, would you be doing the above, either importing make_unique to the global namespace, or into namespace gcc? I think it's safe to say that, no, nobody would be doing that. So once GCC requires C++14, why would you want to preserve once-backported symbols in a namespace other than std, when you no longer have a reason to? It will just be another unnecessary thing that newcomers at that future time will have to learn.
On Tue, 12 Jul 2022 at 15:06, Pedro Alves <pedro@palves.net> wrote: > > On 2022-07-12 2:45 p.m., Jonathan Wakely wrote: > > On Tue, 12 Jul 2022 at 14:24, Pedro Alves wrote: > >> > >> On 2022-07-12 1:25 a.m., David Malcolm via Gcc-patches wrote: > >> > >>> 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 > >> > >> Did you consider making gcc/system.h include gcc/make-unique.h itself > >> if INCLUDE_MEMORY is defined? Something like: > >> > >> #ifdef INCLUDE_MEMORY > >> # include <memory> > >> + #include "make-unique.h" > >> #endif > >> > >> This is because std::make_unique is defined in <memory> in C++14. This would > >> mean fewer changes once GCC requires C++14 (or later) and this new header is eliminated. > > > > That's a good idea. > > > >>> (in the root namespace, rather than std::, which saves a bit more typing). > >> > >> It's less typing now, but it will be more churn once GCC requires C++14 (or later), at > >> which point you'll naturally want to get rid of the custom make_unique. More churn > >> since make_unique -> std::make_unique may require re-indentation of arguments, etc. > >> For that reason, I would suggest instead to put the function (and any other straight > >> standard library backport) in a 3-letter namespace already, like, gcc::make_unique > >> or gnu::make_unique. That way, when the time comes that GCC requires C++14, > >> the patch to replace gcc::make_unique won't have to worry about reindenting code, > >> it'll just replace gcc -> std. > > > > Or (when the time comes) don't change gcc->std and do: > > > > namespace gcc { > > using std::make_unique; > > } > > It will seem like a pointless indirection then, IMO. > > > > > or just leave it in the global namespace as in your current patch, and > > at a later date add a using-declaration to the global namespace: > > > > using std::make_unique; > > > > That's not very idiomatic, though. Let me turn this into a reverse question: > > If GCC required C++14 today, would you be doing the above, either importing make_unique > to the global namespace, or into namespace gcc? I think it's safe to say that, no, > nobody would be doing that. Erm, I might well do exactly that, for either case. I don't see a problem with 'using std::make_unique;' into the global namespace in GCC code. It's not a library header being included by arbitrary code, it's a single application that isn't going to have conflicts for some other ::make_unique defined in GCC (because the ::make_unique that is being proposed today would be removed once C++14's std::make_unique can be used). > So once GCC requires C++14, why would you want to preserve > once-backported symbols in a namespace other than std, when you no longer have a reason to? > It will just be another unnecessary thing that newcomers at that future time will have > to learn. I also don't see a problem with importing std::make_unique into namespace gcc for local use alongside other things in namespace gcc. I do consider that idiomatic. It says "the make_unique for gcc code is std::make_unique". It means you only need a 'using namespace gcc;' at the top of a source file and you get access to everything in namespace gcc, even if it is something like std::make_unique that was originally defined in a different namespace.
On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: >> So once GCC requires C++14, why would you want to preserve >> once-backported symbols in a namespace other than std, when you no longer have a reason to? >> It will just be another unnecessary thing that newcomers at that future time will have >> to learn. > > I also don't see a problem with importing std::make_unique into > namespace gcc for local use alongside other things in namespace gcc. I > do consider that idiomatic. It says "the make_unique for gcc code is > std::make_unique". It means you only need a 'using namespace gcc;' at > the top of a source file and you get access to everything in namespace > gcc, even if it is something like std::make_unique that was originally > defined in a different namespace. > If that's the approach, then GCC should import std::unique_ptr, std::move, std::foo, std::bar into the gcc namespace too, no? Are you really going to propose that?
On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net> wrote: > On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: > > >> So once GCC requires C++14, why would you want to preserve > >> once-backported symbols in a namespace other than std, when you no > longer have a reason to? > >> It will just be another unnecessary thing that newcomers at that future > time will have > >> to learn. > > > > I also don't see a problem with importing std::make_unique into > > namespace gcc for local use alongside other things in namespace gcc. I > > do consider that idiomatic. It says "the make_unique for gcc code is > > std::make_unique". It means you only need a 'using namespace gcc;' at > > the top of a source file and you get access to everything in namespace > > gcc, even if it is something like std::make_unique that was originally > > defined in a different namespace. > > > > If that's the approach, then GCC should import std::unique_ptr, std::move, > std::foo, std::bar into the gcc namespace too, no? Are you really going > to propose that? > No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine. >
On 2022-07-12 7:22 p.m., Jonathan Wakely wrote: > > > On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:pedro@palves.net>> wrote: > > On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: > > >> So once GCC requires C++14, why would you want to preserve > >> once-backported symbols in a namespace other than std, when you no longer have a reason to? > >> It will just be another unnecessary thing that newcomers at that future time will have > >> to learn. > > > > I also don't see a problem with importing std::make_unique into > > namespace gcc for local use alongside other things in namespace gcc. I > > do consider that idiomatic. It says "the make_unique for gcc code is > > std::make_unique". It means you only need a 'using namespace gcc;' at > > the top of a source file and you get access to everything in namespace > > gcc, even if it is something like std::make_unique that was originally > > defined in a different namespace. > > > > If that's the approach, then GCC should import std::unique_ptr, std::move, > std::foo, std::bar into the gcc namespace too, no? Are you really going > to propose that? > > > No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine. > It isn't a strawman, Jon. Maybe there's some miscommunication. The conversion started (and part of it is still quoted above), by thinking about what we'd do once we get to C++14, and my suggestion to optimize for that. When we get to the point when we require C++14, make_unique is no longer different from any other symbol in the std namespace, and there will be no reason to treat it differently anymore. Like, if someone at that point proposes to remove the global make_unique or gcc::make_unique, and replace all references with std::make_unique, there will be no real ground to object to that, why wouldn't you want it? This is very much like when you removed "gnu::unique_ptr" (not going to miss it) a few months back -- you replaced it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of history. Cheers, Pedro Alves
On Tue, 2022-07-12 at 17:40 +0100, Pedro Alves wrote: > On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: > > > > So once GCC requires C++14, why would you want to preserve I look forward to the happy day when we can use C++14 in GCC's implementation, but I don't see it happening anytime soon. GCC's needs may differ from those of GDB's. I'm not very familiar with GDB's insides, but, for example, GCC has its own garbage-collector which complicates everything to do with memory managements. Right now I have comments expressing ownership of some pointers, and e.g. "takes ownership of ...". It would be wonderful to take some baby steps into using C++11 to express the ownership directly in code. > > > once-backported symbols in a namespace other than std, when you > > > no longer have a reason to? > > > It will just be another unnecessary thing that newcomers at that > > > future time will have > > > to learn. > > > > I also don't see a problem with importing std::make_unique into > > namespace gcc for local use alongside other things in namespace > > gcc. I > > do consider that idiomatic. It says "the make_unique for gcc code > > is > > std::make_unique". It means you only need a 'using namespace gcc;' > > at > > the top of a source file and you get access to everything in > > namespace > > gcc, even if it is something like std::make_unique that was > > originally > > defined in a different namespace. Jonathan's idea sounds good to me. > > > > If that's the approach, then GCC should import std::unique_ptr, > std::move, > std::foo, std::bar into the gcc namespace too, no? Are you really > going > to propose that? Pedro, it feels to me like you're constructing a strawman here. Neither me nor Jonathan are proposing that. I just want to be able to comfortably use std::unique_ptr in GCC in the places for which it makes sense, and being able to use "make_unique" is a part of that. Hope this is constructive Dave
On 2022-07-12 7:36 p.m., Pedro Alves wrote: > On 2022-07-12 7:22 p.m., Jonathan Wakely wrote: >> >> >> On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:pedro@palves.net>> wrote: >> >> On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: >> >> >> So once GCC requires C++14, why would you want to preserve >> >> once-backported symbols in a namespace other than std, when you no longer have a reason to? >> >> It will just be another unnecessary thing that newcomers at that future time will have >> >> to learn. >> > >> > I also don't see a problem with importing std::make_unique into >> > namespace gcc for local use alongside other things in namespace gcc. I >> > do consider that idiomatic. It says "the make_unique for gcc code is >> > std::make_unique". It means you only need a 'using namespace gcc;' at >> > the top of a source file and you get access to everything in namespace >> > gcc, even if it is something like std::make_unique that was originally >> > defined in a different namespace. >> > >> >> If that's the approach, then GCC should import std::unique_ptr, std::move, >> std::foo, std::bar into the gcc namespace too, no? Are you really going >> to propose that? >> >> >> No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine. >> > > It isn't a strawman, Jon. Maybe there's some miscommunication. The conversion started (and part of it is > still quoted above), by thinking about what we'd do once we get to C++14, and my suggestion to optimize > for that. When we get to the point when we require C++14, make_unique is no longer different from any other > symbol in the std namespace, and there will be no reason to treat it differently anymore. Like, if someone at > that point proposes to remove the global make_unique or gcc::make_unique, and replace all references with > std::make_unique, there will be no real ground to object to that, why wouldn't you want it? This is very > much like when you removed "gnu::unique_ptr" (not going to miss it) a few months back -- you replaced > it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of history. Sorry to reply to myself -- but I'm not sure it is clear what I meant above in the last sentence, so let me try again: 'the "gnu::unique_ptr" wasn't rewritten as an import of std::unique_ptr into the gnu namespace just because of history.'
On 2022-07-12 7:36 p.m., David Malcolm wrote: > On Tue, 2022-07-12 at 17:40 +0100, Pedro Alves wrote: >>> >> >> If that's the approach, then GCC should import std::unique_ptr, >> std::move, >> std::foo, std::bar into the gcc namespace too, no? Are you really >> going >> to propose that? > > Pedro, it feels to me like you're constructing a strawman here. > Neither me nor Jonathan are proposing that. > See my other reply. I am actually appalled at how the conversion seems to have derailed. > I just want to be able to comfortably use std::unique_ptr in GCC in the > places for which it makes sense, and being able to use "make_unique" is > a part of that. My suggestion was simple: wrap your make_unique in namespace gcc, so that later on when we get to C++14, yanking it out causes as little churn as possible, with just a 3 letters for 3 letters replacement. I never thought this would be objectionable. If you don't want to do that, that's fine, the churn will happen in the future, but it's no big deal. You'll get to live with shorter make_unique for a few years (my bet is not forever). > > Hope this is constructive > Dave > >
On Tue, 12 Jul 2022, 19:36 Pedro Alves, <pedro@palves.net> wrote: > On 2022-07-12 7:22 p.m., Jonathan Wakely wrote: > > > > > > On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto: > pedro@palves.net>> wrote: > > > > On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: > > > > >> So once GCC requires C++14, why would you want to preserve > > >> once-backported symbols in a namespace other than std, when you > no longer have a reason to? > > >> It will just be another unnecessary thing that newcomers at that > future time will have > > >> to learn. > > > > > > I also don't see a problem with importing std::make_unique into > > > namespace gcc for local use alongside other things in namespace > gcc. I > > > do consider that idiomatic. It says "the make_unique for gcc code > is > > > std::make_unique". It means you only need a 'using namespace gcc;' > at > > > the top of a source file and you get access to everything in > namespace > > > gcc, even if it is something like std::make_unique that was > originally > > > defined in a different namespace. > > > > > > > If that's the approach, then GCC should import std::unique_ptr, > std::move, > > std::foo, std::bar into the gcc namespace too, no? Are you really > going > > to propose that? > > > > > > No, I don't follow the logic of "if you do it for one thing you must do > it for everything". That's a straw man. But I don't really mind how this > gets done. Your suggestion is fine. > > > > It isn't a strawman, Jon. Maybe there's some miscommunication. The > conversion started (and part of it is > still quoted above), by thinking about what we'd do once we get to C++14, > and my suggestion to optimize > for that. Yeah, and I don't think optimizing for indentation is the right trade off. Putting something in a namespace with a three-letter name just so you don't have to re-indent some statements in a few years seems odd. I see no technical difficulty replacing a custom make_unique (in any namespace) with std::make_unique at a later date. If a namespace made sense to avoid name clashes, or to enable finding functions by ADL, or other technical reasons, I'd be all for it. But no such rationale was given, and using a namespace for indentation just seems odd to me. But I really don't care. Putting it in the global namespace or a 'gcc' namespace or anything else appropriate to GCC is fine. I'll leave this thread now. I don't think this is very constructive and I'm sorry for objecting to the suggestion. When we get to the point when we require C++14, make_unique is no longer > different from any other > symbol in the std namespace, and there will be no reason to treat it > differently anymore. Like, if someone at > that point proposes to remove the global make_unique or gcc::make_unique, > and replace all references with > std::make_unique, there will be no real ground to object to that, why > wouldn't you want it? This is very > much like when you removed "gnu::unique_ptr" (not going to miss it) a few > months back -- you replaced > it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of > history. > > Cheers, > Pedro Alves >
On 2022-07-12 7:50 p.m., Jonathan Wakely wrote: > Yeah, and I don't think optimizing for indentation is the right trade off. Putting something in a namespace with a three-letter name just so you don't have to re-indent some statements in a few years seems odd. > > I see no technical difficulty replacing a custom make_unique (in any namespace) with std::make_unique at a later date. > > If a namespace made sense to avoid name clashes, or to enable finding functions by ADL, or other technical reasons, I'd be all for it. But no such rationale was given, and using a namespace for indentation just seems odd to me. > > But I really don't care. Putting it in the global namespace or a 'gcc' namespace or anything else appropriate to GCC is fine. I'll leave this thread now. I don't think this is very constructive and I'm sorry for objecting to the suggestion. That's fair. Cheers! Hope we'll be able to laugh about it in Prague!
On Tue, 12 Jul 2022 at 19:41, Pedro Alves <pedro@palves.net> wrote: > > On 2022-07-12 7:36 p.m., Pedro Alves wrote: > > On 2022-07-12 7:22 p.m., Jonathan Wakely wrote: > >> > >> > >> On Tue, 12 Jul 2022, 17:40 Pedro Alves, <pedro@palves.net <mailto:pedro@palves.net>> wrote: > >> > >> On 2022-07-12 4:14 p.m., Jonathan Wakely wrote: > >> > >> >> So once GCC requires C++14, why would you want to preserve > >> >> once-backported symbols in a namespace other than std, when you no longer have a reason to? > >> >> It will just be another unnecessary thing that newcomers at that future time will have > >> >> to learn. > >> > > >> > I also don't see a problem with importing std::make_unique into > >> > namespace gcc for local use alongside other things in namespace gcc. I > >> > do consider that idiomatic. It says "the make_unique for gcc code is > >> > std::make_unique". It means you only need a 'using namespace gcc;' at > >> > the top of a source file and you get access to everything in namespace > >> > gcc, even if it is something like std::make_unique that was originally > >> > defined in a different namespace. > >> > > >> > >> If that's the approach, then GCC should import std::unique_ptr, std::move, > >> std::foo, std::bar into the gcc namespace too, no? Are you really going > >> to propose that? > >> > >> > >> No, I don't follow the logic of "if you do it for one thing you must do it for everything". That's a straw man. But I don't really mind how this gets done. Your suggestion is fine. > >> > > > > It isn't a strawman, Jon. Maybe there's some miscommunication. The conversion started (and part of it is > > still quoted above), by thinking about what we'd do once we get to C++14, and my suggestion to optimize > > for that. When we get to the point when we require C++14, make_unique is no longer different from any other > > symbol in the std namespace, and there will be no reason to treat it differently anymore. Like, if someone at > > that point proposes to remove the global make_unique or gcc::make_unique, and replace all references with > > std::make_unique, there will be no real ground to object to that, why wouldn't you want it? This is very > > much like when you removed "gnu::unique_ptr" (not going to miss it) a few months back -- you replaced > > it by "std::unique_ptr"; gnu::unique_ptr wasn't kept just because of history. > > Sorry to reply to myself -- but I'm not sure it is clear what I meant above in the last sentence, so let > me try again: 'the "gnu::unique_ptr" wasn't rewritten as an import of std::unique_ptr into the gnu namespace > just because of history.' [OFFLIST] I considered doing exactly that. But because namespace gnu wasn't used anywhere else in GCC it didn't make sense. If it had been put in namespace gcc, which is still used elsewhere in the GCC codebase, I might have decided differently. But keeping namespace 'gnu' with no content except 'using std::unique_ptr;' would have been pointless. It wouldn't have made it easier for other things in namespace gnu to refer to it, because there were no other things. It wouldn't have allowed 'using namespace gnu;' to make various useful utilities available with a single using-directive, because that would only make one thing available, and 'using std::unique_ptr;' does that just as effectively.
On Tue, 12 Jul 2022 at 19:58, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> [OFFLIST]
Oops, sorry, reply-all fail. I meant to spare everybody any further
musing on this topic.
Really leaving the thread now!
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 + +/* 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 */