Dejagnu: use -isystem to include system header files.

Dejagnu: use -isystem to include system header files.

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

Hi Guys,

  I am going to check in the attached patch which imports a fix from
  the mainline dejagnu sources.  This fix is to use the -isystem
  switch to include system header files rather than -I.  This fixes
  several unexpected failures in the GCC and G++ testsuites where the
  newlib system header file <limits.h> is included in strict ANSI
  mode, and the compiler barfs on the #include_next directive.

Cheers
  Nick
  
dejagnu/ChangeLog  
2004-11-11  Nick Clifton  <nickc@redhat.com>

	* Import this patch from dejagnu mainline sources.  It fixes
	unexpected failures in the GCC and G++ testsuites where <limits.h>
	is #include's in strict ANSI mode.
	
	2002-11-12  Hans-Peter Nilsson  <hp@bitrange.com>

        * lib/libgloss.exp (newlib_include_flags): Use -isystem, not -I.
        (libio_include_flags, g++_include_flags, libstdc++_include_flags,
        winsup_include_flags): Ditto.
        * doc/user.sgml (Local Config File): Use -isystem, not -I, in
        example.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: isystem.patch --]
[-- Type: text/x-patch, Size: 10520 bytes --]

Index: dejagnu/lib/libgloss.exp
===================================================================
RCS file: /cvs/src/src/dejagnu/lib/libgloss.exp,v
retrieving revision 1.10
diff -c -3 -p -r1.10 libgloss.exp
*** dejagnu/lib/libgloss.exp	11 Feb 2003 13:51:32 -0000	1.10
--- dejagnu/lib/libgloss.exp	11 Nov 2004 11:52:12 -0000
*************** proc newlib_include_flags { args } {
*** 159,165 ****
  	if { ${newlib_dir} != "" } {
  	    set newlib_dir [file dirname ${newlib_dir}]
  	}
! 	return " -I$gccpath/newlib/targ-include -I${newlib_dir}"
      } else {
  	verbose "No newlib support for this target"
      }
--- 159,165 ----
  	if { ${newlib_dir} != "" } {
  	    set newlib_dir [file dirname ${newlib_dir}]
  	}
! 	return " -isystem $gccpath/newlib/targ-include -isystem ${newlib_dir}"
      } else {
  	verbose "No newlib support for this target"
      }
*************** proc libio_include_flags { args } {
*** 191,197 ****
      if { $libio_bin_dir != "" && $libio_src_dir != "" } {
  	set libio_src_dir [file dirname ${libio_src_dir}]
  	set libio_bin_dir [file dirname ${libio_bin_dir}];
! 	return " -I${libio_src_dir} -I${libio_bin_dir}"
      } else {
  	return ""
      }
--- 191,197 ----
      if { $libio_bin_dir != "" && $libio_src_dir != "" } {
  	set libio_src_dir [file dirname ${libio_src_dir}]
  	set libio_bin_dir [file dirname ${libio_bin_dir}];
! 	return " -isystem ${libio_src_dir} -isystem ${libio_bin_dir}"
      } else {
  	return ""
      }
*************** proc g++_include_flags { args } {
*** 226,247 ****
  
      set dir [lookfor_file ${srcdir} libg++]
      if { ${dir} != "" } {
! 	append flags " -I${dir} -I${dir}/src"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/std -I${dir}/include/c_std -I${dir}/libsupc++"
      }
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -I${dir} -I${dir}/stl"
      }
  
      return "$flags"
--- 226,247 ----
  
      set dir [lookfor_file ${srcdir} libg++]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir} -isystem ${dir}/src"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/std -isystem ${dir}/include/c_std -isystem ${dir}/libsupc++"
      }
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir} -isystem ${dir}/stl"
      }
  
      return "$flags"
*************** proc libstdc++_include_flags { args } {
*** 317,335 ****
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/std -I${dir}/include/c_std -I${dir}/libsupc++"
      }
  
      set gccpath [get_multilibs]
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -I${dir} -I${dir}/stl"
      }
  
      return "$flags"
