fixinc regression for <math.h> with C++ on Solaris 9

fixinc regression for <math.h> with C++ on Solaris 9

[Gerald Pfeifer]

[ C++ gurus, below there is a four-liner which I believe to be
  valid C++, but a double check would be appreciated. ]

Hi Bruce,

the following difference between the fixincluded math.h on
sparc-sun-solaris2.9 seems to be responsible for a regression:

  /sw/gcc-3.2.3/lib/gcc-lib/sparc-sun-solaris2.9/3.2.3/include/math.h
  /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/include/math.h
  @@ -7,12 +7,6 @@
       This had to be done to correct non-standard usages in the
       original, manufacturer supplied header file.  */

  -#ifndef FIXINC_WRAP_MATH_H_MATH_EXCEPTION
  -#define FIXINC_WRAP_MATH_H_MATH_EXCEPTION 1
  -
  -#ifdef __cplusplus
  -#define exception __math_exception
  -#endif
   /*
    * Copyright 00/09/07 Sun Microsystems, Inc.  All Rights Reserved.
    */
  @@ -255,8 +249,3 @@ extern float modff __P((float, float *))
   #endif

   #endif /* _MATH_H */
  -#ifdef __cplusplus
  -#undef exception
  -#endif
  -
  -#endif  /* FIXINC_WRAP_MATH_H_MATH_EXCEPTION */

For the following C++ program

  #include <stdexcept>
  using namespace std;
  #include <cmath>

  int main() { return 0; }

we now get

  In file included from /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/../../../../include/c++/3.5.0/cmath:52,
     from x.cc:3:
  /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/include/math.h:202:
     error: use of `exception' is ambiguous
  /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/include/math.h:99:
    error:   first declared as `struct exception' here
  /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/../../../../include/c++/3.5.0/exception:53:
    error:   also declared as `class std::exception' here
  /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/include/math.h:202:
    error: conflicting declaration 'struct exception'
  /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/include/math.h:99:
    error: 'struct exception' has a previous declaration as `struct exception'

Gerald
-- 
Gerald Pfeifer (Jerry)   gerald@pfeifer.com   http://www.pfeifer.com/gerald/

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Gabriel Dos Reis]

Gerald Pfeifer <gerald@pfeifer.com> writes:

| [ C++ gurus, below there is a four-liner which I believe to be
|   valid C++, but a double check would be appreciated. ]

[...]

| For the following C++ program
| 
|   #include <stdexcept>
|   using namespace std;
|   #include <cmath>
| 
|   int main() { return 0; }


It is a valid C++ program.

-- Gaby

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Bruce Korb]

Gerald Pfeifer wrote:
> 
> [ C++ gurus, below there is a four-liner which I believe to be
>   valid C++, but a double check would be appreciated. ]
> 
> Hi Bruce,
> 
> the following difference between the fixincluded math.h on
> sparc-sun-solaris2.9 seems to be responsible for a regression:
> 
>   /sw/gcc-3.2.3/lib/gcc-lib/sparc-sun-solaris2.9/3.2.3/include/math.h
>   /sw/gcc-current/lib/gcc/sparc-sun-solaris2.9/3.5.0/include/math.h
>   @@ -7,12 +7,6 @@
>        This had to be done to correct non-standard usages in the
>        original, manufacturer supplied header file.  */
> 
>   -#ifndef FIXINC_WRAP_MATH_H_MATH_EXCEPTION
>   -#define FIXINC_WRAP_MATH_H_MATH_EXCEPTION 1
>   -
>   -#ifdef __cplusplus
>   -#define exception __math_exception
>   -#endif

I don't have CVS access here.  It might be interesting to do a ``cvs log''
on gcc/fixinc/tests/base/math.h and see what happened to the same code.
If it is still there, then either the fix does not trigger on the real header
but does on the test, or else someone committed an inclhack change without
running the fixinc ``make check'' on the result.  If it has been removed,
then you can determine the version it was removed in and look at the
associated inclhack.def change and try to find the cause.

