incorrect implementation of isnan and similar functions

[llvmbot]

Bugzilla Link	18218
Version	unspecified
OS	All
Depends On	#18759
Reporter	LLVM Bugzilla Contributor
CC	@dlj-NaN,@eugenis,@K-ballo,@mclow,@zygoloid,@seanm

Extended Description

The standard specifies in [c.math] that isnan and other classification functions must have the following overloads:

bool isnan(float x);
bool isnan(double x);
bool isnan(long double x);

Implementing it as a template seems like a good idea, except that the following valid code is rejected:

#include

struct A {
operator float() { return 0.0; }
};

int main() {
std::isnan(A());
}

The template should be enabled for every type that is convertible to an arithmetic type, to make the implementation indistinguishable from what is specified in the standard.

[mclow]

The template should be enabled for every type that is convertible to an
arithmetic type, to make the implementation indistinguishable from what is
specified in the standard.

Can you point me to where it says that?
[ That it should work with things that are convertible to an arithmetic type? ]

[llvmbot]

It doesn't say that. That was just my comment that if someone wants to implement it as a template, it should behave as if it was implemented as it says in the standard, so it should work with convertible types.

From [c.math]:

The classification/comparison functions behave the same as the C macros with the corresponding names defined in 7.12.3, Classification macros, and 7.12.14, Comparison macros in the C Standard. Each function is overloaded for the three floating-point types, as follows:

(...)
bool isnan(float x);
(...)
bool isnan(double x);
(...)
bool isnan(long double x);
(...)

From [global.functions]:

A call to a global or non-member function signature described in Clauses 18 through 30 and Annex D shall behave as if the implementation declared no additional global or non-member function signatures.

If the implementation declared those three and no additional signatures, it would work for convertible types, so if it is implemented in some other way, it should also work for convertible types.

[mclow]

A call to a global or non-member function signature described in Clauses 18
through 30 and Annex D shall behave as if the implementation declared no
additional global or non-member function signatures.

If the implementation declared those three and no additional signatures, it
would work for convertible types, so if it is implemented in some other way,
it should also work for convertible types.

Ok, that's just wrong. [ I'm not saying that you are wrong; it's quite possible that the standard is inconsistent here. ]
Because if that was true, then you couldn't call any of them with an int (for example).

int -> float, int -> double, int -> long double are all equally good, and the compiler would just claim that the calls are ambiguous.

void foo ( float ) {}
void foo ( double ) {}
void foo ( long double ) {}

struct A { operator double () const { return 0; }};
struct B { operator int    () const { return 0; }};

int main () {
    foo ( 1.0f );  // this works fine
    foo ( 1.0  );  // this is fine, too
    foo ( 1.0L );  // this one, too
    foo ( 1 );     // this does not compile
foo ( A() );   // this compiles
foo ( B() );   // this does not compile
}

[llvmbot]

Ok, I missed one last point in [c.math]:

Moreover, there shall be additional overloads sufficient to ensure:

1. If any argument corresponding to a double parameter has type long double, then all arguments corresponding to double parameters are effectively cast to long double.
2. Otherwise, if any argument corresponding to a double parameter has type double or an integer type, then all arguments corresponding to double parameters are effectively cast to double.
3. Otherwise, all arguments corresponding to double parameters are effectively cast to float.

which is a rather cryptic way of saying that it should work for integers, and also (in functions with more than one parameter) for combinations of different floating-point types.

But, anyway, these additional standard-sanctioned overloads should allow it to work with integers, not prevent it from working with types which are convertible to arithmetic types.

[llvmbot]

I'm taking this bug since I already have DR19921. I'm also removing the blocked bug. DR18382 is caused by something else.

[mclow]

*** Bug llvm/llvm-bugzilla-archive#22707 has been marked as a duplicate of this bug. ***

[mclow]

The template should be enabled for every type that is convertible to an
arithmetic type, to make the implementation indistinguishable from what is
specified in the standard.

