[Mesa-dev] [PATCH 1/4] mesa: move fpclassify work-arounds into c99_math.h

[Jose Fonseca]

On 11/03/15 15:48, Brian Paul wrote:
> On 03/11/2015 08:20 AM, Brian Paul wrote:
>> On 03/11/2015 01:23 AM, Jose Fonseca wrote:
>>> On 11/03/15 01:41, Brian Paul wrote:
>>>> ---
>>>>   include/c99_math.h          | 52
>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>>   src/mesa/main/querymatrix.c | 51
>>>> +-------------------------------------------
>>>>   2 files changed, 53 insertions(+), 50 deletions(-)
>>>>
>>>> diff --git a/include/c99_math.h b/include/c99_math.h
>>>> index 0a49950..f1a6685 100644
>>>> --- a/include/c99_math.h
>>>> +++ b/include/c99_math.h
>>>> @@ -161,4 +161,56 @@ llrintf(float f)
>>>>   #endif
>>>>
>>>>
>>>> +#if defined(fpclassify)
>>>> +/* ISO C99 says that fpclassify is a macro.  Assume that any
>>>> implementation
>>>> + * of fpclassify, whether it's in a C99 compiler or not, will be a
>>>> macro.
>>>> + */
>>>> +#elif defined(__cplusplus)
>>>> +/* For C++, fpclassify() should be defined in <cmath> */
>>>
>>> Does MSVC's cmath implement fpclassify?  If not this #elif clause should
>>> be moved after MSC_VER clause.
>>
>> Looks like the answer is "no".
>
> Actually, I take that back.  This patch works as-is on MSVC.  I also
> added an fpclassify() test in a .cpp file and it builds too (both MSVC
> and gcc).
>
> The MSVC header #includes files are hard to follow, but somewhere,
> fpclassify() and the FP_* tokens are getting defined when compiling C++
> code.

It looks like fpclassify is part of C++11 -- 
http://en.cppreference.com/w/cpp/numeric/math/fpclassify  -- and MSVC 
has much better C++11 complaince than C99.

I doubt that the same is true for MSVC 2008 though, as C++11 was first 
added in 2012.

So we might need

   [...]
   #elif defined(__cplusplus) && (!defined(_MSC_VER) || _MSC_VER >= 1700)
   /* For C++11, fpclassify() should be defined in <cmath> */
   #elif defined(_MSC_VER)
   [...]

Anyway I doubt this will be enough...

The problem is that for C++11 fpclassify is a function inside std 
namespace  and to get it one needs to include <cmath> . But if there's a 
`include <math.h>` first the std::fpclassify is never defined. 
Furthermore certain system headers include <cmath>/<math.h> internally, 
so often apps don't even have full control over which gets included when.

Honestly, whoever thought of reusing C99 macros and functions in C++ std 
namespace did a huge disservice to everybody. :(


Mesa is not using C++11 by defaut on GCC, which is why it gets away. 
MSVC 2013 enables C++11 support by default.  I recently hit this issue 
(with isnan/isinf and not fpclassify) on Apitrace after enabling C++11 
support [1].


Anyway I don't want to make this any harder for you.  I think it's fine 
for you to commit whatever you have at the moment, and deal with the 
problems later when we actually hit them.  Just warning  that 
fpclassify/isinf/isnan and C++ business will probably come up again (and 
again)...


> If I move the  #elif defined(__cplusplus) case after the #elif
> defined(_MSC_VER) case then things blow up.
>
> I'll repost the patch with just the #error change.


[1] 
https://github.com/apitrace/apitrace/commit/fc740d702d3a1369603d08444ec8809d1a109160