--- 317,335 ----
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/std -isystem ${dir}/include/c_std -isystem ${dir}/libsupc++"
      }
  
      set gccpath [get_multilibs]
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir} -isystem ${dir}/stl"
      }
  
      return "$flags"
*************** proc libstdc++_link_flags { args } {
*** 376,382 ****
  
  #
  # Get the list of directories and -m options for gcc. This is kinda bogus that
! # generic testing software needs support for gcc hardwired in, but to make 
  # testing the GNU tools work right, there didn't seem to be any other way.
  #
  
--- 376,382 ----
  
  #
  # Get the list of directories and -m options for gcc. This is kinda bogus that
! # generic testing software needs support for gcc hardwired in, but to make
  # testing the GNU tools work right, there didn't seem to be any other way.
  #
  
*************** proc winsup_include_flags { args } {
*** 867,873 ****
  	set winsup_dir [lookfor_file ${srcdir} winsup/include/windows.h]
  	if { ${winsup_dir} != "" } {
  	    set winsup_dir [file dirname ${winsup_dir}]
! 	    return " -I${winsup_dir}"
  	}
      }
      verbose "No winsup support for this target"
--- 867,873 ----
  	set winsup_dir [lookfor_file ${srcdir} winsup/include/windows.h]
  	if { ${winsup_dir} != "" } {
  	    set winsup_dir [file dirname ${winsup_dir}]
! 	    return " -isystem ${winsup_dir}"
  	}
      }
      verbose "No winsup support for this target"
Index: dejagnu/doc/user.sgml
===================================================================
RCS file: /cvs/src/src/dejagnu/doc/user.sgml,v
retrieving revision 1.2
diff -c -3 -p -r1.2 user.sgml
*** dejagnu/doc/user.sgml	21 Apr 2002 08:47:03 -0000	1.2
--- dejagnu/doc/user.sgml	11 Nov 2004 11:52:13 -0000
***************
*** 803,815 ****
  
      <sect1 id=local xreflabel="Local Config File">
        <title>Local Config File</title>
!     
        <para>It is usually more convenient to keep these <emphasis>manual
        overrides</emphasis> in the <filename>site.exp</filename>
        local to each test directory, rather than in the global
        <filename>site.exp</filename> in the installed DejaGnu
        library. This file is mostly for supplying tool specific info
!       that is required by the test suite.</para>
  
        <para>All local <filename>site.exp</filename> files have
        two sections, separated by comment text. The first section is
--- 803,815 ----
  
      <sect1 id=local xreflabel="Local Config File">
        <title>Local Config File</title>
! 
        <para>It is usually more convenient to keep these <emphasis>manual
        overrides</emphasis> in the <filename>site.exp</filename>
        local to each test directory, rather than in the global
        <filename>site.exp</filename> in the installed DejaGnu
        library. This file is mostly for supplying tool specific info
!       that is required by the testsuite.</para>
  
        <para>All local <filename>site.exp</filename> files have
        two sections, separated by comment text. The first section is
***************
*** 869,875 ****
        <example>
          <title>Local Config File</title>
  
!       <programlisting>  
        ## these variables are automatically generated by make ##
        # Do not edit here. If you wish to override these values
        # add them to the last section
--- 869,875 ----
        <example>
          <title>Local Config File</title>
  
!       <programlisting>
        ## these variables are automatically generated by make ##
        # Do not edit here. If you wish to override these values
        # add them to the last section
***************
*** 879,904 ****
        set target_triplet i586-pc-linux-gnulibc1
        set target_alias i586-pc-linux-gnulibc1
        set CFLAGS ""
