| Message ID | 502D4638.40009@google.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 16, 2012 at 12:12 PM, Diego Novillo <dnovillo@google.com> wrote: > > This is the patch I'm currently testing. I need someone with a very old > toolchain (4.1 or earlier) to also give this a try (the original problem > does not occur in g++ more recent than 4.1). > > > Thanks. Diego. > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index a8ff00d..8798b8f 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,5 +1,11 @@ > > 2012-08-16 Diego Novillo <dnovillo@google.com> > > + PR bootstrap/54281 > + * intl.h: Prevent libintl.h from being included when > + ENABLE_NLS is not set. It's hard to convince myself that this patch is portable. I don't think we can assume that every system that provides <libintl.h> uses _LIBINTL_H as the preprocessor guard. The issue is that we must not #include <libintl.h> after #defining those macros. So the fix is to #include <libintl.h> in all cases before #defining those macros. Your proposal of unconditionally doing #include <libintl.h> is not a good idea, because <libintl.h> is not available on every system. But we are compiling host files here, so we can just use autoconf. I recommend that you add libintl.h to the AC_CHECK_HEADERS list in gcc/configure.ac, and then change intl.h to do #ifdef HAVE_LIBINTL_H #include <libintl.h> #endif before the #ifdef ENABLE_NLS test. Ian
On Thu, 16 Aug 2012, Ian Lance Taylor wrote: > On Thu, Aug 16, 2012 at 12:12 PM, Diego Novillo <dnovillo@google.com> wrote: > > > > This is the patch I'm currently testing. I need someone with a very old > > toolchain (4.1 or earlier) to also give this a try (the original problem > > does not occur in g++ more recent than 4.1). > > > > > > Thanks. Diego. > > > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > > index a8ff00d..8798b8f 100644 > > --- a/gcc/ChangeLog > > +++ b/gcc/ChangeLog > > @@ -1,5 +1,11 @@ > > > > 2012-08-16 Diego Novillo <dnovillo@google.com> > > > > + PR bootstrap/54281 > > + * intl.h: Prevent libintl.h from being included when > > + ENABLE_NLS is not set. > > It's hard to convince myself that this patch is portable. I don't > think we can assume that every system that provides <libintl.h> uses > _LIBINTL_H as the preprocessor guard. > > The issue is that we must not #include <libintl.h> after #defining > those macros. So the fix is to #include <libintl.h> in all cases > before #defining those macros. Your proposal of unconditionally doing > #include <libintl.h> is not a good idea, because <libintl.h> is not > available on every system. But we are compiling host files here, so > we can just use autoconf. I recommend that you add libintl.h to the > AC_CHECK_HEADERS list in gcc/configure.ac, and then change intl.h to > do > > #ifdef HAVE_LIBINTL_H > #include <libintl.h> > #endif > > before the #ifdef ENABLE_NLS test. Yes. Still I think including system headers from random gcc headers is bogus (the gmp.h include from double-int.h), we have system.h exactly to be able to setup things that the includes will work before poisoning anything or for systems that require special care. The configure check including system.h should define GENERATOR_FILE eventually. Or we need to split system.h. Richard.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a8ff00d..8798b8f 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,11 @@ 2012-08-16 Diego Novillo <dnovillo@google.com> + PR bootstrap/54281 + * intl.h: Prevent libintl.h from being included when + ENABLE_NLS is not set. + +2012-08-16 Diego Novillo <dnovillo@google.com> + Revert PR bootstrap/54281 diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..e10e357 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -32,6 +32,11 @@ extern void gcc_init_libintl (void); extern size_t gcc_gettext_width (const char *); #else +/* Prevent libintl.h from being included, since we are truncating + some functions (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281). */ +# ifndef _LIBINTL_H +# define _LIBINTL_H 1 +# endif /* Stubs. */ # undef textdomain # define textdomain(domain) (domain)