[libcxx-commits] [PATCH] D135781: [libc++] Assume that builtins for math.h functions are available

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/math.h:294
 
 #include <__config>
 
----------------
Mingw is failing the `clang-tidy` test, and only that. Can you please figure out what's wrong and add the missing tests?


================
Comment at: libcxx/include/math.h:379
+template <class _Tp, std::__enable_if_t<std::is_integral<_Tp>::value && std::is_signed<_Tp>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI bool signbit(_Tp __v) _NOEXCEPT {
+  return __v < 0;
----------------
Those all need to be `inline` for ODR.


================
Comment at: libcxx/include/math.h:457
+_LIBCPP_HIDE_FROM_ABI bool isless(_Tp __lhs, _Up __rhs) _NOEXCEPT {
+  return __builtin_isless(__lhs, __rhs);
+}
----------------
You removed `__promote` here and in a bunch of other places. Can you please do that as a separate patch (including the ones that are trivial, i.e. `__promote<T>` where `T` is a floating point). That will make this change easier to validate.


================
Comment at: libcxx/include/math.h:850
+  using __result_type = typename std::__promote<_Tp, _Up>::type;
+  return ::copysign(static_cast<__result_type>(__x), static_cast<__result_type>(__y));
 }
----------------
If the C library does not provide `copysign(double, double)`, this is infinite recursion. This is too subtle and the failure mode is terrible, i.e. this should never be a runtime error. Instead, let's do this (per live review just now):

```
#include <type_traits>

// in C library
float copysignf(float, float);
double copysign(double, double);
long double copysignl(long double, long double);

// in libc++
inline auto copysign(float, float);
inline auto copysign(long double, long double);
template <class = int>
inline auto copysign(double x, double) { return x; }
template <class _Tp, class _Up,
          std::enable_if_t<std::is_arithmetic<_Tp>::value && std::is_arithmetic<_Up>::value, int> = 0>
inline auto copysign(_Tp __x, _Up __y);


int main() {
    double d = 0;
    copysign(d, d);
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135781