!       set CXXFLAGS "-I/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../libio -I$srcdir/../libg++/src -I$srcdir/../libio -I$srcdir/../libstdc++ -I$srcdir/../libstdc++/stl -L/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../libg++ -L/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../libstdc++"
        append LDFLAGS " -L/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../ld"
        set tmpdir /build/devo-builds/i586-pc-linux-gnulibc1/gcc/testsuite
        set srcdir "${srcdir}/testsuite"
        ## All variables above are generated by configure. Do Not Edit ##
!     
!       </programlisting>  
      </example>
  
      <para>This file defines the required fields for a local config
      file, namely the three config triplets, and the srcdir. It also
      defines several other Tcl variables that are used exclusivly by
!     the GCC test suite. For most test cases, the CXXFLAGS and LDFLAGS
      are supplied by DejaGnu itself for cross testing, but to test a
      compiler, GCC needs to manipulate these itself.</para>
  
      </sect1>
       <sect1 id=global xreflabel="Global Config File">
        <title>Global Config File</title>
!     
        <para>The master config file is where all the target specific
        config variables get set for a whole site get set. The idea is
        that for a centralized testing lab where people have to share a
--- 879,904 ----
        set target_triplet i586-pc-linux-gnulibc1
        set target_alias i586-pc-linux-gnulibc1
        set CFLAGS ""
!       set CXXFLAGS "-isystem /build/devo-builds/i586-pc-linux-gnulibc1/gcc/../libio -isystem $srcdir/../libg++/src -isystem $srcdir/../libio -isystem $srcdir/../libstdc++ -isystem $srcdir/../libstdc++/stl -L/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../libg++ -L/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../libstdc++"
        append LDFLAGS " -L/build/devo-builds/i586-pc-linux-gnulibc1/gcc/../ld"
        set tmpdir /build/devo-builds/i586-pc-linux-gnulibc1/gcc/testsuite
        set srcdir "${srcdir}/testsuite"
        ## All variables above are generated by configure. Do Not Edit ##
! 
!       </programlisting>
      </example>
  
      <para>This file defines the required fields for a local config
      file, namely the three config triplets, and the srcdir. It also
      defines several other Tcl variables that are used exclusivly by
!     the GCC testsuite. For most test cases, the CXXFLAGS and LDFLAGS
      are supplied by DejaGnu itself for cross testing, but to test a
      compiler, GCC needs to manipulate these itself.</para>
  
      </sect1>
       <sect1 id=global xreflabel="Global Config File">
        <title>Global Config File</title>
! 
        <para>The master config file is where all the target specific
        config variables get set for a whole site get set. The idea is
        that for a centralized testing lab where people have to share a

Re: Dejagnu: use -isystem to include system header files.

[Daniel Jacobowitz]

On Thu, Nov 11, 2004 at 11:58:15AM +0000, Nick Clifton wrote:
> Hi Guys,
> 
>   I am going to check in the attached patch which imports a fix from
>   the mainline dejagnu sources.  This fix is to use the -isystem
>   switch to include system header files rather than -I.  This fixes
>   several unexpected failures in the GCC and G++ testsuites where the
>   newlib system header file <limits.h> is included in strict ANSI
>   mode, and the compiler barfs on the #include_next directive.

This patch will break in-tree testing for yet other targets.  I believe
arm-elf was affected - anything which does not set
NO_IMPLICIT_EXTERN_C.  I discussed this with H-P on the dejagnu list
but never figured out a solution, but...

>         * lib/libgloss.exp (newlib_include_flags): Use -isystem, not -I.
>         (libio_include_flags, g++_include_flags, libstdc++_include_flags,
>         winsup_include_flags): Ditto.

... I strongly suspect that g++ and winsup should be left out.

-- 
Daniel Jacobowitz

Re: Dejagnu: use -isystem to include system header files.

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

Hi Daniel,

> This patch will break in-tree testing for yet other targets.  I believe
> arm-elf was affected - anything which does not set
> NO_IMPLICIT_EXTERN_C.  I discussed this with H-P on the dejagnu list
> but never figured out a solution, but...