After some research, and talking to a bunch of people, I have come to the conclusion that this is incorrect.

This is LWG issue #​2086 - http://cplusplus.github.io/LWG/lwg-defects.html#2086, and the discussion there is quite clear:

  The only valid types that can be passed to isnan (and others in <cmath>) are arithmetic types; i.e, the built-in integral and floating point types. No user-defined types.

[zygoloid]

  The only valid types that can be passed to isnan (and others in

) are arithmetic types; i.e, the built-in integral and floating point
types. No user-defined types.

That may have been the intent, but I don't see any way to read the standard's wording that way. From the example in comment#0:

  std::isnan(A());

There are no arguments of arithmetic type, so none of the bullets in 26.8/11 apply. The overload set contains 'isnan(float)', 'isnan(double)', and 'isnan(long double)', and 'isnan(float)' should be selected.

To my reading, what paragraph 11 requires is overload sets like this:

template<typename T, typename U>
typename conditional<is_arithmetic::value, T, U>::type
cast_if_arithmetic(U u) { return u; }

template
struct is_double_or_integral :
integral_constant<bool, is_arithmetic::value &&
!is_same<T, float>::value &&
!is_same<T, long double>::value> {};

bool isgreater(float, float);
bool isgreater(double, double);
bool isgreater(long double, long double);

// bullet 1
template<typename T, typename U>
typename enable_if<is_same<T, long double>::value ||
is_same<U, long double>::value, bool>::type
isgreater(T t, U u) {
return isgreater(cast_if_arithmetic(t),
cast_if_arithmetic(u));
}
// bullet 2
template<typename T, typename U>
typename enable_if<is_double_or_integral::value ||
is_double_or_integral::value, bool>::type
isgreater(T t, U u) {
return isgreater(cast_if_arithmetic(t),
cast_if_arithmetic(u));
}
// bullet 3 requires no overloads: bullets 1 and 2 would have applied
// if there were an arithmetic argument of a type other than float

[mclow]

*** Bug llvm/llvm-bugzilla-archive#34566 has been marked as a duplicate of this bug. ***

[mclow]

Sorry, Richard - with your code, the following call:
isgreater(1.0, 1.l); // double and long double
is ambiguous.

Both "bullet #​1" and "bullet #​2" are matched.

[zygoloid]

Sorry, Richard - with your code, the following call:
isgreater(1.0, 1.l); // double and long double
is ambiguous.

Both "bullet #​1" and "bullet #​2" are matched.

Right you are, we need to exclude the bullet#1 cases from bullet#2.

[zygoloid]

#​2086 has now been resolved, and the wording now looks much more reasonable.

I added the missing (float), (double), (long double) overloads for isinf and isnan in r331241; this fix was necessary in order to fix another problem with isinf and isnan where they'd have the wrong return type in glibc. For the single-parameter functions, I think that's all we're missing.

For the multi-parameter functions, we still need something like the approach in comment#8. This seems to do the trick:

struct __nat {};
template<typename T, typename U> struct _MathOverload :
conditional<is_same<T, long double>::value || is_same<U, long double>::value,
decay,
typename conditional<(is_arithmetic::value && !is_same<T, float>::value) ||
(is_arithmetic::value && !is_same<U, float>::value),
decay, __nat>::type
>::type {};

template<typename T, typename U>
typename conditional<is_arithmetic::value, T, U>::type
cast_if_arithmetic(U u) { return u; }

bool isgreater(float, float);
bool isgreater(double, double);
bool isgreater(long double, long double);

template<typename T, typename U, typename V = typename _MathOverload<T, U>::type>
bool isgreater(T t, U u) {
return isgreater(cast_if_arithmetic(t), cast_if_arithmetic(u));
}

[llvmbot]

mentioned in issue #18759

[mclow]

mentioned in issue llvm/llvm-bugzilla-archive#22707

[mclow]

mentioned in issue llvm/llvm-bugzilla-archive#34566