Just from inspection, there are *two* patches related to exceptions:

  sunos_matherr_decl and math_exception

but I do not see how they could interact on the header in question.
They should be re-ordered, though.  It looks like the second fix would
put a ``struct exception;'' in before the ``#ifdef __cplusplus'' thing.
(The bypass clause should prevent the application of the fix on the
header in question:

> fix = {
>     hackname  = sunos_matherr_decl;
>     files     = math.h;
> 
>     /*  If matherr has a prototype already, the header needs no fix.  */
>     bypass    = 'matherr.*(struct exception|__MATH_EXCEPTION)';
>     select    = matherr;
> 
>     c_fix     = wrap;
>     c_fix_arg = "struct exception;\n";
> 
>     test_text = "extern int matherr();";
> };

and from the Sol 9 header:

> /*
>  * SVID
>  */
> extern int matherr __P((struct exception *));

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Gerald Pfeifer]

On Mon, 19 Jan 2004, Bruce Korb wrote:
> I don't have CVS access here.  It might be interesting to do a ``cvs log''
> on gcc/fixinc/tests/base/math.h and see what happened to the same code.

It seems the code is still there. The only relevant changes I found were
the following, which seems harmless,

  date: 2003/01/04 21:14:54;  author: korbb;  state: Exp;  lines: +6 -6
  branches:  1.11.4;
  When wrapping files, guard with both the fix name and the file name

and

  revision 1.13
  date: 2003/08/01 22:48:24;  author: neroden;  state: Exp;  lines: +1 -0
        * fixinc/inclhack.def (broken_cabs): Make matching more generous.
        * fixinc/fixincl.x: Regenerate.
        * fixinc/tests/base/math.h: Regenerate to match test_text change.

which lead me to the following:

  revision 1.160
  date: 2003/07/09 21:00:01;  author: neroden;  state: Exp;  lines: +3 -2
        * fixinc/inclhack.def (math_exception):  Improve bypass and comment.
        * fixinc/fixincl.x: Rebuild.

