Re: missing isinf declaration on solaris

GCC 4.9 build on Solaris 10 shows these warnings about isinf:

float.c: In function 'is_infinite':
float.c:178:2: warning: implicit declaration of function 'isinf'
[-Wimplicit-function-declaration]

See
http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dingo&dt=2014-09-23%2002%3A52%3A00&stg=make

isinf declaration is in <iso/math_c99.h> which is included by <math.h>,
but it's surrounded by #if defined(_STDC_C99) || _XOPEN_SOURCE - 0 >=
600 || defined(__C99FEATURES__). A couple of quick Google searches
suggests that some other projects have worked around this by always
defining __C99FEATURES__ even if compiling in C89 mode. __C99FEATURES__
is only used by math.h and fenv.h in /usr/include.

Should we just add -D__C99FEATURES__ to CPPFLAGS in
src/template/solaris, add our own declaration of isinf() or do something
else about the warning?

/ Oskari

Oskari Saarenmaa <os(at)ohmu(dot)fi> writes:
> GCC 4.9 build on Solaris 10 shows these warnings about isinf:
> float.c: In function 'is_infinite':
> float.c:178:2: warning: implicit declaration of function 'isinf'

Ugh.

> isinf declaration is in <iso/math_c99.h> which is included by <math.h>,
> but it's surrounded by #if defined(_STDC_C99) || _XOPEN_SOURCE - 0 >=
> 600 || defined(__C99FEATURES__). A couple of quick Google searches
> suggests that some other projects have worked around this by always
> defining __C99FEATURES__ even if compiling in C89 mode. __C99FEATURES__
> is only used by math.h and fenv.h in /usr/include.

> Should we just add -D__C99FEATURES__ to CPPFLAGS in
> src/template/solaris, add our own declaration of isinf() or do something
> else about the warning?

I'm worried that __C99FEATURES__ might do other, not-so-C89-compatible
things in later Solaris releases. Possibly that risk could be addressed
by having src/template/solaris make an OS version check before adding the
switch, but it'd be a bit painful probably.

Based on the #if you show, I'd be more inclined to think about defining
_XOPEN_SOURCE to get the result. There is precedent for that in
src/template/hpux which does
CPPFLAGS="$CPPFLAGS -D_XOPEN_SOURCE_EXTENDED"
I forget what-all _XOPEN_SOURCE_EXTENDED enables exactly, but if memory
serves there were a nontrivial number of now-considered-standard features
turned on by that in ancient HPUX releases. If you want to pursue this
route, you'd need to do a bit of digging to see what else _XOPEN_SOURCE
controls in Solaris and if there is some front-end feature macro (like
_XOPEN_SOURCE_EXTENDED) that you're supposed to set instead of touching
it directly.

regards, tom lane

24.09.2014, 15:25, Tom Lane kirjoitti:
> Oskari Saarenmaa <os(at)ohmu(dot)fi> writes:
>> GCC 4.9 build on Solaris 10 shows these warnings about isinf:
>> float.c: In function 'is_infinite':
>> float.c:178:2: warning: implicit declaration of function 'isinf'
>
> Ugh.
>
>> isinf declaration is in <iso/math_c99.h> which is included by <math.h>,
>> but it's surrounded by #if defined(_STDC_C99) || _XOPEN_SOURCE - 0 >=
>> 600 || defined(__C99FEATURES__). A couple of quick Google searches
>> suggests that some other projects have worked around this by always
>> defining __C99FEATURES__ even if compiling in C89 mode. __C99FEATURES__
>> is only used by math.h and fenv.h in /usr/include.
>
>> Should we just add -D__C99FEATURES__ to CPPFLAGS in
>> src/template/solaris, add our own declaration of isinf() or do something
>> else about the warning?
>
> I'm worried that __C99FEATURES__ might do other, not-so-C89-compatible
> things in later Solaris releases. Possibly that risk could be addressed
> by having src/template/solaris make an OS version check before adding the
> switch, but it'd be a bit painful probably.
>
> Based on the #if you show, I'd be more inclined to think about defining
> _XOPEN_SOURCE to get the result. There is precedent for that in
> src/template/hpux which does
> CPPFLAGS="$CPPFLAGS -D_XOPEN_SOURCE_EXTENDED"
> I forget what-all _XOPEN_SOURCE_EXTENDED enables exactly, but if memory
> serves there were a nontrivial number of now-considered-standard features
> turned on by that in ancient HPUX releases. If you want to pursue this
> route, you'd need to do a bit of digging to see what else _XOPEN_SOURCE
> controls in Solaris and if there is some front-end feature macro (like
> _XOPEN_SOURCE_EXTENDED) that you're supposed to set instead of touching
> it directly.

Looking at standards(5) and /usr/include/sys/feature_tests.h it looks
like _XOPEN_SOURCE_EXTENDED enables XPG4v2 environment.
_XOPEN_SOURCE=600 enables XPG6, but feature_tests.h also has this bit:

/*
* It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application
* using c99. The same is true for POSIX.1-1990, POSIX.2-1992, POSIX.1b,
* and POSIX.1c applications. Likewise, it is invalid to compile an XPG6
* or a POSIX.1-2001 application with anything other than a c99 or later
* compiler. Therefore, we force an error in both cases.
*/

so to enable XPG6 we'd need to use C99 mode anyway. Could we just use
-std=gnu99 (with -fgnu89-inline if required) with GCC on Solaris? ISTM
it would be cleaner to just properly enable c99 mode rather than define
an undocumented macro to use a couple of c99 declarations.

/ Oskari

On 2014-09-24 08:25:34 -0400, Tom Lane wrote:
> I'm worried that __C99FEATURES__ might do other, not-so-C89-compatible
> things in later Solaris releases. Possibly that risk could be addressed
> by having src/template/solaris make an OS version check before adding the
> switch, but it'd be a bit painful probably.

As we want to actually be able to compile with a C99 compiler I don't
see much harm in that? Many compilers these day have C99 stuff enabled
by default anyway...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Oskari Saarenmaa <os(at)ohmu(dot)fi> writes:
> ... so to enable XPG6 we'd need to use C99 mode anyway.

OK.

> Could we just use
> -std=gnu99 (with -fgnu89-inline if required) with GCC on Solaris? ISTM
> it would be cleaner to just properly enable c99 mode rather than define
> an undocumented macro to use a couple of c99 declarations.

Agreed, but what about non-GCC compilers?

regards, tom lane

24.09.2014, 16:21, Tom Lane kirjoitti:
> Oskari Saarenmaa <os(at)ohmu(dot)fi> writes:
>> ... so to enable XPG6 we'd need to use C99 mode anyway.
>
> OK.
>
>> Could we just use
>> -std=gnu99 (with -fgnu89-inline if required) with GCC on Solaris? ISTM
>> it would be cleaner to just properly enable c99 mode rather than define
>> an undocumented macro to use a couple of c99 declarations.
>
> Agreed, but what about non-GCC compilers?

Solaris Studio defaults to "-xc99=all,no_lib" which, according to the
man page, enables c99 language features but doesn't use c99 standard
library semantics. isinf is defined to be a macro by c99 and doesn't
require changing the c99 mode so I'd just keep using the defaults with
Solaris Studio for now.

For GCC

--- a/src/template/solaris
+++ b/src/template/solaris
@@ -0,0 +1,4 @@
+if test "$GCC" = yes ; then
+ CPPFLAGS="$CPPFLAGS -std=gnu99"
+fi
+

gets rid of the warnings and passes tests.

/ Oskari

On 9/24/14 9:21 AM, Tom Lane wrote:
> Oskari Saarenmaa <os(at)ohmu(dot)fi> writes:
>> ... so to enable XPG6 we'd need to use C99 mode anyway.
>
> OK.
>
>> Could we just use
>> -std=gnu99 (with -fgnu89-inline if required) with GCC on Solaris? ISTM
>> it would be cleaner to just properly enable c99 mode rather than define
>> an undocumented macro to use a couple of c99 declarations.
>
> Agreed, but what about non-GCC compilers?

Stick AC_PROG_CC_C99 into configure.in.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On 9/24/14 9:21 AM, Tom Lane wrote:
>> Agreed, but what about non-GCC compilers?

> Stick AC_PROG_CC_C99 into configure.in.

I think that's a bad idea, unless you mean to do it only on Solaris.
If we do that unconditionally, we will pretty much stop getting any
warnings about C99-isms on modern platforms. I am not aware that
there has been any agreement to move our portability goalposts up
to C99.

regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > On 9/24/14 9:21 AM, Tom Lane wrote:
> >> Agreed, but what about non-GCC compilers?
>
> > Stick AC_PROG_CC_C99 into configure.in.
>
> I think that's a bad idea, unless you mean to do it only on Solaris.
> If we do that unconditionally, we will pretty much stop getting any
> warnings about C99-isms on modern platforms. I am not aware that
> there has been any agreement to move our portability goalposts up
> to C99.