>>        * lib/libgloss.exp (newlib_include_flags): Use -isystem, not -I.
>>        (libio_include_flags, g++_include_flags, libstdc++_include_flags,
>>        winsup_include_flags): Ditto.

> ... I strongly suspect that g++ and winsup should be left out.


Ok - what about this revision to the patched sources based upon Rob 
Savoye's suggestion that -isystem is only needed for newlib includes ?

It appears to work for the xstormy16 port and I did not detect any 
regression for the arm-elf port, so I think that it should be OK.

Assuming that you like this patch, shall I submit it to Rob for 
inclusion in the official sources as well ?

Cheers
   Nick

dejagnu/ChangeLog
2004-11-11  Nick Clifton  <nickc@redhat.com>

	* lib/libgloss.exp (libio_include_flags, g++_include_flags,
	winsup_include_flags): Revert previous patch, restoring the use of
	-I, for all libraries except newlib.  Newlib needs -isystem to
	avoid the problems with <limits.h> but the C++ and winsup
	libraries need -I because -isystem generates an implicit 'extern
	"C"' which may not be appropriate for certain targets.




[-- Attachment #2: libgloss.exp.patch --]
[-- Type: text/plain, Size: 4167 bytes --]

Index: dejagnu/lib/libgloss.exp
===================================================================
RCS file: /cvs/src/src/dejagnu/lib/libgloss.exp,v
retrieving revision 1.11
diff -c -3 -p -r1.11 libgloss.exp
*** dejagnu/lib/libgloss.exp	11 Nov 2004 11:55:11 -0000	1.11
--- dejagnu/lib/libgloss.exp	11 Nov 2004 15:46:03 -0000
*************** proc libio_include_flags { args } {
*** 191,197 ****
      if { $libio_bin_dir != "" && $libio_src_dir != "" } {
  	set libio_src_dir [file dirname ${libio_src_dir}]
  	set libio_bin_dir [file dirname ${libio_bin_dir}];
! 	return " -isystem ${libio_src_dir} -isystem ${libio_bin_dir}"
      } else {
  	return ""
      }
--- 191,197 ----
      if { $libio_bin_dir != "" && $libio_src_dir != "" } {
  	set libio_src_dir [file dirname ${libio_src_dir}]
  	set libio_bin_dir [file dirname ${libio_bin_dir}];
! 	return " -I${libio_src_dir} -I${libio_bin_dir}"
      } else {
  	return ""
      }
*************** proc g++_include_flags { args } {
*** 226,247 ****
  
      set dir [lookfor_file ${srcdir} libg++]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir} -isystem ${dir}/src"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/std -isystem ${dir}/include/c_std -isystem ${dir}/libsupc++"
      }
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir} -isystem ${dir}/stl"
      }
  
      return "$flags"
--- 226,247 ----
  
      set dir [lookfor_file ${srcdir} libg++]
      if { ${dir} != "" } {
! 	append flags " -I${dir} I${dir}/src"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/std -I${dir}/include/c_std -I${dir}/libsupc++"
      }
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -I${dir} -I${dir}/stl"
      }
  
      return "$flags"
*************** proc libstdc++_include_flags { args } {
*** 317,335 ****
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/std -isystem ${dir}/include/c_std -isystem ${dir}/libsupc++"
      }
  
      set gccpath [get_multilibs]
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir}/include -isystem ${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -isystem ${dir} -isystem ${dir}/stl"
      }
  
      return "$flags"
--- 317,335 ----
  
      set dir [lookfor_file ${srcdir} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/std -I${dir}/include/c_std -I${dir}/libsupc++"
      }
  
      set gccpath [get_multilibs]
  
      set dir [lookfor_file ${gccpath} libstdc++-v3]
      if { ${dir} != "" } {
! 	append flags " -I${dir}/include -I${dir}/include/${target_alias}"
      }
  
      set dir [lookfor_file ${srcdir} libstdc++]
      if { ${dir} != "" } {
! 	append flags " -I${dir} -I${dir}/stl"
      }
  
      return "$flags"
