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?
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
{
--
1.8.5.3
