Thread: missing isinf declaration on solaris

missing isinf declaration on solaris

From
Oskari Saarenmaa
Date:
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



Re: missing isinf declaration on solaris

From
Tom Lane
Date:
Oskari Saarenmaa <os@ohmu.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 doesCPPFLAGS="$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



Re: missing isinf declaration on solaris

From
Oskari Saarenmaa
Date:
24.09.2014, 15:25, Tom Lane kirjoitti:
> Oskari Saarenmaa <os@ohmu.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-2001application 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



Re: missing isinf declaration on solaris

From
Andres Freund
Date:
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



Re: missing isinf declaration on solaris

From
Tom Lane
Date:
Oskari Saarenmaa <os@ohmu.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



Re: missing isinf declaration on solaris

From
Oskari Saarenmaa
Date:
24.09.2014, 16:21, Tom Lane kirjoitti:
> Oskari Saarenmaa <os@ohmu.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



Re: missing isinf declaration on solaris

From
Peter Eisentraut
Date:
On 9/24/14 9:21 AM, Tom Lane wrote:
> Oskari Saarenmaa <os@ohmu.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.




Re: missing isinf declaration on solaris

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.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



Re: missing isinf declaration on solaris

From
Alvaro Herrera
Date:
Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.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



Re: missing isinf declaration on solaris

From
Peter Eisentraut
Date:
On 9/24/14 4:26 PM, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.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.




Re: missing isinf declaration on solaris

From
Andres Freund
Date:
On 2014-09-24 17:39:19 -0300, Alvaro Herrera wrote:
> Tom Lane wrote:
> > Peter Eisentraut <peter_e@gmx.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



Re: missing isinf declaration on solaris

From
Oskari Saarenmaa
Date:
24.09.2014, 23:26, Tom Lane kirjoitti:
> Peter Eisentraut <peter_e@gmx.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



Re: missing isinf declaration on solaris

From
Alvaro Herrera
Date:
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



Re: missing isinf declaration on solaris

From
Andres Freund
Date:
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



Re: [HACKERS] missing isinf declaration on solaris

From
Andres Freund
Date:
On 2014-09-24 16:26:33 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.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