[libcxx-commits] [PATCH] D106364: [libc++] Add __copysign conditionally constexpr overloads.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/math.h:1169-1190
+inline _LIBCPP_INLINE_VISIBILITY float
+copysign(float __lcpp_x, float __lcpp_y) _NOEXCEPT {
+  return __copysign(__lcpp_x, __lcpp_y);
+}
+
+inline _LIBCPP_INLINE_VISIBILITY long double
+copysign(long double __lcpp_x, long double __lcpp_y) _NOEXCEPT {
----------------
curdeius wrote:
> If anyone has an idea about how to replace these 3 overloads with something simpler, without breaking things, I'm all ears.
> Just writing:
> ```
> template <class _A1, class _A2>
> auto copysign(_A1 __lcpp_x, _A2 __lcpp_y) _NOEXCEPT {
>     return __copysign(__lcpp_x, __lcpp_y);
> }
> ```
> doesn't work before C++14 because of `auto`.
> Using `std::__promote` instead results in ambiguities.
> Also, type deduction might fail if passed e.g. a float and a type implicitly convertible to float (works now, deduction of _A2 would fail without using tricks like `_A2 = identity<_A1>`, but even then I'm not sure of consequences.
At least it needs more comments about the design constraints ;) because I don't see why there are overloads only for `float` and `long double`, but not `double`. What's wrong with simply...?
```
#if __has_builtin(__builtin_copysignf)
inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR float
copysign(float __x, float __y) _NOEXCEPT
    { return __builtin_copysignf(__x, __y); }
#else
inline _LIBCPP_INLINE_VISIBILITY float
copysign(float __x, float __y) _NOEXCEPT
    { return ::copysignf(__x, __y); }
#endif

#if __has_builtin(__builtin_copysign)
inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR double
copysign(double __x, double __y) _NOEXCEPT
    { return __builtin_copysign(__x, __y); }
#else
inline _LIBCPP_INLINE_VISIBILITY double
copysign(double __x, double __y) _NOEXCEPT
    { return ::copysign(__x, __y); }
#endif

#if __has_builtin(__builtin_copysignl)
inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR long double
copysign(long double __x, long double __y) _NOEXCEPT
    { return __builtin_copysignl(__x, __y); }
#else
inline _LIBCPP_INLINE_VISIBILITY long double
copysign(long double __x, long double __y) _NOEXCEPT
    { return ::copysignl(__x, __y); }
#endif

inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
typename _EnableIf<
    is_arithmetic<_A1>::value && is_arithmetic<_A2>::value,
    __promote<_A1, _A2>
>::type
copysign(_A1 __x, _A2 __y) _NOEXCEPT
{
    typedef typename std::__promote<_A1, _A2>::type __result_type;
    static_assert((!(std::_IsSame<_A1, __result_type>::value &&
                     std::_IsSame<_A2, __result_type>::value)), "");
    return _VSTD::copysign(__result_type(__x), __result_type(__y));
}
```
It's about 6 lines longer than what's there now, but I think it's massively simpler.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106364/new/

https://reviews.llvm.org/D106364