[Mesa-dev] [PATCH] mesa: use c99 functions on non-linux platforms if supported

[Ian Romanick]

On 08/05/2013 06:52 PM, Jonathan Gray wrote:
> On Mon, Aug 05, 2013 at 09:46:58AM -0700, Chad Versace wrote:
>> On 08/01/2013 03:52 PM, Jonathan Gray wrote:
>>> On Thu, Aug 01, 2013 at 11:21:57AM -0700, Ian Romanick wrote:
>>>> On 08/01/2013 09:54 AM, Chad Versace wrote:
>>>>> On 08/01/2013 09:48 AM, Chad Versace wrote:
>>>>>> On 08/01/2013 12:27 AM, Jonathan Gray wrote:
>>>>>>> Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
>>>>>>> ---
>>>>>>>   src/mesa/main/imports.h | 2 +-
>>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git src/mesa/main/imports.h src/mesa/main/imports.h
>>>>>>> index 53e40b4..aa7dc49 100644
>>>>>>> --- src/mesa/main/imports.h
>>>>>>> +++ src/mesa/main/imports.h
>>>>>>> @@ -230,7 +230,7 @@ static inline int IS_INF_OR_NAN( float x )
>>>>>>>    *** LDEXPF: multiply value by an integral power of two
>>>>>>>    *** FREXPF: extract mantissa and exponent from value
>>>>>>>    ***/
>>>>>>> -#if defined(__gnu_linux__)
>>>>>>> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
>>>>>>>   /* C99 functions */
>>>>>>>   #define CEILF(x)   ceilf(x)
>>>>>>>   #define FLOORF(x)  floorf(x)
>>>>>>
>>>>>> This patch looks good to me, but I'm unable to test it.
>>>>>> On what platform did you test it?
>>>>>
>>>
>>> OpenBSD.
>>>
>>>>> I retract the "looks good". These functions are also available
>>>>> when compiling with `gcc -std=gnu89`, yet gnu89 does not
>>>>> define __STDC_VERSION__.
>>>>
>>>> This is the sort of issue I had in mind in my reply to this patch.
>>>> That's why autoconf (and other build systems) can figure out what
>>>> library functions are available and generate HAVE_CEILF, etc. macros
>>>> for them.
>>>
>>> There are already checks along the lines of what I'm adding
>>> as configure.ac makes gcc compile with -std=c99

True, but that doesn't make it right. :)  A lot of that code was added 
before we used autoconf, so using HAVE_<foo> macros would have been a 
lot of work.

>>>>> So, use
>>>>> #if defined(__gnu_linux) || (your_new_c99_checks)
>>>
>>> It doesn't really make sense to do that, especially as the
>>> rest of the file is different.
>>
>> I see your point. The #ifdefs throughout the rest of the file have
>> sensible conditions. This is the only #ifdef that uses __gnu_linux__.
>>
>> I checked the Linux manpages for all functions in this #ifdef block.
>> According to the manpages, at least for glibc, all the functions have
>> the same feature macro requirement.
>>
>> _BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE >= 600 || _ISOC99_SOURCE ||
>> _POSIX_C_SOURCE >= 200112L
>>
>> Do you see any problem with using this as the #ifdef condition? It seems
>> that this condition will be safe to use on BSD as well as Linux.
>
> The various _SOURCE macros control the visibility of definitions in
> headers and have to be defined by the program being compiled so I
> think you're a bit confused there, see
>
> http://linux.die.net/man/7/feature_test_macros
>
> The ifdef test is more about trying to spot systems that don't have c99
> functions (msvc and ?).  The impression I get from the glibc man page
> is you'll have to compile with --std=c99 (which mesa already does)
> or define _ISOC99_SOURCE before the relevant include.

hmm... we keep talking about compiling core Mesa with -std=c89 to 
improve compatibility with MSVC.  I could have sworn that had already 
been done, but grep disagrees.  Weird.

It looks like this may be less of an issue soon:

http://herbsutter.com/2012/05/03/reader-qa-what-about-vc-and-c99/

It seems that designated initializers may not be supported (ugh), but 
most of the "slips" we've had over the years have been 
mixed-code-and-declarations.