*************** proc winsup_include_flags { args } {
*** 867,873 ****
  	set winsup_dir [lookfor_file ${srcdir} winsup/include/windows.h]
  	if { ${winsup_dir} != "" } {
  	    set winsup_dir [file dirname ${winsup_dir}]
! 	    return " -isystem ${winsup_dir}"
  	}
      }
      verbose "No winsup support for this target"
--- 867,873 ----
  	set winsup_dir [lookfor_file ${srcdir} winsup/include/windows.h]
  	if { ${winsup_dir} != "" } {
  	    set winsup_dir [file dirname ${winsup_dir}]
! 	    return " -I${winsup_dir}"
  	}
      }
      verbose "No winsup support for this target"

Re: Dejagnu: use -isystem to include system header files.

[Daniel Jacobowitz]

On Thu, Nov 11, 2004 at 03:57:19PM +0000, Nick Clifton wrote:
> Hi Daniel,
> 
> >This patch will break in-tree testing for yet other targets.  I believe
> >arm-elf was affected - anything which does not set
> >NO_IMPLICIT_EXTERN_C.  I discussed this with H-P on the dejagnu list
> >but never figured out a solution, but...
> 
> >>       * lib/libgloss.exp (newlib_include_flags): Use -isystem, not -I.
> >>       (libio_include_flags, g++_include_flags, libstdc++_include_flags,
> >>       winsup_include_flags): Ditto.
> 
> >... I strongly suspect that g++ and winsup should be left out.
> 
> 
> Ok - what about this revision to the patched sources based upon Rob 
> Savoye's suggestion that -isystem is only needed for newlib includes ?
> 
> It appears to work for the xstormy16 port and I did not detect any 
> regression for the arm-elf port, so I think that it should be OK.
> 
> Assuming that you like this patch, shall I submit it to Rob for 
> inclusion in the official sources as well ?

I do like it, and I'd appreciate that!


-- 
Daniel Jacobowitz

Re: Dejagnu: use -isystem to include system header files.

[Hans-Peter Nilsson]

Yay cross-posting!  I helped by adding a list!

On Thu, 11 Nov 2004, Daniel Jacobowitz wrote:

> On Thu, Nov 11, 2004 at 11:58:15AM +0000, Nick Clifton wrote:
> > Hi Guys,
> >
> >   I am going to check in the attached patch which imports a fix from
> >   the mainline dejagnu sources.  This fix is to use the -isystem
> >   switch to include system header files rather than -I.  This fixes
> >   several unexpected failures in the GCC and G++ testsuites where the
> >   newlib system header file <limits.h> is included in strict ANSI
> >   mode, and the compiler barfs on the #include_next directive.
>
> This patch will break in-tree testing for yet other targets.

But presumably only for old broken systems: no non-obsolete
system should need to fake an extern "C" as is done when *not*
defining NO_IMPLICIT_EXTERN_C; they should all be C++-aware.

>  I believe
> arm-elf was affected - anything which does not set
> NO_IMPLICIT_EXTERN_C.

Bug in the arm-elf port: it should define NO_IMPLICIT_EXTERN_C.

Sounds like there's a bug in the -isystem interaction with
NO_IMPLICIT_EXTERN_C too.  (Like, always assume
NO_IMPLICIT_EXTERN_C for all passed -isystem options aka.
never fake 'extern "C"' for path-options given on the command
line.)

>  I discussed this with H-P on the dejagnu list
> but never figured out a solution, but...

Wot, there was no closure?  For the record, the original email:
<URL:http://lists.gnu.org/archive/html/bug-dejagnu/2002-10/msg00009.html>
(the reply was in 2002-11).  Our later conversation is at
<URL:http://lists.gnu.org/archive/html/dejagnu/2004-05/msg00003.html>.

