On October 23, 2017 8:15:20 PM GMT+02:00, David Malcolm <dmalc...@redhat.com> wrote: >On Mon, 2017-10-23 at 16:40 +0100, Pedro Alves wrote: >> On 10/23/2017 04:17 PM, Jonathan Wakely wrote: >> > On 23/10/17 17:07 +0200, Michael Matz wrote: >> > > Hi, >> > > >> > > On Mon, 23 Oct 2017, Richard Biener wrote: >> > > >> > > > I guess so. But we have to make gdb happy as well. It really >> > > > depends how >> > > > much each TU grows with the extra (unneeded) include grows in >> > > > C++11 and >> > > > C++04 mode. >> > > >> > > The c++ headers unconditionally included from system.h, with: >> > > >> > > % echo '#include <$name>' | g++-7 -E -x c++ - | wc -l >> > > new: 3564 >> > > cstring: 533 >> > > utility: 3623 >> > > memory: 28066 >> > >> > That's using the -std=gnu++4 default for g++-7, and for that mode >> > the header *is* needed, to get the definition of std::unique_ptr. >> > >> > For C++98 (when it isn't needed) that header is much smaller: >> > >> > tmp$ echo '#include <memory>' | g++ -E -x c++ - | wc -l >> > 28101 >> > tmp$ echo '#include <memory>' | g++ -E -x c++ - -std=gnu++98 | wc >> > -l >> > 4267 >> > >> > (Because it doesn't contain std::unique_ptr and std::shared_ptr >> > before >> > C++11). >> > >> > > compile time: >> > > % echo -e '#include <$name>\nint i;' | time g++-7 -c -x c++ - >> > > new: 0:00.06elapsed, 17060maxresident, 0major+3709minor >> > > cstring: 0:00.03elapsed, 13524maxresident, 0major+3075minor >> > > utility: 0:00.05elapsed, 16952maxresident, 0major+3776minor >> > > memory: 0:00.25elapsed, 40356maxresident, 0major+9764minor >> > > >> > > Hence, <memory> is not cheap at all, including it unconditionally >> > > from >> > > system.h when it isn't actually used by many things doesn't seem >> > > a good >> > > idea. >> > > >> >> I think the real question is whether it makes a difference in >> a full build. There won't be many translation units that >> don't include some other headers. (though of course I won't >> be surprised if it does make a difference.) >> >> If it's a real issue, you could fix this like how the >> other similar cases were handled by system.h, by adding this >> in system.h: >> >> #ifdef __cplusplus >> #ifdef INCLUDE_UNIQUE_PTR >> # include "unique-ptr.h" >> #endif >> #endif >> >> instead of unconditionally including <memory> there, >> and then translation units that want unique-ptr.h would >> do "#define INCLUDE_UNIQUE_PTR" instead of #include "unique-ptr.h", >> like done for a few other C++ headers. >> >> (I maintain that IMO this is kind of self-inflicted GCC pain due >> to the fact that "#pragma poison" poisons too much. If #pragma >> poison's behavior were adjusted (or a new variant/mode created) to >> ignore references to the poisoned symbol names in system headers (or >> something like that), then you wouldn't need this manual management >> of header dependencies in gcc/system.h and the corresponding >> '#define INCLUDE_FOO' contortions. There's nothing that you can >> reasonably >> do with a reference to a poisoned symbol in a system header, other >> than >> avoid having the system header have the '#pragma poison' in effect >> when >> its included, which leads to contortions like system.h's. Note that >> the poisoned names are _still used anyway_. So can we come up with >> a GCC change that would avoid having to worry about manually doing >> this? It'd likely help other projects too.) >> >> Thanks, >> Pedro Alves > >Here's a different patch, which instead moves the include of our >"unique-ptr.h" to system.h (conditionalized on INCLUDE_UNIQUE_PTR), >after the decl of "free" and before the redefinition of "abort". > >It also makes the include of <memory> in unique-ptr.h be conditional >on C++11 or later. > >Hence it makes the new stuff only be included for the places where >we're actually using unique_ptr. > >Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, >using gcc 4.8 for the initial bootstrap (hence testing both gnu+03 >and then gnu++14 in the selftests, for stage 1 and stages 2 and 3 >respectively). > >I don't know if it actually fixes the bootstrap issue seen with >clang on Darwin and OpenBSD, though - but I expect it to. > >OK for trunk?
OK. Richard. >gcc/ChangeLog: > PR bootstrap/82610 > * system.h: Conditionally include "unique-ptr.h" if > INCLUDE_UNIQUE_PTR is defined. > * unique-ptr-tests.cc: Remove include of "unique-ptr.h" in favor > of defining INCLUDE_UNIQUE_PTR before including "system.h". > >include/ChangeLog: > * unique-ptr.h: Make include of <memory> conditional on C++11 or > later. >--- > gcc/system.h | 10 ++++++++++ > gcc/unique-ptr-tests.cc | 2 +- > include/unique-ptr.h | 4 +++- > 3 files changed, 14 insertions(+), 2 deletions(-) > >diff --git a/gcc/system.h b/gcc/system.h >index f0664e9..1714af4 100644 >--- a/gcc/system.h >+++ b/gcc/system.h >@@ -720,6 +720,16 @@ extern int vsnprintf (char *, size_t, const char >*, va_list); > #define __builtin_expect(a, b) (a) > #endif > >+/* Some of the headers included by <memory> can use "abort" within a >+ namespace, e.g. "_VSTD::abort();", which fails after we use the >+ preprocessor to redefine "abort" as "fancy_abort" below. >+ Given that unique-ptr.h can use "free", we need to do this after >"free" >+ is declared but before "abort" is overridden. */ >+ >+#ifdef INCLUDE_UNIQUE_PTR >+# include "unique-ptr.h" >+#endif >+ > /* Redefine abort to report an internal error w/o coredump, and > reporting the location of the error in the source file. */ > extern void fancy_abort (const char *, int, const char *) >diff --git a/gcc/unique-ptr-tests.cc b/gcc/unique-ptr-tests.cc >index f5b72a8..d275694 100644 >--- a/gcc/unique-ptr-tests.cc >+++ b/gcc/unique-ptr-tests.cc >@@ -18,9 +18,9 @@ along with GCC; see the file COPYING3. If not see > <http://www.gnu.org/licenses/>. */ > > #include "config.h" >+#define INCLUDE_UNIQUE_PTR > #include "system.h" > #include "coretypes.h" >-#include "unique-ptr.h" > #include "selftest.h" > > #if CHECKING_P >diff --git a/include/unique-ptr.h b/include/unique-ptr.h >index c5ca031..0b6f11a 100644 >--- a/include/unique-ptr.h >+++ b/include/unique-ptr.h >@@ -74,7 +74,9 @@ > #ifndef GNU_UNIQUE_PTR_H > #define GNU_UNIQUE_PTR_H 1 > >-#include <memory> >+#if __cplusplus >= 201103 >+# include <memory> >+#endif > > namespace gnu > {