AFAIK we cannot move all the way to C99, because MSVC doesn't support
it. Presumably we're okay on the isinf() front because MSVC inherited
it from somewhere else (it's on BSD 4.3 according to my Linux manpage),
but other things are known not to work.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 9/24/14 4:26 PM, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> On 9/24/14 9:21 AM, Tom Lane wrote:
>>> Agreed, but what about non-GCC compilers?
>
>> Stick AC_PROG_CC_C99 into configure.in.
>
> I think that's a bad idea, unless you mean to do it only on Solaris.
> If we do that unconditionally, we will pretty much stop getting any
> warnings about C99-isms on modern platforms. I am not aware that
> there has been any agreement to move our portability goalposts up
> to C99.

I don't disagree with that concern. But let's look at it this way.

isinf() is a C99 function. If we want to have it, the canonical way is
to put the compiler into C99 mode. Anything else is just pure luck
(a.k.a. GNU extension). It's conceivable that on other platforms we
fail to detect a system isinf() function because of that. The only way
we learned about this is because the current configure check is
inconsistent on Solaris.

The first thing to fix is the configure check. It shouldn't detect a
function if it creates a warning when used.

What will happen then is probably that isinf() isn't detected on
Solaris, and we use the fallback implementation. Are we happy with
that? Maybe.

If not, well, then we need to put the compiler into C99 mode. But then
it's not a Solaris-specific problem anymore, because Solaris will then
behave like any other platform in this regard.

On 2014-09-24 17:39:19 -0300, Alvaro Herrera wrote:
> Tom Lane wrote:
> > Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > > On 9/24/14 9:21 AM, Tom Lane wrote:
> > >> Agreed, but what about non-GCC compilers?
> >
> > > Stick AC_PROG_CC_C99 into configure.in.
> >
> > I think that's a bad idea, unless you mean to do it only on Solaris.
> > If we do that unconditionally, we will pretty much stop getting any
> > warnings about C99-isms on modern platforms. I am not aware that
> > there has been any agreement to move our portability goalposts up
> > to C99.
>
> AFAIK we cannot move all the way to C99, because MSVC doesn't support
> it.

FWIW, msvc has supported a good part of C99 for long while. There's bits
and pieces it doesn't, but it's not things I think we're likely to
adopt. The most commonly complained about one is C99 variable
declarations. I can't see PG adopting that tomorrow.

From VS 2013 onwards they're trying hard to be C99 and C11 compatible.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

24.09.2014, 23:26, Tom Lane kirjoitti:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> On 9/24/14 9:21 AM, Tom Lane wrote:
>>> Agreed, but what about non-GCC compilers?
>
>> Stick AC_PROG_CC_C99 into configure.in.
>
> I think that's a bad idea, unless you mean to do it only on Solaris.
> If we do that unconditionally, we will pretty much stop getting any
> warnings about C99-isms on modern platforms. I am not aware that
> there has been any agreement to move our portability goalposts up
> to C99.

We don't currently try to select a C89 mode in configure.in, we just use
the default mode which may be C89 or C99 or something in between.

GCC docs used to say that once C99 support is complete it'll switch
defaults from gnu90 to gnu99, now the latest docs say that the default
will change to gnu11 at some point
(https://gcc.gnu.org/onlinedocs/gcc/Standards.html). Solaris Studio
already defaults to C99 and it looks like the latest versions of MSVC
also support it.

I think we should just enable C99 mode when possible to use the
backwards compatible features of it (like isinf). If C89 support is
still needed we should set up a new buildfarm animal that really uses a
C89 mode compiler and makes sure it compiles without warnings.

/ Oskari

Andres Freund wrote:
> On 2014-09-24 17:39:19 -0300, Alvaro Herrera wrote:

> > AFAIK we cannot move all the way to C99, because MSVC doesn't support
> > it.
>
> FWIW, msvc has supported a good part of C99 for long while. There's bits
> and pieces it doesn't, but it's not things I think we're likely to
> adopt. The most commonly complained about one is C99 variable
> declarations. I can't see PG adopting that tomorrow.

I got unlucky that I got bitten by precisely that missing feature twice,
I guess.

> From VS 2013 onwards they're trying hard to be C99 and C11 compatible.

Sounds great. Is VS2013 released already? If so, maybe we can think
about moving to C99 in 2016 or so; at least assuming you can build for
older Windows versions with the new compiler.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-09-25 10:56:56 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > From VS 2013 onwards they're trying hard to be C99 and C11 compatible.
>
> Sounds great. Is VS2013 released already?

Yes.

> If so, maybe we can think about moving to C99 in 2016 or so; at least
> assuming you can build for

I think we should simply pick and choose the features we want. And go
for it now. Besidesthe fact that one of them wasn't applied, I'd sent
patches a couple months back that'd allow setting up a buildfarm animal
failing for any patch going above C89 + chosen features.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-09-24 16:26:33 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > On 9/24/14 9:21 AM, Tom Lane wrote:
> >> Agreed, but what about non-GCC compilers?
>
> > Stick AC_PROG_CC_C99 into configure.in.
>
> I think that's a bad idea, unless you mean to do it only on Solaris.
> If we do that unconditionally, we will pretty much stop getting any
> warnings about C99-isms on modern platforms.

FWIW, that's not that much of an issue these days, because e.g. mylodon
runs with -std=c89 -Werror=c99-extensions - so far that seems to have been
reasonably accurate.

- Andres