[C++] Fix option handling when -std=gnu++14 is not used (PR 69865)

On 19.02.2016 11:56, Jakub Jelinek wrote:
>
> On Fri, Feb 19, 2016 at 10:50:34AM +0000, Bernd Edlinger wrote:
> > While I think that we should probably not define __GNUC_GNU_INLINE__ at all for C++,
> > because it is meaningless, I am warned that this could break (already broken) header files.
> 
> It is not meaningless.  The various headers need to know if it is safe to
> use the gnu_inline attribute in C++.
> 
> In any case, the desirable state is that e.g. the -E -dD output should be
> identical if you explicitly request the default -std= version vs. if it is
> set implicitly.  We should verify it is the case even for C.
> 
>         Jakub

I absolutely agree with you.
The correct solution is probably doing this:


and remove this hunk instead:

@@ -786,7 +790,7 @@ c_common_post_options (const char **pfilename)
   /* By default we use C99 inline semantics in GNU99 or C99 mode.  C99
      inline semantics are not supported in GNU89 or C89 mode.  */
   if (flag_gnu89_inline == -1)
-    flag_gnu89_inline = !flag_isoc99;
+    flag_gnu89_inline = c_dialect_cxx () || !flag_isoc99;
   else if (!flag_gnu89_inline && !flag_isoc99)
     error ("-fno-gnu89-inline is only supported in GNU99 or C99 mode");


Would you like that better?


Thanks
Bernd.

On Fri, Feb 19, 2016 at 11:08:48AM +0000, Bernd Edlinger wrote:
> On 19.02.2016 11:56, Jakub Jelinek wrote:
> >
> > On Fri, Feb 19, 2016 at 10:50:34AM +0000, Bernd Edlinger wrote:
> > > While I think that we should probably not define __GNUC_GNU_INLINE__ at all for C++,
> > > because it is meaningless, I am warned that this could break (already broken) header files.
> > 
> > It is not meaningless.  The various headers need to know if it is safe to
> > use the gnu_inline attribute in C++.
> > 
> > In any case, the desirable state is that e.g. the -E -dD output should be
> > identical if you explicitly request the default -std= version vs. if it is
> > set implicitly.  We should verify it is the case even for C.
> > 
> >         Jakub
> 
> I absolutely agree with you.
> The correct solution is probably doing this:
> 
> --- gcc/cp/cfns.h.jj	2016-01-04 15:30:50.000000000 +0100
> +++ gcc/cp/cfns.h	2016-02-19 12:00:15.730375049 +0100
> @@ -124,9 +124,6 @@
>  
>  #ifdef __GNUC__
>  __inline
> -#ifdef __GNUC_STDC_INLINE__
> -__attribute__ ((__gnu_inline__))
> -#endif
>  #endif
>  const char *
>  libc_name_p (register const char *str, register unsigned int len)

