Revert "Teach __libcpp_is_floating_point that __fp16 and _Float16 are"

[phosek]

Summary

This reverts commits r333103 and r333108. _Float16 and __fp16 are C11
extensions and compilers other than Clang don't define these for C++.

See D53486 for related discussion.

[EricWF]

Sorry I'm late to the party, but we should just be falling back to __is_floating_point if we have it. Since it will always give us the answer we want. (Including for __float128 which we don't handle ATM).

[ldionne]

In D53670#1275107, @EricWF wrote:

Sorry I'm late to the party, but we should just be falling back to __is_floating_point if we have it. Since it will always give us the answer we want. (Including for __float128 which we don't handle ATM).

That doesn't change the fact that it would be lying about what it means to be a floating point type, wouldn't it? For example, numeric_limits would still not be defined for __fp16, right?

As a side question, do you know how numeric_limits are implemented for __float128?

[EricWF]

In D53670#1275144, @ldionne wrote:

In D53670#1275107, @EricWF wrote:

Sorry I'm late to the party, but we should just be falling back to __is_floating_point if we have it. Since it will always give us the answer we want. (Including for __float128 which we don't handle ATM).

That doesn't change the fact that it would be lying about what it means to be a floating point type, wouldn't it? For example, numeric_limits would still not be defined for __fp16, right?

If it acts like a floating point type, and the compiler says it's a floating point type, then it's a floating point type; regardless of what the C++ standard says.
We shouldn't pretend these types don't exist.

If the compiler provides the types, it means someone went through a ton of trouble in order to provide these additional types to users. So the question we should be asking is "What's in the best interest of our users?",

As a side question, do you know how numeric_limits are implemented for __float128?

[rsmith]

What's the problem with the state of affairs prior to this change? If some compiler / environment is defining a __FLT16_MANT_DIG__ macro but not providing a _Float16 type, isn't that just a bug in that environment?

[ldionne]

In D53670#1275188, @rsmith wrote:

What's the problem with the state of affairs prior to this change? If some compiler / environment is defining a __FLT16_MANT_DIG__ macro but not providing a _Float16 type, isn't that just a bug in that environment?

There's also the fact that e.g. __fp16 doesn't work with the stream's operator>> and similar problems. Because of this, we're effectively lying about the fact that __fp16 is a floating point type. This was Marshall's point, and I agree with it.

@EricWF I do agree that if it quacks like a floating point and it walks like a floating point, then it is a floating point. However, __fp16 doesn't seem to quack quite right, so it's not a floating point (at least not without additional work).

[mclow.lists]

In D53670#1275107, @EricWF wrote:

Sorry I'm late to the party, but we should just be falling back to __is_floating_point if we have it. Since it will always give us the answer we want. (Including for __float128 which we don't handle ATM).

@EricWF and I discussed this privately, and I disagree.
There's lots of things in the standard library that key off of std::is_floating_point, and we don't support *any* of them for __fp16 or _Float16 or __float128.

[rsmith]

In D53670#1275677, @ldionne wrote:

In D53670#1275188, @rsmith wrote:

What's the problem with the state of affairs prior to this change? If some compiler / environment is defining a __FLT16_MANT_DIG__ macro but not providing a _Float16 type, isn't that just a bug in that environment?

There's also the fact that e.g. __fp16 doesn't work with the stream's operator>> and similar problems. Because of this, we're effectively lying about the fact that __fp16 is a floating point type. This was Marshall's point, and I agree with it.

@EricWF I do agree that if it quacks like a floating point and it walks like a floating point, then it is a floating point. However, __fp16 doesn't seem to quack quite right, so it's not a floating point (at least not without additional work).

I do mostly agree with this for __fp16 (which is a weird storage-only type with some arbitrary limitations). But not for _Float16 nor __float128, which are both proper floating-point types.

For whatever it's worth, in libstdc++ std::is_floating_point<__float128>::value yields true by default (which in this case means in -std=gnu++XY but not in -std=c++XY) on targets that have a __float128 type. They also supply an abs overload for __float128 in that case, but nothing else (surprisingly not even numeric_limits).

[mclow.lists]

In D53670#1276224, @rsmith wrote:

For whatever it's worth, in libstdc++ std::is_floating_point<__float128>::value yields true by default (which in this case means in -std=gnu++XY but not in -std=c++XY) on targets that have a __float128 type. They also supply an abs overload for __float128 in that case, but nothing else (surprisingly not even numeric_limits).

Yes, but clang doesn't have a "not a standard" mode like gcc does. (nor do I think we should have one).

[ldionne]

In my opinion, the proper way forward is to standardize a notion of "extended floating point types" and establish a set of things that those must support. Then, when vendors want to provide such types, they can make sure that we (the library implementation) can implement the required functionality for their custom types to be proper floating point types. This would mean abs , isnan, numeric_limits, etc...

We don't have such a notion right now. If we did, we could arguably make _Float16 and __float128 such extended floating point types, but in all cases __fp16 would be dead in the water. I hear that someone is going to propose such floating point types -- I would be inclined to wait and see how this pans out before we spend more cycles trying to figure the issue out ourselves.

libcxx/include/type_traits

 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_integral_v
     = is_integral<_Tp>::value;
 #endif
 
 // is_floating_point
 
 template <class _Tp> struct __libcpp_is_floating_point              : public false_type {};
-#ifdef __clang__
-template <>          struct __libcpp_is_floating_point<__fp16>      : public true_type {};
-#endif
-#ifdef __FLT16_MANT_DIG__
-template <>          struct __libcpp_is_floating_point<_Float16>    : public true_type {};
-#endif
 template <>          struct __libcpp_is_floating_point<float>       : public true_type {};
 template <>          struct __libcpp_is_floating_point<double>      : public true_type {};
 template <>          struct __libcpp_is_floating_point<long double> : public true_type {};
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_floating_point
     : public __libcpp_is_floating_point<typename remove_cv<_Tp>::type> {};
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)

libcxx/test/libcxx/type_traits/is_floating_point.pass.cpp

-//===----------------------------------------------------------------------===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// <type_traits>
-//
-// Test that is_floating_point<T>::value is true when T=__fp16 or T=_Float16.
-
-#include <type_traits>
-
-int main() {
-#ifdef __clang__
-  static_assert(std::is_floating_point<__fp16>::value, "");
-#endif
-#ifdef __FLT16_MANT_DIG__
-  static_assert(std::is_floating_point<_Float16>::value, "");
-#endif
-  return 0;
-}