Index: inclhack.def
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fixinc/inclhack.def,v
retrieving revision 1.159
retrieving revision 1.160
diff -u -3 -p -r1.159 -r1.160
--- inclhack.def        8 Jul 2003 20:42:19 -0000       1.159
+++ inclhack.def        9 Jul 2003 21:00:01 -0000       1.160
@@ -1680,7 +1680,8 @@ fix = {


 /*
- *  Some math.h files define struct exception, which conflicts with
+ *  Some math.h files define struct exception (it's in the System V
+ *  Interface Definition), which conflicts with
  *  the class exception defined in the C++ file std/stdexcept.h.  We
  *  redefine it to __math_exception.  This is not a great fix, but I
  *  haven't been able to think of anything better.
@@ -1692,7 +1693,7 @@ fix = {
     hackname  = math_exception;
     files     = math.h;
     select    = "struct exception";
-    bypass    = 'We have a problem when using C\+\+';
+    bypass    = '__cplusplus';
     c_fix     = wrap;

     c_fix_arg = "#ifdef __cplusplus\n"

> If it is still there, then either the fix does not trigger on the real
> header but does on the test, or else someone committed an inclhack
> change without running the fixinc ``make check'' on the result.

From my results above, it seems that Nathanel indeed only ran the
test weeks after making the patch.  fixinc is black magic to me, but
might that be the culprit?

> Just from inspection, there are *two* patches related to exceptions:
>
>   sunos_matherr_decl and math_exception
>
> but I do not see how they could interact on the header in question.
> They should be re-ordered, though.  It looks like the second fix would
> put a ``struct exception;'' in before the ``#ifdef __cplusplus'' thing.
> (The bypass clause should prevent the application of the fix on the
> header in question:

Hmm, that one's definitely a bit too steep for me. :-)

If there's any patch you'd like me to test, or any (header) file you'd
like me to send, I'll happily do that, though.

Gerald

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Bruce Korb]

Gerald Pfeifer wrote:

> which lead me to the following:
> 
>   revision 1.160
>   date: 2003/07/09 21:00:01;  author: neroden;  state: Exp;  lines: +3 -2
>         * fixinc/inclhack.def (math_exception):  Improve bypass and comment.
>         * fixinc/fixincl.x: Rebuild.
> 
> Index: inclhack.def

> @@ -1692,7 +1693,7 @@ fix = {
>      hackname  = math_exception;
>      files     = math.h;
>      select    = "struct exception";
> -    bypass    = 'We have a problem when using C\+\+';
> +    bypass    = '__cplusplus';
>      c_fix     = wrap;
> 
>      c_fix_arg = "#ifdef __cplusplus\n"

Hmmm.  Odd, that.  The Sol 8 and 9 headers claim to be C++ aware
(they use "__cplusplus"), yet they contain:

  struct exception { ...

outside of the protection against C++.  We need to back out that change
and determine which headers (besides glibc) have the unprotected usage
of struct exception.

> > Just from inspection, there are *two* patches related to exceptions:
> >
> >   sunos_matherr_decl and math_exception
> >
> > but I do not see how they could interact on the header in question.
> > They should be re-ordered, though.  It looks like the second fix would
> > put a ``struct exception;'' in before the ``#ifdef __cplusplus'' thing.
> > (The bypass clause should prevent the application of the fix on the
> > header in question:
> 
> Hmm, that one's definitely a bit too steep for me. :-)

I'm trying to make the stuff accessible.  I am referring to
gcc/fixinc/tests/base/math.h:

> #ifndef FIXINC_WRAP_MATH_H_SUNOS_MATHERR_DECL
> #define FIXINC_WRAP_MATH_H_SUNOS_MATHERR_DECL 1
> 
> struct exception;  <<<<<<=====  OOPS!  comes from sunos_matherr_decl fix
> #ifndef FIXINC_WRAP_MATH_H_MATH_EXCEPTION
> #define FIXINC_WRAP_MATH_H_MATH_EXCEPTION 1
> 
> #ifdef __cplusplus
> #define exception __math_exception
> #endif

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Gerald Pfeifer]

On Mon, 19 Jan 2004, Bruce Korb wrote:
> Hmmm.  Odd, that.  The Sol 8 and 9 headers claim to be C++ aware
> (they use "__cplusplus"), yet they contain:
>
>   struct exception { ...
>
> outside of the protection against C++.  We need to back out that change
> and determine which headers (besides glibc) have the unprotected usage
> of struct exception.

Yes, I double checked, and it seems you are right.

Nathanael, can you please fix this regression by reverting your change?

(Or will you be able to have CVS access, Bruce?  If you have a patch, I
can easily apply that for you, by the way, if that's more convenient.)

Thanks,
Gerald
-- 
Gerald Pfeifer (Jerry)   gerald@pfeifer.com   http://www.pfeifer.com/gerald/

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Nathanael Nerode]

Gerald Pfeifer wrote:
> On Mon, 19 Jan 2004, Bruce Korb wrote:
> 
>>Hmmm.  Odd, that.  The Sol 8 and 9 headers claim to be C++ aware
>>(they use "__cplusplus"), yet they contain:
>>
>>  struct exception { ...
>>
>>outside of the protection against C++.  We need to back out that change
>>and determine which headers (besides glibc) have the unprotected usage
>>of struct exception.
> 
> 
> Yes, I double checked, and it seems you are right.
> 
> Nathanael, can you please fix this regression by reverting your change?
Ugh -- I'm not up to date on CVS and my internet connectivity is flaky 
(new cable modem on Wednesday) so I hope somebody else could do so 
instead.  This has to be done on both the 3.4 branch and mainline of course.

When you revert my change, please add a comment to the math_exception 
hack regarding the problem:

/* This should be bypassed on __cplusplus, but some supposedly C++
  * C++ aware headers, such as Solaris 8 and 9, don't wrap their struct
  * exception either.  So currently we bypass only for glibc, based on
  * a comment in the fixed glibc header.  Ick. */

> 
> (Or will you be able to have CVS access, Bruce?  If you have a patch, I
> can easily apply that for you, by the way, if that's more convenient.)
> 
> Thanks,
> Gerald

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Nathanael Nerode]

Bruce Korb wrote:
 > Gerald Pfeifer wrote:
 >
 >
 >>which lead me to the following:
 >>
 >>  revision 1.160
 >>  date: 2003/07/09 21:00:01;  author: neroden;  state: Exp;  lines: +3 -2
 >>        * fixinc/inclhack.def (math_exception):  Improve bypass and 
comment.
 >>        * fixinc/fixincl.x: Rebuild.
 >>
 >>Index: inclhack.def
 >
 >
 >>@@ -1692,7 +1693,7 @@ fix = {
 >>     hackname  = math_exception;
 >>     files     = math.h;
 >>     select    = "struct exception";
 >>-    bypass    = 'We have a problem when using C\+\+';
 >>+    bypass    = '__cplusplus';
 >>     c_fix     = wrap;
 >>
 >>     c_fix_arg = "#ifdef __cplusplus\n"
 >
 >
 > Hmmm.  Odd, that.  The Sol 8 and 9 headers claim to be C++ aware
 > (they use "__cplusplus"), yet they contain:
 >
 >   struct exception { ...
 >
 > outside of the protection against C++.  We need to back out that change
 > and determine which headers (besides glibc) have the unprotected usage
 > of struct exception.
 >
 >
 >>>Just from inspection, there are *two* patches related to exceptions:
 >>>
 >>>  sunos_matherr_decl and math_exception
 >>>
 >>>but I do not see how they could interact on the header in question.
 >>>They should be re-ordered, though.  It looks like the second fix would
 >>>put a ``struct exception;'' in before the ``#ifdef __cplusplus'' thing.
 >>>(The bypass clause should prevent the application of the fix on the
 >>>header in question:
 >>
 >>Hmm, that one's definitely a bit too steep for me. :-)
 >
 >
 > I'm trying to make the stuff accessible.  I am referring to
 > gcc/fixinc/tests/base/math.h:
 >
 >
 >>#ifndef FIXINC_WRAP_MATH_H_SUNOS_MATHERR_DECL
 >>#define FIXINC_WRAP_MATH_H_SUNOS_MATHERR_DECL 1
 >>
 >>struct exception;  <<<<<<=====  OOPS!  comes from sunos_matherr_decl fix
 >>#ifndef FIXINC_WRAP_MATH_H_MATH_EXCEPTION
 >>#define FIXINC_WRAP_MATH_H_MATH_EXCEPTION 1
 >>
 >>#ifdef __cplusplus
 >>#define exception __math_exception
 >>#endif

Could that be the culprit?  I'd be much happier if the struct exception; 
was coming from fixincludes (in which case we have to do less 
fixincluding) than if it's coming from bogus Solaris headers (in which 
case we have to do more fixincluding).

Not having Solaris, of course, I can't actually tell....

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Bruce Korb]

Nathanael Nerode wrote:
>  > I'm trying to make the stuff accessible.  I am referring to
>  > gcc/fixinc/tests/base/math.h:
>  >
>  >
>  >>#ifndef FIXINC_WRAP_MATH_H_SUNOS_MATHERR_DECL
>  >>#define FIXINC_WRAP_MATH_H_SUNOS_MATHERR_DECL 1
>  >>
>  >>struct exception;  <<<<<<=====  OOPS!  comes from sunos_matherr_decl fix
>  >>#ifndef FIXINC_WRAP_MATH_H_MATH_EXCEPTION
>  >>#define FIXINC_WRAP_MATH_H_MATH_EXCEPTION 1
>  >>
>  >>#ifdef __cplusplus
>  >>#define exception __math_exception
>  >>#endif
> 
> Could that be the culprit?  I'd be much happier if the struct exception;
> was coming from fixincludes (in which case we have to do less
> fixincluding) than if it's coming from bogus Solaris headers (in which
> case we have to do more fixincluding).
> 
> Not having Solaris, of course, I can't actually tell....

This is a nit that is most likely theoretical, but nonetheless possible.
The "sunos_matherr_decl" fix must precede the "math_exception" fix.

Re: fixinc regression for <math.h> with C++ on Solaris 9

[Nathanael Nerode]

Gerald Pfeifer wrote:
> On Mon, 19 Jan 2004, Bruce Korb wrote:
> 
>>I don't have CVS access here.  It might be interesting to do a ``cvs log''
>>on gcc/fixinc/tests/base/math.h and see what happened to the same code.
> 
> 
> It seems the code is still there. The only relevant changes I found were
> the following, which seems harmless,
> 
>   date: 2003/01/04 21:14:54;  author: korbb;  state: Exp;  lines: +6 -6
>   branches:  1.11.4;
>   When wrapping files, guard with both the fix name and the file name
> 
> and
> 
>   revision 1.13
>   date: 2003/08/01 22:48:24;  author: neroden;  state: Exp;  lines: +1 -0
>         * fixinc/inclhack.def (broken_cabs): Make matching more generous.
>         * fixinc/fixincl.x: Regenerate.
>         * fixinc/tests/base/math.h: Regenerate to match test_text change.
> 
> which lead me to the following:
> 
>   revision 1.160
>   date: 2003/07/09 21:00:01;  author: neroden;  state: Exp;  lines: +3 -2
>         * fixinc/inclhack.def (math_exception):  Improve bypass and comment.
>         * fixinc/fixincl.x: Rebuild.
> 
> Index: inclhack.def
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/fixinc/inclhack.def,v
> retrieving revision 1.159
> retrieving revision 1.160
> diff -u -3 -p -r1.159 -r1.160
> --- inclhack.def        8 Jul 2003 20:42:19 -0000       1.159
> +++ inclhack.def        9 Jul 2003 21:00:01 -0000       1.160
> @@ -1680,7 +1680,8 @@ fix = {
> 
> 
>  /*
> - *  Some math.h files define struct exception, which conflicts with
> + *  Some math.h files define struct exception (it's in the System V
> + *  Interface Definition), which conflicts with
>   *  the class exception defined in the C++ file std/stdexcept.h.  We
>   *  redefine it to __math_exception.  This is not a great fix, but I
>   *  haven't been able to think of anything better.
> @@ -1692,7 +1693,7 @@ fix = {
>      hackname  = math_exception;
>      files     = math.h;
>      select    = "struct exception";
> -    bypass    = 'We have a problem when using C\+\+';
> +    bypass    = '__cplusplus';
>      c_fix     = wrap;
> 
>      c_fix_arg = "#ifdef __cplusplus\n"
> 
> 
>>If it is still there, then either the fix does not trigger on the real
>>header but does on the test,
This seems likely, actually -- I made the bypass more generous so that 
it will be bypassed on more systems.  It's quite possible that the 
Solaris header claims to be c++-aware but doesn't do it correctly; I 
seem to remember something to that effect elsewhere.

* What was the original failure when using math.h with C++ on Solaris 9?
* Is Solaris 9's math.h properly guarded with #ifdef __cplusplus?
* Does it contain the text "__cplusplus"?
If it contains the text and isn't properly guarded, we have a winner.  :-(

  or else someone committed an inclhack
>>change without running the fixinc ``make check'' on the result.
> 
> 
>From my results above, it seems that Nathanel indeed only ran the
> test weeks after making the patch.  fixinc is black magic to me, but
> might that be the culprit?
> 
> 
>>Just from inspection, there are *two* patches related to exceptions:
>>
>>  sunos_matherr_decl and math_exception
>>
>>but I do not see how they could interact on the header in question.
>>They should be re-ordered, though.  It looks like the second fix would
>>put a ``struct exception;'' in before the ``#ifdef __cplusplus'' thing.
>>(The bypass clause should prevent the application of the fix on the
>>header in question:
> 
> 
> Hmm, that one's definitely a bit too steep for me. :-)
> 
> If there's any patch you'd like me to test, or any (header) file you'd
> like me to send, I'll happily do that, though.
> 
> Gerald
>