I thought we had closure at
<URL:http://lists.gnu.org/archive/html/dejagnu/2004-05/msg00007.html>
or at least at
<URL:http://lists.gnu.org/archive/html/dejagnu/2004-05/msg00011.html>
but now that you bring it up, I just think it's more of a bug in
the arm-elf port and non-NO_IMPLICIT_EXTERN_C targets should be
xfailed for applicable tests. ;-)

> >         * lib/libgloss.exp (newlib_include_flags): Use -isystem, not -I.
> >         (libio_include_flags, g++_include_flags, libstdc++_include_flags,
> >         winsup_include_flags): Ditto.
>
> ... I strongly suspect that g++ and winsup should be left out.

Mh, a bit spurious.  Only actual C++ include directories could
be badly affected by non-NO_IMPLICIT_EXTERN_C-ness.

brgds, H-P

Re: Dejagnu: use -isystem to include system header files.

[Daniel Jacobowitz]

On Thu, Nov 11, 2004 at 07:25:19PM -0500, Hans-Peter Nilsson wrote:
> Bug in the arm-elf port: it should define NO_IMPLICIT_EXTERN_C.

Zack's been wanting to flip the default for NO_IMPLICIT_EXTERN_C for
ages.  But until that's been done - and done for a couple of years
- let's try to accomodate this in DejaGNU, OK :-)

> Wot, there was no closure?  For the record, the original email:
> <URL:http://lists.gnu.org/archive/html/bug-dejagnu/2002-10/msg00009.html>
> (the reply was in 2002-11).  Our later conversation is at
> <URL:http://lists.gnu.org/archive/html/dejagnu/2004-05/msg00003.html>.
> 
> I thought we had closure at
> <URL:http://lists.gnu.org/archive/html/dejagnu/2004-05/msg00007.html>
> or at least at
> <URL:http://lists.gnu.org/archive/html/dejagnu/2004-05/msg00011.html>
> but now that you bring it up, I just think it's more of a bug in
> the arm-elf port and non-NO_IMPLICIT_EXTERN_C targets should be
> xfailed for applicable tests. ;-)

There was closure that I needed to go try something, which I never got
around to :-)

-- 
Daniel Jacobowitz

Re: Dejagnu: use -isystem to include system header files.

[Zack Weinberg]

Daniel Jacobowitz <drow@false.org> writes:

> On Thu, Nov 11, 2004 at 07:25:19PM -0500, Hans-Peter Nilsson wrote:
>> Bug in the arm-elf port: it should define NO_IMPLICIT_EXTERN_C.
>
> Zack's been wanting to flip the default for NO_IMPLICIT_EXTERN_C for
> ages.  But until that's been done - and done for a couple of years
> - let's try to accomodate this in DejaGNU, OK :-)

