Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

Hi Bruce,

You're joining in this discussion starting in the middle, so you probably missed the earlier explanation.

On 11 Jul 2013, at 05:21, Bruce Evans <brde_at_optusnet.com.au> wrote:

> I don't see how any conforming program can access the isnan() function
> directly.  It is just as protected as __isnan() would be.  (isnan)()
> gives the function (the function prototype uses this), but conforming
> programs can't do that since the function might not exist.  Maybe some
> non-conforming program like autoconfig reads <math.h> or libm.a and
> creates a bug for C++.

The cmath header defines a template function isnan that invokes the isnan macro, but then undefines the isnan macro.  This causes a problem because when someone does something along the lines of using namespace std then they end up with two functions called isnan and the compiler gets to pick the one to use.  Unfortunately, std::isnan() returns a bool, whereas isnan() returns an int.  

The C++ headers are not required to be conforming C code, because they are not C, and our math.h causes namespace pollution in C++ when included from <cmath>.

> The FreeBSD isnan() implementation would be broken by removing the
> isnan() function from libm.a or ifdefing it in <math.h>.  Changing the
> function to __isnan() would cause compatibility problems.  The function
> is intentionally named isnan() to reduce compatibility problems.


On OS X this is avoided because their isnan() macro expands to call one of the __-prefixed inline functions (which adopt your suggestion of being implemented as x != x, for all types).  I am not sure that this is required for standards conformance, but it is certainly cleaner.  Your statement that having the function not called isnan() causes compatibility problems is demonstrably false, as neither OS X nor glibc has a function called isnan() and, unlike us, they do not experience problems with this macro.  

It would also be nice to implement these macros using _Generic when compiling in C11 mode, as it will allow the compiler to produce more helpful warning messages.  I would propose this implementation:


#if __has_builtin(__builtin_isnan)
#define isnan(x) __builtin_isnan(x)
#else
static __inline int
__isnanf(float __x)
{
        return (__x != __x);
}
static __inline int
__isnand(double __x)
{
        return (__x != __x);
}
static __inline int
__isnanl(long double __x)
{
        return (__x != __x);
}
#if __STDC_VERSION__ >= 201112L
#define isnan(x) _Generic((x),     \
        float: __isnanf(x),        \
        double: __isnand(x),       \
        long double: __isnanl(x))
#else
#define isnan(x)                                                \
        ((sizeof (x) == sizeof (float)) ? __isnanf(x)           \
             : (sizeof (x) == sizeof (double)) ? __isnand(x)    \
             : __isnanl(x))
#endif
#endif

For a trivial example of why this is an improvement in terms of error reporting, consider this trivial piece of code:

int is(int x)
{
        return isnan(x);
}

With our current implementation, this compiles and links without any warnings, although it will have somewhat interesting results at run time.  With the __builtin_isnan() version, clang reports this error:

isnan.c:35:15: error: floating point classification requires argument of
      floating point type (passed in 'int')
        return isnan(x);
                     ^
(and then some more about the macro expansion)

With the C11 version, it reports this error:

isnan.c:35:15: error: controlling expression type 'int' not compatible with any
      generic association type
        return isnan(x);
                     ^



David