Re: [bootstrap] Tentative fix for PR 54281
On 2012-08-19 07:18 , Arnaud Charlet wrote:
The conditionals cannot be removed for the time being because the
foreign language interface of the Ada part of the compiler is
hardcoded for C.
Barring a massive switch to the Ada language for the GNU project,
we would need to switch the foreign language interface to C++,
which might introduce various maintenance issues in the short term
(Arno CCed).
Yes, that would be a large hearthquake for both the compiler and the
run-time, which is unwanted at this stage. I guess the solution for
now is to more selectively export the various symbols as C symbols
and include header files as is.
Thanks.
One potential issue I see here is that as we start using more C++
headers (and more C++ in existing headers), we will reach a point in
which including something like system.h inside an extern C {} block
will fail.
None of the GCC headers should be included as C code anymore.
Diego.
Re: [bootstrap] Tentative fix for PR 54281
[Sorry for the delay]
So, I had failed to include Ada in my testing and my patch breaks it.
In several ada/*.c files, we forcefully include system.h as a C header:
#ifdef __cplusplus
extern C {
#endif
...
#include system.h
...
#ifdef __cplusplus
}
#endif
Eric, is this really needed now? We are including gmp.h from system.h
due to PR 54281. This is causing Ada builds to fail.
Would it be OK to remove all the '#ifdef __cplusplus' conditionals from
ada/*.c or is there something I'm missing?
The conditionals cannot be removed for the time being because the foreign
language interface of the Ada part of the compiler is hardcoded for C.
Barring a massive switch to the Ada language for the GNU project, we would need
to switch the foreign language interface to C++, which might introduce various
maintenance issues in the short term (Arno CCed).
--
Eric Botcazou
Re: [bootstrap] Tentative fix for PR 54281
The conditionals cannot be removed for the time being because the foreign language interface of the Ada part of the compiler is hardcoded for C. Barring a massive switch to the Ada language for the GNU project, we would need to switch the foreign language interface to C++, which might introduce various maintenance issues in the short term (Arno CCed). Yes, that would be a large hearthquake for both the compiler and the run-time, which is unwanted at this stage. I guess the solution for now is to more selectively export the various symbols as C symbols and include header files as is. Arno
Re: [bootstrap] Tentative fix for PR 54281
From: Diego Novillo dnovi...@google.com Date: Thu, 16 Aug 2012 21:12:56 +0200 On 12-08-16 15:00 , Diego Novillo 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). + PR bootstrap/54281 + * intl.h: Prevent libintl.h from being included when + ENABLE_NLS is not set. Regtested on a x86_64 F 8 system with gcc-4.1.2-33, no regressions at r190450 (with --disable-nls) compared to results at r190381 (without). brgds, H-P
Re: [bootstrap] Tentative fix for PR 54281
On Thu, 16 Aug 2012, Ian Lance Taylor wrote: On Thu, Aug 16, 2012 at 12:12 PM, Diego Novillo dnovi...@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 dnovi...@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.
Re: [bootstrap] Tentative fix for PR 54281
On 12-08-16 20:29 , Ian Lance Taylor wrote: I recommend that you add libintl.h to the AC_CHECK_HEADERS list in gcc/configure.ac Thanks. The attached patch implements the approach. Tested with --disable-nls and --enable-nls. Folks with 4.1 compilers, could you test it there? OK for trunk if testing succeeds? Thanks. Diego. commit 8583ba1f22ba310114bf64960f61f6bcc805e9c2 Author: Diego Novillo dnovi...@google.com Date: Thu Aug 16 14:27:49 2012 -0400 2012-08-17 Diego Novillo dnovi...@google.com PR bootstrap/54281 * configure.ac: Add libintl.h to AC_CHECK_HEADERS list. * config.in: Regenerate. * configure: Regenerate. * intl.h: Always include libintl.h if HAVE_LIBINTL_H is set. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c9a81d1..43b0af7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2012-08-17 Diego Novillo dnovi...@google.com + + PR bootstrap/54281 + * configure.ac: Add libintl.h to AC_CHECK_HEADERS list. + * config.in: Regenerate. + * configure: Regenerate. + * intl.h: Always include libintl.h if HAVE_LIBINTL_H is + set. + 2012-08-17 Richard Guenther rguent...@suse.de * bitmap.h (struct bitmap_element_def): GTY annotate next/prev. @@ -213,7 +222,7 @@ * config/tilegx/feedback.h: New file. * config/tilepro/feedback.h: New file. -2012-08-16 Diego Novillo dnovi...@google.com +2012-08-16 Diego Novillo dnovi...@google.com Revert diff --git a/gcc/config.in b/gcc/config.in index 6d986be..a9417df 100644 --- a/gcc/config.in +++ b/gcc/config.in @@ -1260,6 +1260,12 @@ #endif +/* Define to 1 if you have the libintl.h header file. */ +#ifndef USED_FOR_TARGET +#undef HAVE_LIBINTL_H +#endif + + /* Define to 1 if you have the limits.h header file. */ #ifndef USED_FOR_TARGET #undef HAVE_LIMITS_H diff --git a/gcc/configure b/gcc/configure index 1585bae..7f3489d 100755 --- a/gcc/configure +++ b/gcc/configure @@ -8248,7 +8248,7 @@ fi for ac_header in limits.h stddef.h string.h strings.h stdlib.h time.h iconv.h \ fcntl.h unistd.h sys/file.h sys/time.h sys/mman.h \ sys/resource.h sys/param.h sys/times.h sys/stat.h \ -direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h +direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h libintl.h do : as_ac_Header=`$as_echo ac_cv_header_$ac_header | $as_tr_sh` ac_fn_c_check_header_preproc $LINENO $ac_header $as_ac_Header @@ -17742,7 +17742,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 17744 configure +#line 17745 configure #include confdefs.h #if HAVE_DLFCN_H @@ -17848,7 +17848,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 17850 configure +#line 17851 configure #include confdefs.h #if HAVE_DLFCN_H diff --git a/gcc/configure.ac b/gcc/configure.ac index 579d9a8..6bfbf35 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -912,7 +912,7 @@ AC_HEADER_SYS_WAIT AC_CHECK_HEADERS(limits.h stddef.h string.h strings.h stdlib.h time.h iconv.h \ fcntl.h unistd.h sys/file.h sys/time.h sys/mman.h \ sys/resource.h sys/param.h sys/times.h sys/stat.h \ -direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h) +direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h libintl.h) # Check for thread headers. AC_CHECK_HEADER(thread.h, [have_thread_h=yes], [have_thread_h=]) diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..42ca873 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,16 @@ # define setlocale(category, locale) (locale) #endif +/* If libintl.h is available, include it before testing for NLS. If we + are building with --disable-nls and another header file includes + libintl.h, the stubs defined down below will cause syntax errors + when parsing libintl.h. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 + for details. */ +#ifdef HAVE_LIBINTL_H +# include libintl.h +#endif + #ifdef ENABLE_NLS -#include libintl.h extern void gcc_init_libintl (void); extern size_t gcc_gettext_width (const char *); #else
Re: [bootstrap] Tentative fix for PR 54281
Hi! On Fri, Aug 17, 2012 at 08:23:02AM -0400, Diego Novillo wrote: --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,16 @@ # define setlocale(category, locale) (locale) #endif +/* If libintl.h is available, include it before testing for NLS. If we + are building with --disable-nls and another header file includes + libintl.h, the stubs defined down below will cause syntax errors + when parsing libintl.h. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 + for details. */ +#ifdef HAVE_LIBINTL_H +# include libintl.h +#endif + #ifdef ENABLE_NLS -#include libintl.h extern void gcc_init_libintl (void); extern size_t gcc_gettext_width (const char *); #else Will that handle even the case where without --disable-nls intl/ creates its own libintl.h? Dunno which targets need that, but I'd guess configury wouldn't find it in that case. So perhaps it should be #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS) ? Jakub
Re: [bootstrap] Tentative fix for PR 54281
On Fri, Aug 17, 2012 at 5:32 AM, Jakub Jelinek ja...@redhat.com wrote: Will that handle even the case where without --disable-nls intl/ creates its own libintl.h? Dunno which targets need that, but I'd guess configury wouldn't find it in that case. So perhaps it should be #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS) ? Makes sense to me. Ian
Re: [bootstrap] Tentative fix for PR 54281
On 12-08-17 08:32 , Jakub Jelinek wrote: Hi! On Fri, Aug 17, 2012 at 08:23:02AM -0400, Diego Novillo wrote: --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,16 @@ # define setlocale(category, locale) (locale) #endif +/* If libintl.h is available, include it before testing for NLS. If we + are building with --disable-nls and another header file includes + libintl.h, the stubs defined down below will cause syntax errors + when parsing libintl.h. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 + for details. */ +#ifdef HAVE_LIBINTL_H +# include libintl.h +#endif + #ifdef ENABLE_NLS -#include libintl.h extern void gcc_init_libintl (void); extern size_t gcc_gettext_width (const char *); #else Will that handle even the case where without --disable-nls intl/ creates its own libintl.h? Dunno which targets need that, but I'd guess configury wouldn't find it in that case. So perhaps it should be #if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS) ? Sounds reasonable. Amended patch attached. Re-tested with --disable-nls. OK for trunk? diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c9a81d1..43b0af7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2012-08-17 Diego Novillo dnovi...@google.com + + PR bootstrap/54281 + * configure.ac: Add libintl.h to AC_CHECK_HEADERS list. + * config.in: Regenerate. + * configure: Regenerate. + * intl.h: Always include libintl.h if HAVE_LIBINTL_H is + set. + 2012-08-17 Richard Guenther rguent...@suse.de * bitmap.h (struct bitmap_element_def): GTY annotate next/prev. @@ -213,7 +222,7 @@ * config/tilegx/feedback.h: New file. * config/tilepro/feedback.h: New file. -2012-08-16 Diego Novillo dnovi...@google.com +2012-08-16 Diego Novillo dnovi...@google.com Revert diff --git a/gcc/config.in b/gcc/config.in index 6d986be..a9417df 100644 --- a/gcc/config.in +++ b/gcc/config.in @@ -1260,6 +1260,12 @@ #endif +/* Define to 1 if you have the libintl.h header file. */ +#ifndef USED_FOR_TARGET +#undef HAVE_LIBINTL_H +#endif + + /* Define to 1 if you have the limits.h header file. */ #ifndef USED_FOR_TARGET #undef HAVE_LIMITS_H diff --git a/gcc/configure b/gcc/configure index 1585bae..7f3489d 100755 --- a/gcc/configure +++ b/gcc/configure @@ -8248,7 +8248,7 @@ fi for ac_header in limits.h stddef.h string.h strings.h stdlib.h time.h iconv.h \ fcntl.h unistd.h sys/file.h sys/time.h sys/mman.h \ sys/resource.h sys/param.h sys/times.h sys/stat.h \ -direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h +direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h libintl.h do : as_ac_Header=`$as_echo ac_cv_header_$ac_header | $as_tr_sh` ac_fn_c_check_header_preproc $LINENO $ac_header $as_ac_Header @@ -17742,7 +17742,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 17744 configure +#line 17745 configure #include confdefs.h #if HAVE_DLFCN_H @@ -17848,7 +17848,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 17850 configure +#line 17851 configure #include confdefs.h #if HAVE_DLFCN_H diff --git a/gcc/configure.ac b/gcc/configure.ac index 579d9a8..6bfbf35 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -912,7 +912,7 @@ AC_HEADER_SYS_WAIT AC_CHECK_HEADERS(limits.h stddef.h string.h strings.h stdlib.h time.h iconv.h \ fcntl.h unistd.h sys/file.h sys/time.h sys/mman.h \ sys/resource.h sys/param.h sys/times.h sys/stat.h \ -direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h) +direct.h malloc.h langinfo.h ldfcn.h locale.h wchar.h libintl.h) # Check for thread headers. AC_CHECK_HEADER(thread.h, [have_thread_h=yes], [have_thread_h=]) diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..03be420 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,16 @@ # define setlocale(category, locale) (locale) #endif +/* If libintl.h is available, include it before testing for NLS. If we + are building with --disable-nls and another header file includes + libintl.h, the stubs defined down below will cause syntax errors + when parsing libintl.h. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 + for details. */ +#if defined(HAVE_LIBINTL_H) || defined(ENABLE_NLS) +# include libintl.h +#endif + #ifdef ENABLE_NLS -#include libintl.h extern void gcc_init_libintl (void); extern size_t gcc_gettext_width (const char *); #else
Re: [bootstrap] Tentative fix for PR 54281
On Fri, Aug 17, 2012 at 10:58:25AM -0400, Diego Novillo wrote: Sounds reasonable. Amended patch attached. OK for trunk? Yes, except for: diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c9a81d1..43b0af7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -213,7 +222,7 @@ * config/tilegx/feedback.h: New file. * config/tilepro/feedback.h: New file. -2012-08-16 Diego Novillo dnovi...@google.com +2012-08-16 Diego Novillo dnovi...@google.com Revert the above hunk. Jakub
Re: [bootstrap] Tentative fix for PR 54281
On Fri, Aug 17, 2012 at 11:05 AM, Jakub Jelinek ja...@redhat.com wrote: On Fri, Aug 17, 2012 at 10:58:25AM -0400, Diego Novillo wrote: Sounds reasonable. Amended patch attached. OK for trunk? Yes, except for: diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c9a81d1..43b0af7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -213,7 +222,7 @@ * config/tilegx/feedback.h: New file. * config/tilepro/feedback.h: New file. -2012-08-16 Diego Novillo dnovi...@google.com +2012-08-16 Diego Novillo dnovi...@google.com Revert the above hunk. Done. Committed. Thanks. Diego.
[bootstrap] Tentative fix for PR 54281
Richi, this implements your idea for fixing PR 54281. I don't
have an old enough compiler. Could you please test it in your
system?
I debated whether to remove the GENERATOR_FILE predicate from the
inclusion (some files include gmp.h unconditionally). I think it
could be removed, but only a minority of files build with
GENERATOR_FILE set, so it didn't seem harmful.
OK if tests pass?
Diego.
2012-08-16 Diego Novillo dnovi...@google.com
Richard Guenther rguent...@suse.de
PR bootstrap/54281
* double-int.h: Move including of gmp.h ...
* system.h: ... here.
* realmpfr.h: Do not include gmp.h.
* tree-ssa-loop-niter.c: Do not include gmp.h.
fortran/ChangeLog
* gfortran.h: Do not include gmp.h.
diff --git a/gcc/double-int.h b/gcc/double-int.h
index 3d9aa2c..7ea0528 100644
--- a/gcc/double-int.h
+++ b/gcc/double-int.h
@@ -20,10 +20,6 @@ along with GCC; see the file COPYING3. If not see
#ifndef DOUBLE_INT_H
#define DOUBLE_INT_H
-#ifndef GENERATOR_FILE
-#include gmp.h
-#endif
-
/* A large integer is currently represented as a pair of HOST_WIDE_INTs.
It therefore represents a number with precision of
2 * HOST_BITS_PER_WIDE_INT bits (it is however possible that the
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 7c4c0a4..611d16d 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1681,7 +1681,6 @@ gfc_intrinsic_sym;
EXPR_COMPCALL Function (or subroutine) call of a procedure pointer
component or type-bound procedure. */
-#include gmp.h
#include mpfr.h
#include mpc.h
#define GFC_RND_MODE GMP_RNDN
diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
index ab234e9..ada876e 100644
--- a/gcc/realmpfr.h
+++ b/gcc/realmpfr.h
@@ -22,7 +22,10 @@
#ifndef GCC_REALGMP_H
#define GCC_REALGMP_H
-#include gmp.h
+/* Note that we do not include gmp.h. It is included in system.h
+ because it wrecks intl.h when compiling in C++ mode.
+ See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 for details. */
+
#include mpfr.h
#include mpc.h
#include real.h
diff --git a/gcc/system.h b/gcc/system.h
index 9e7d503..0ccd991 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -1037,4 +1037,8 @@ helper_const_non_const_cast (const char *p)
#define DEBUG_VARIABLE
#endif
+#ifndef GENERATOR_FILE
+#include gmp.h
+#endif
+
#endif /* ! GCC_SYSTEM_H */
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index c719a74..4c67c26 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -38,7 +38,6 @@ along with GCC; see the file COPYING3. If not see
#include flags.h
#include diagnostic-core.h
#include tree-inline.h
-#include gmp.h
#define SWAP(X, Y) do { affine_iv *tmp = (X); (X) = (Y); (Y) = tmp; } while (0)
Re: [bootstrap] Tentative fix for PR 54281
On 12-08-16 09:08 , Richard Guenther wrote: It fixes it. Thus, ok from my side (if your testing succeeds). Worked here too. Committed to trunk. Diego.
Re: [bootstrap] Tentative fix for PR 54281
On 12-08-16 09:08 , Richard Guenther wrote:
On Thu, 16 Aug 2012, Diego Novillo wrote:
Richi, this implements your idea for fixing PR 54281. I don't
have an old enough compiler. Could you please test it in your
system?
I debated whether to remove the GENERATOR_FILE predicate from the
inclusion (some files include gmp.h unconditionally). I think it
could be removed, but only a minority of files build with
GENERATOR_FILE set, so it didn't seem harmful.
OK if tests pass?
It fixes it.
Thus, ok from my side (if your testing succeeds).
So, I had failed to include Ada in my testing and my patch breaks it.
In several ada/*.c files, we forcefully include system.h as a C header:
#ifdef __cplusplus
extern C {
#endif
...
#include system.h
...
#ifdef __cplusplus
}
#endif
Eric, is this really needed now? We are including gmp.h from system.h
due to PR 54281. This is causing Ada builds to fail.
Would it be OK to remove all the '#ifdef __cplusplus' conditionals from
ada/*.c or is there something I'm missing?
Thanks. Diego.
Re: [bootstrap] Tentative fix for PR 54281
On Thu, Aug 16, 2012 at 07:55:51AM -0400, Diego Novillo wrote: Richi, this implements your idea for fixing PR 54281. I don't have an old enough compiler. Could you please test it in your system? I debated whether to remove the GENERATOR_FILE predicate from the inclusion (some files include gmp.h unconditionally). I think it could be removed, but only a minority of files build with GENERATOR_FILE set, so it didn't seem harmful. OK if tests pass? This patch breaks building with gmp, cloog and isl in subdirectories of the source tree. echo timestamp s-options gawk -f ../../trunk/gcc/opt-functions.awk -f ../../trunk/gcc/opt-read.awk \ -f ../../trunk/gcc/opth-gen.awk \ optionlist tmp-options.h /bin/sh ../../trunk/gcc/../move-if-change tmp-options.h options.h echo timestamp s-options-h TARGET_CPU_DEFAULT= \ HEADERS=auto-host.h ansidecl.h DEFINES= \ /bin/sh ../../trunk/gcc/mkconfig.sh bconfig.h g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../trunk/gcc -I../../trunk/gcc/build -I../../trunk/gcc/../include -I../../trunk/gcc/../libcpp/include -I/home/magfr/src/gcc/build/./gmp -I/home/magfr/src/gcc/trunk/gmp -I/home/magfr/src/gcc/build/./mpfr -I/home/magfr/src/gcc/trunk/mpfr -I/home/magfr/src/gcc/trunk/mpc/src -I../../trunk/gcc/../libdecnumber -I../../trunk/gcc/../libdecnumber/bid -I../libdecnumber -DCLOOG_INT_GMP -I/home/magfr/src/gcc/build/./cloog/include -I/home/magfr/src/gcc/trunk/cloog/include -I../trunk/cloog/include -I/home/magfr/src/gcc/build/./isl/include -I/home/magfr/src/gcc/trunk/isl/include \ -o build/genconstants.o ../../trunk/gcc/genconstants.c In file included from /usr/include/sys/resource.h:25:0, from /usr/include/sys/wait.h:32, from ../../trunk/gcc/system.h:351, from ../../trunk/gcc/genconstants.c:29: /usr/include/bits/resource.h:133:18: error: declaration does not declare anything [-fpermissive] In file included from ../../trunk/gcc/genconstants.c:29:0: ../../trunk/gcc/system.h:443:23: error: declaration of C function `void* sbrk(int)' conflicts with In file included from ../../trunk/gcc/system.h:253:0, from ../../trunk/gcc/genconstants.c:29: /usr/include/unistd.h:1067:14: error: previous declaration `void* sbrk(intptr_t)' here In file included from ../../trunk/gcc/genconstants.c:29:0: ../../trunk/gcc/system.h:447:48: error: new declaration `char* strstr(const char*, const char*)' In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0, from ../../trunk/gcc/system.h:207, from ../../trunk/gcc/genconstants.c:29: /usr/include/string.h:323:22: error: ambiguates old declaration `const char* strstr(const char*, const char*)' In file included from ../../trunk/gcc/genconstants.c:29:0: ../../trunk/gcc/system.h:499:34: error: declaration of C function `const char* strsignal(int)' conflicts with In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0, from ../../trunk/gcc/system.h:207, from ../../trunk/gcc/genconstants.c:29: /usr/include/string.h:566:14: error: previous declaration `char* strsignal(int)' here In file included from ../../trunk/gcc/system.h:639:0, from ../../trunk/gcc/genconstants.c:29: ../../trunk/gcc/../include/libiberty.h:110:36: error: new declaration `char* basename(const char*)' In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0, from ../../trunk/gcc/system.h:207, from ../../trunk/gcc/genconstants.c:29: /usr/include/string.h:603:28: error: ambiguates old declaration `const char* basename(const char*)' make[3]: *** [build/genconstants.o] Error 1 make[3]: Leaving directory `/home/magfr/src/gcc/build/gcc' make[2]: *** [all-stage1-gcc] Error 2 make[2]: Leaving directory `/home/magfr/src/gcc/build' make[1]: *** [stage1-bubble] Error 2 make[1]: Leaving directory `/home/magfr/src/gcc/build' make: *** [all] Error 2 The problem seems to be that during configure time the test scripts include system.h, but it fails to add the gmp include directory to the compiler flags so the checks for sbrk, strstr and so on fails with configure:10294: checking whether sbrk is declared configure:10317: gcc -c -g -I../../trunk/gcc -I../../trunk/gcc/../include conftest.c 5 In file included from conftest.c:125:0: ../../trunk/gcc/system.h:1041:17: fatal error: gmp.h: No such file or directory compilation terminated. configure:10317: $? = 1 Reverting this patch makes the compiler build. /MF
Re: [bootstrap] Tentative fix for PR 54281
On Thu, Aug 16, 2012 at 1:50 PM, Magnus Fromreide ma...@lysator.liu.se wrote: On Thu, Aug 16, 2012 at 07:55:51AM -0400, Diego Novillo wrote: Richi, this implements your idea for fixing PR 54281. I don't have an old enough compiler. Could you please test it in your system? I debated whether to remove the GENERATOR_FILE predicate from the inclusion (some files include gmp.h unconditionally). I think it could be removed, but only a minority of files build with GENERATOR_FILE set, so it didn't seem harmful. OK if tests pass? This patch breaks building with gmp, cloog and isl in subdirectories of the source tree. Working on a fix. It also exposed problems in Ada. Diego.
Re: [bootstrap] Tentative fix for PR 54281
I have reverted my original fix and propose this one. My fix caused build failures in Ada (which includes system.h inside 'extern C' blocks) and it also breaks in-tree isl/cloog. Richi, I've tried building my own 4.1, but it doesn't build on my system. Could you try this patch? It includes libintl.h before undefining the names, this way the inclusion done from gmp.h turns into a nop. Thanks. Diego. commit 96e3d8108901c6f94fa3b0f2de769370688836cb Author: Diego Novillo dnovi...@google.com Date: Thu Aug 16 14:27:49 2012 -0400 2012-08-16 Diego Novillo dnovi...@google.com PR bootstrap/54281 * intl.h: Always include libintl.h. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a8ff00d..5252122 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,10 @@ 2012-08-16 Diego Novillo dnovi...@google.com + PR bootstrap/54281 + * intl.h: Always include libintl.h. + +2012-08-16 Diego Novillo dnovi...@google.com + Revert PR bootstrap/54281 diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..745fefd 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,8 @@ # define setlocale(category, locale) (locale) #endif -#ifdef ENABLE_NLS #include libintl.h +#ifdef ENABLE_NLS extern void gcc_init_libintl (void); extern size_t gcc_gettext_width (const char *); #else
Re: [bootstrap] Tentative fix for PR 54281
On Thu, 16 Aug 2012, Diego Novillo wrote: diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..745fefd 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,8 @@ # define setlocale(category, locale) (locale) #endif -#ifdef ENABLE_NLS #include libintl.h +#ifdef ENABLE_NLS I'm not sure it's safe to assume libintl.h exists on all hosts (e.g. MinGW) unless ENABLE_NLS. (If ENABLE_NLS, the intl/ directory will have built that header if the host didn't have it.) -- Joseph S. Myers jos...@codesourcery.com
Re: [bootstrap] Tentative fix for PR 54281
On 12-08-16 14:46 , Joseph S. Myers wrote: On Thu, 16 Aug 2012, Diego Novillo wrote: diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..745fefd 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -27,8 +27,8 @@ # define setlocale(category, locale) (locale) #endif -#ifdef ENABLE_NLS #include libintl.h +#ifdef ENABLE_NLS I'm not sure it's safe to assume libintl.h exists on all hosts (e.g. MinGW) unless ENABLE_NLS. (If ENABLE_NLS, the intl/ directory will have built that header if the host didn't have it.) I wonder if we couldn't simply '#define _LIBINTL_H 1' in the #else branch then. Something like this (though it seems a bit hacky to me): diff --git a/gcc/intl.h b/gcc/intl.h index c4db354..3da4738 100644 --- a/gcc/intl.h +++ b/gcc/intl.h @@ -32,6 +32,9 @@ 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). */ +# define _LIBINTL_H 1 /* Stubs. */ # undef textdomain # define textdomain(domain) (domain)
Re: [bootstrap] Tentative fix for PR 54281
On Thu, Aug 16, 2012 at 12:12 PM, Diego Novillo dnovi...@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 dnovi...@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