This is of course wrong.  cfns.h is a generated header, so you shouldn't
patch it.
If it is regenerated with a newer gperf (I have 3.0.4 installed), you get there:
@@ -124,7 +124,7 @@ hash (register const char *str, register
 
 #ifdef __GNUC__
 __inline
-#ifdef __GNUC_STDC_INLINE__
+#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
 __attribute__ ((__gnu_inline__))
 #endif
 #endif

	Jakub

On 19.02.2016 12:31, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 11:08:48AM +0000, Bernd Edlinger wrote:
> > On 19.02.2016 11:56, Jakub Jelinek wrote:
> > >
> > > On Fri, Feb 19, 2016 at 10:50:34AM +0000, Bernd Edlinger wrote:
> > > > While I think that we should probably not define __GNUC_GNU_INLINE__ at all for C++,
> > > > because it is meaningless, I am warned that this could break (already broken) header files.
> > >
> > > It is not meaningless.  The various headers need to know if it is safe to
> > > use the gnu_inline attribute in C++.
> > >
> > > In any case, the desirable state is that e.g. the -E -dD output should be
> > > identical if you explicitly request the default -std= version vs. if it is
> > > set implicitly.  We should verify it is the case even for C.
> > >
> > >         Jakub
> >
> > I absolutely agree with you.
> > The correct solution is probably doing this:
> >
> > --- gcc/cp/cfns.h.jj  2016-01-04 15:30:50.000000000 +0100
> > +++ gcc/cp/cfns.h     2016-02-19 12:00:15.730375049 +0100
> > @@ -124,9 +124,6 @@
> >
> >  #ifdef __GNUC__
> >  __inline
> > -#ifdef __GNUC_STDC_INLINE__
> > -__attribute__ ((__gnu_inline__))
> > -#endif
> >  #endif
> >  const char *
> >  libc_name_p (register const char *str, register unsigned int len)
> 
> This is of course wrong.  cfns.h is a generated header, so you shouldn't
> patch it.
> If it is regenerated with a newer gperf (I have 3.0.4 installed), you get there:
> @@ -124,7 +124,7 @@ hash (register const char *str, register
> 
>  #ifdef __GNUC__
>  __inline
> -#ifdef __GNUC_STDC_INLINE__
> +#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
>  __attribute__ ((__gnu_inline__))
>  #endif
>  #endif
> 
>         Jakub

Damnit!!

I dont know how this file is ever supposed to compile on a C99 compiler.
but with this change it does not even compile with -std=c90 any more:

cat test.c
#include "cfns.h"

gcc -std=c90 test.c
In file included from test.c:1:0:
cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
cfns.gperf:26:14: error: but not here

gcc -std=c99 test.c
In file included from test.c:1:0:
cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
cfns.gperf:26:14: error: but not here
cfns.gperf: In function 'libc_name_p':
cfns.gperf:328:34: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]


So this leaves only one solution, to define neither __GNUC_STDC_INLINE__
nor __GNUC_GNU_INLINE__ for C++, at least the inline and gnu_inline
is completely different on C and C++. But I am anxious this will break more
things than gperf, and I would like to fix this in a safe way.


Bernd.

On Fri, Feb 19, 2016 at 11:53:03AM +0000, Bernd Edlinger wrote:
> >  #ifdef __GNUC__
> >  __inline
> > -#ifdef __GNUC_STDC_INLINE__
> > +#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
> >  __attribute__ ((__gnu_inline__))
> >  #endif
> >  #endif
> > 
> >         Jakub
> 
> Damnit!!
> 
> I dont know how this file is ever supposed to compile on a C99 compiler.
> but with this change it does not even compile with -std=c90 any more:
> 
> cat test.c
> #include "cfns.h"
> 
> gcc -std=c90 test.c
> In file included from test.c:1:0:
> cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
> cfns.gperf:26:14: error: but not here
> 
> gcc -std=c99 test.c
> In file included from test.c:1:0:
> cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
> cfns.gperf:26:14: error: but not here
> cfns.gperf: In function 'libc_name_p':
> cfns.gperf:328:34: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
> 
> 
> So this leaves only one solution, to define neither __GNUC_STDC_INLINE__

Of course not, and that would be the wrong thing to do.
The definition spot of libc_name_p comes from gperf itself, the prototype
from cfns.gperf, which we can of course adjust.

> nor __GNUC_GNU_INLINE__ for C++, at least the inline and gnu_inline
> is completely different on C and C++. But I am anxious this will break more
> things than gperf, and I would like to fix this in a safe way.

IMNSHO we should just keep it consistent with what g++ e.g. 5.x did.
Thus,
$ g++ -E -dD -xc++ /dev/null -O2 -std=c++98 2>&1 | grep _INLINE_
#define __GNUC_GNU_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=c++11 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=c++14 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++98 2>&1 | grep _INLINE_
#define __GNUC_GNU_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++11 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++14 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 2>&1 | grep _INLINE_
#define __GNUC_GNU_INLINE__ 1
We want to define what we did with the explicit -std= options, and just
change the output in the default case (last invocation), to
#define __GNUC_STDC_INLINE__ 1
because the default is different.

	Jakub

On 19.02.2016 12:59, Jakub Jelinek wrote:
> 
> Of course not, and that would be the wrong thing to do.
> The definition spot of libc_name_p comes from gperf itself, the prototype
> from cfns.gperf, which we can of course adjust.
>

Yes, now I understand.  Thanks.
 
>
> IMNSHO we should just keep it consistent with what g++ e.g. 5.x did.
>Thus,
> $ g++ -E -dD -xc++ /dev/null -O2 -std=c++98 2>&1 | grep _INLINE_
> #define __GNUC_GNU_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=c++11 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=c++14 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++98 2>&1 | grep _INLINE_
> #define __GNUC_GNU_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++11 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++14 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 2>&1 | grep _INLINE_
> #define __GNUC_GNU_INLINE__ 1
> We want to define what we did with the explicit -std= options, and just
> change the output in the default case (last invocation), to
> #define __GNUC_STDC_INLINE__ 1
> because the default is different.
> 
>        Jakub

OK. I can do that.
I will send a new patch in the evening.


Thanks
Bernd.