[libc++] Add `__libcpp_copysign` conditionally constexpr overloads.

[curdeius]

Summary

This is a spin-off from D79555 review, that with this patch will be able to use __libcpp_copysign instead of adhoc __copysign_constexpr helper.

[curdeius]

libcxx/include/math.h

1169–1190

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.

[• Quuxplusone]

libcxx/include/math.h

1169–1190

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.

[ldionne]

I'm not a big fan of the duplication, but unless we find a way to improve that, I think this LGTM (I mean.. I don't see a better way right now). Requesting changes until we've resolved the discussion about duplication.

Thanks for working on this, I think it'll make the constexpr-for-complex patch better.

libcxx/include/math.h

1169–1190

We don't want to make copysign constexpr, only the library-internal version __copysign.

Regarding an overload for double, it is handled by the _A1, _A2 overload which ends up calling ::copysign(double, double) from the C library. Or were you thinking about something else?

1171

_VSTD:: here and below (it doesn't make a difference here, but let's be consistent).

[• Quuxplusone]

libcxx/include/math.h

1169–1190

We don't want to make copysign constexpr, only the library-internal version __copysign.

Oh, I see. In that case, my suggestion would make the code ~2x as long as it was before; and so I have no suggestion better than this PR.

Regarding an overload for double, it is handled by the _A1, _A2 overload

My godbolt testing suggests that copysign(double, double) is not handled by the _A1, _A2 overload, but instead by using ::copysign _LIBCPP_USING_IF_EXISTS; elsewhere in this file... Ah, but __copysign(double, double) is handled by the template. Which is fine (only) because users don't call __copysign directly. But actually, does libc++ ever call __copysign(_A1, _A2) directly? Maybe the templated version should be provided only for users, i.e. template copysign because the Standard says we have to, but do not template __copysign.

[curdeius]

• Qualify calls.

[curdeius]

libcxx/include/math.h

1169–1190

We don't want to make copysign constexpr, only the library-internal version __copysign.

Actually, why don't we?
Can it break anything?

[curdeius]

Regarding naming, maybe I should rename __copysign to __libcpp_copysign to match what is done currently?

[curdeius]

• Rename to libcpp_copysign due to clashes with Linux ::copysign. Add double overload.

[curdeius]

libcxx/include/math.h

1143

FYI, this overload is necessary because, as Arthur correctly mentioned, copysign(double, double) is handled by using ::copysign _LIBCPP_USING_IF_EXISTS;, but there's no equivalent for __libcpp_copysign and using __libcpp_copysign(double, double) provokes an assertion failure in the template function because both types are the same.

[ldionne]

LGTM pending CI.

libcxx/include/math.h

1169–1190

1. If there ever was a reason why copysign couldn't be constexpr anymore, it would be API breaking for us to make that change. It's arguably not going to happen for something like copysign, though, but I think it's better to be consistent everywhere and not add constexpr as an extension.

2. It's pedantic, but the standard does not allow us to add constexpr as an extension.

[curdeius]

libcxx/include/math.h

1169–1190

Fair enough. Thank you for the answer.

libcxx/include/math.h

 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
 typename std::enable_if<std::is_integral<_A1>::value, double>::type
 cbrt(_A1 __lcpp_x) _NOEXCEPT {return ::cbrt((double)__lcpp_x);}
 
 // copysign
 
-inline _LIBCPP_INLINE_VISIBILITY float copysign(float __lcpp_x,
-                                                float __lcpp_y) _NOEXCEPT {
+#if __has_builtin(__builtin_copysignf)
+_LIBCPP_CONSTEXPR
+#endif
+inline _LIBCPP_INLINE_VISIBILITY float
+__copysign(float __lcpp_x, float __lcpp_y) _NOEXCEPT {
 #if __has_builtin(__builtin_copysignf)
   return __builtin_copysignf(__lcpp_x, __lcpp_y);
 #else
   return ::copysignf(__lcpp_x, __lcpp_y);
 #endif
 }
+
+#if __has_builtin(__builtin_copysignl)
+_LIBCPP_CONSTEXPR
+#endif
 inline _LIBCPP_INLINE_VISIBILITY long double
-copysign(long double __lcpp_x, long double __lcpp_y) _NOEXCEPT {
+__copysign(long double __lcpp_x, long double __lcpp_y) _NOEXCEPT {
 #if __has_builtin(__builtin_copysignl)
   return __builtin_copysignl(__lcpp_x, __lcpp_y);
 #else
   return ::copysignl(__lcpp_x, __lcpp_y);
 #endif
 }
 
 template <class _A1, class _A2>
+#if __has_builtin(__builtin_copysign)
+_LIBCPP_CONSTEXPR
+#endif
 inline _LIBCPP_INLINE_VISIBILITY
 typename std::_EnableIf
 <
     std::is_arithmetic<_A1>::value &&
     std::is_arithmetic<_A2>::value,
     std::__promote<_A1, _A2>
 >::type
-copysign(_A1 __lcpp_x, _A2 __lcpp_y) _NOEXCEPT
-{
+__copysign(_A1 __lcpp_x, _A2 __lcpp_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)), "");
 #if __has_builtin(__builtin_copysign)
     return __builtin_copysign((__result_type)__lcpp_x, (__result_type)__lcpp_y);
 #else
     return ::copysign((__result_type)__lcpp_x, (__result_type)__lcpp_y);
 #endif
 }
 
+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 {
+  return __copysign(__lcpp_x, __lcpp_y);
+}
+
+template <class _A1, class _A2>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::_EnableIf
+<
+    std::is_arithmetic<_A1>::value &&
+    std::is_arithmetic<_A2>::value,
+    std::__promote<_A1, _A2>
+>::type
+copysign(_A1 __lcpp_x, _A2 __lcpp_y) _NOEXCEPT {
+    return __copysign(__lcpp_x, __lcpp_y);
+}
+
 // erf
 
 inline _LIBCPP_INLINE_VISIBILITY float       erf(float __lcpp_x) _NOEXCEPT       {return ::erff(__lcpp_x);}
 inline _LIBCPP_INLINE_VISIBILITY long double erf(long double __lcpp_x) _NOEXCEPT {return ::erfl(__lcpp_x);}
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
 typename std::enable_if<std::is_integral<_A1>::value, double>::type