I agree with Dan, but would like to point out that any newlib target
(I'm assuming arm-elf is such) ought to be able to take
NO_IMPLICIT_EXTERN_C right now.

zw

Re: Dejagnu: use -isystem to include system header files.

[Nick Clifton]

Hi Richard,

> The issue wasn't really about picking up gcc/include/limits.h over
> newlib/include/limits.h, it was about how we processed the newlib
> version when in strict ANSI mode.  That should be handled correctly when
> we use -isystem (because that relaxes the rules).  So I think we should
> try and get the search order back to what it was before, but when still
> using -isystem.

I am not sure that this is possible.  The <build-dir>/gcc/include 
directory is added via an -isystem switch that is synthesised by the gcc 
(or xgcc) compiler driver.  It is processed before any-isystem switch 
specified by the user on the command line so that gcc's fixed versions 
of system header files will be included before the systems.  Thus if the 
dejagnu test harness uses the -isystem switch to add the 
newlib/libc/include directory it is always going to be after 
<build-dir>/gcc/include in the list of directories searched.

This sounds correct to me.  The intention is that the fixed version of 
limits.h should be included before the system version of limits.h.  Thus 
builtins-config.h should not assume that #include <limits.h> will return 
the system limits.h.  It might well return a fixed version instead.

Cheers
   Nick

RE: Dejagnu: use -isystem to include system header files.

[Richard Earnshaw]

> > I think the gcc/include directory must be added implicitly 
> from the -B
> > option.  It would appear that these add -isystem type include
> > directories, so it might be just a matter of ordering the -B and
> > -isystem options appropriately.
> 
> But - how would this help in the situation where -ansi and -pedantic 
> have been specified as well.  In those cases we do not want 
> to get the 
> limits.h file from newlib.

The issue wasn't really about picking up gcc/include/limits.h over
newlib/include/limits.h, it was about how we processed the newlib
version when in strict ANSI mode.  That should be handled correctly when
we use -isystem (because that relaxes the rules).  So I think we should
try and get the search order back to what it was before, but when still
using -isystem.

R.

Re: Dejagnu: use -isystem to include system header files.

[Nick Clifton]

Hi Richard,

>> Or maybe 
>>builtins-config.h could include say <stdio.h> rather than <limits.h> so 
>>that it would pickup the newlib version and not the gcc version ?

> That might be OK for this case, but I'm not sure if will solve the
> problem generally.

I have confirmed that using <stdio.h> in place of <limits.h> does fix 
the unexpected failures.

Is there actually a general problem ?  The issue seems to be which 
version of <limits.h> is included by a test case using the dejagnu test 
harness.  If we include the newlib version then tests that use strict 
ANSI parsing will fail.  If we include the version in the gcc build 
directory then tests that assume that <limits.h> has come from newlib 
will fail.

It seems to me that using -isystem to include header files in the newlib 
include directories is the correct thing to do.  They are system header 
files after all.  Assuming that you can determine whether you are going 
to link with a C-library created by newlib by #include'ing <limits.h> 
seems dodgy to me.  Using <stdio.h> instead of <limits.h> is a 
workaround, but really the testcases ought to provide some weak aliases 
for the missing functions and then the _NEWLIB_VERSION check would be 
unnecessary.  (It would limit the tests to only being run by targets 
which support weak aliases but I do not consider this to be a serious 
restriction).

> I think the gcc/include directory must be added implicitly from the -B
> option.  It would appear that these add -isystem type include
> directories, so it might be just a matter of ordering the -B and
> -isystem options appropriately.

But - how would this help in the situation where -ansi and -pedantic 
have been specified as well.  In those cases we do not want to get the 
limits.h file from newlib.

Cheers
   Nick

Re: Dejagnu: use -isystem to include system header files.

[Richard Earnshaw]

On Thu, 2004-11-18 at 09:19, Nick Clifton wrote:
> Hi Richard,
> 
> > Nick Clifton wrote:
> >         I am going to check in the attached patch which imports a fix from
> >         the mainline dejagnu sources.  This fix is to use the -isystem
> >         switch to include system header files rather than -I.  This fixes
> >         several unexpected failures in the GCC and G++ testsuites where the
> >         newlib system header file <limits.h> is included in strict ANSI
> >         mode, and the compiler barfs on the #include_next directive.
> >         
> > Unfortunately this patch causes regressions on the gcc builtins tests. 
> > These tests rely on detecting newlib by looking for the definition of
> > _NEWLIB_VERSION being added by including limits.h; but the change in the
> > search order means that we now pick up a dummy version of newlib.h from
> > the gcc include directory.  
> > 
> > With your patch the search path has now become
> > 
> >  /work/rearnsha/gnu/egcs/gcc/include
> >  /work/rearnsha/gnu/egcs/arm-elf/./newlib/targ-include
> >  /home/rearnsha/gnusrc/egcs-cross/newlib/libc/include
> > 
> > Whereas previously the gcc/include directory came later in the search.
> 
> Hmmm, maybe newlib could provide the "l" variants of the builtin 
> functions ?  What are these functions anyway ? 

Long double.  I'm not sure if newlib wants to go that way, but if it
does it's probably a fair amount of work, especially since long double
means different things on different targets.

>
>  Or maybe 
> builtins-config.h could include say <stdio.h> rather than <limits.h> so 
> that it would pickup the newlib version and not the gcc version ?
> 

That might be OK for this case, but I'm not sure if will solve the
problem generally.

> Alternatively - can you think of another way of solving the problem that 
> my patch was originally fixing ?  Namely that several GCC and G++ tests 
> fail because they include <limits.h> whilst in strict ANSI mode and this 
> fails because the newlib limits.h uses the GNU extension #include_next 
> directive.  My first patch to solve this -  by undefining __GNUC__ if 
> __STRICT_ANSI__ was defined was rejected on the gcc lists.

I think the gcc/include directory must be added implicitly from the -B
option.  It would appear that these add -isystem type include
directories, so it might be just a matter of ordering the -B and
-isystem options appropriately.

R.

Re: Dejagnu: use -isystem to include system header files.

[Nick Clifton]

Hi Richard,

> Nick Clifton wrote:
>         I am going to check in the attached patch which imports a fix from
>         the mainline dejagnu sources.  This fix is to use the -isystem
>         switch to include system header files rather than -I.  This fixes
>         several unexpected failures in the GCC and G++ testsuites where the
>         newlib system header file <limits.h> is included in strict ANSI
>         mode, and the compiler barfs on the #include_next directive.
>         
> Unfortunately this patch causes regressions on the gcc builtins tests. 
> These tests rely on detecting newlib by looking for the definition of
> _NEWLIB_VERSION being added by including limits.h; but the change in the
> search order means that we now pick up a dummy version of newlib.h from
> the gcc include directory.  
> 
> With your patch the search path has now become
> 
>  /work/rearnsha/gnu/egcs/gcc/include
>  /work/rearnsha/gnu/egcs/arm-elf/./newlib/targ-include
>  /home/rearnsha/gnusrc/egcs-cross/newlib/libc/include
> 
> Whereas previously the gcc/include directory came later in the search.

Hmmm, maybe newlib could provide the "l" variants of the builtin 
functions ?  What are these functions anyway ?  Or maybe 
builtins-config.h could include say <stdio.h> rather than <limits.h> so 
that it would pickup the newlib version and not the gcc version ?

Alternatively - can you think of another way of solving the problem that 
my patch was originally fixing ?  Namely that several GCC and G++ tests 
fail because they include <limits.h> whilst in strict ANSI mode and this 
fails because the newlib limits.h uses the GNU extension #include_next 
directive.  My first patch to solve this -  by undefining __GNUC__ if 
__STRICT_ANSI__ was defined was rejected on the gcc lists.

Cheers
   Nick

Re: Dejagnu: use -isystem to include system header files.

[Richard Earnshaw]

Nick Clifton wrote:
        I am going to check in the attached patch which imports a fix from
        the mainline dejagnu sources.  This fix is to use the -isystem
        switch to include system header files rather than -I.  This fixes
        several unexpected failures in the GCC and G++ testsuites where the
        newlib system header file <limits.h> is included in strict ANSI
        mode, and the compiler barfs on the #include_next directive.
        
Unfortunately this patch causes regressions on the gcc builtins tests. 
These tests rely on detecting newlib by looking for the definition of
_NEWLIB_VERSION being added by including limits.h; but the change in the
search order means that we now pick up a dummy version of newlib.h from
the gcc include directory.  

With your patch the search path has now become

 /work/rearnsha/gnu/egcs/gcc/include
 /work/rearnsha/gnu/egcs/arm-elf/./newlib/targ-include
 /home/rearnsha/gnusrc/egcs-cross/newlib/libc/include

Whereas previously the gcc/include directory came later in the search.

R.