[libc++] Move std::abs into __math/abs.h

[philnik777]

template <class = int> is also added to our implementations to avoid an ambiguity between the libc's version and our version when both are visible.

This avoids including <stdlib.h> in <math.h>.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- libcxx/include/__math/abs.h libcxx/include/math.h libcxx/include/stdlib.h libcxx/test/std/numerics/c.math/abs.verify.cpp

View the diff from clang-format here.

diff --git a/libcxx/test/std/numerics/c.math/abs.verify.cpp b/libcxx/test/std/numerics/c.math/abs.verify.cpp
index cc30112f0..9e5bfda52 100644
--- a/libcxx/test/std/numerics/c.math/abs.verify.cpp
+++ b/libcxx/test/std/numerics/c.math/abs.verify.cpp
@@ -13,10 +13,12 @@ void f() {
     (void)std::abs(ui); // expected-error {{call to 'abs' is ambiguous}}
 
     unsigned char uc = -5;
-    (void)std::abs(uc); // expected-warning 0-1 {{taking the absolute value of unsigned type 'unsigned char' has no effect}}
+    (void)std::abs(
+        uc); // expected-warning 0-1 {{taking the absolute value of unsigned type 'unsigned char' has no effect}}
 
     unsigned short us = -5;
-    (void)std::abs(us); // expected-warning 0-1 {{taking the absolute value of unsigned type 'unsigned short' has no effect}}
+    (void)std::abs(
+        us); // expected-warning 0-1 {{taking the absolute value of unsigned type 'unsigned short' has no effect}}
 
     unsigned long ul = -5;
     (void)std::abs(ul); // expected-error {{call to 'abs' is ambiguous}}

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This avoids including <stdlib.h> in <math.h>.

---

Full diff: https://github.com/llvm/llvm-project/pull/139586.diff

4 Files Affected:

• (modified) libcxx/include/__math/abs.h (+24)
• (modified) libcxx/include/math.h (+1-10)
• (modified) libcxx/include/stdlib.h (+2-17)
• (modified) libcxx/test/std/numerics/c.math/abs.verify.cpp (+2-2)

diff --git a/libcxx/include/__math/abs.h b/libcxx/include/__math/abs.h
index fc3bf3a2c7c32..b780159f11ebf 100644
--- a/libcxx/include/__math/abs.h
+++ b/libcxx/include/__math/abs.h
@@ -39,6 +39,30 @@ template <class _A1, __enable_if_t<is_integral<_A1>::value, int> = 0>
   return __builtin_fabs((double)__x);
 }
 
+// abs
+
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI inline float abs(float __x) _NOEXCEPT { return __builtin_fabsf(__x); }
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI inline double abs(double __x) _NOEXCEPT { return __builtin_fabs(__x); }
+
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI inline long double abs(long double __x) _NOEXCEPT {
+  return __builtin_fabsl(__x);
+}
+
+template <class = int>
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI inline int abs(int __x) _NOEXCEPT {
+  return __builtin_abs(__x);
+}
+
+template <class = int>
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI inline long abs(long __x) _NOEXCEPT {
+  return __builtin_labs(__x);
+}
+
+template <class = int>
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI inline long long abs(long long __x) _NOEXCEPT {
+  return __builtin_llabs(__x);
+}
+
 } // namespace __math
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/math.h b/libcxx/include/math.h
index de2dacde282c1..929bef6385043 100644
--- a/libcxx/include/math.h
+++ b/libcxx/include/math.h
@@ -378,9 +378,7 @@ extern "C++" {
 #      include <__math/traits.h>
 #      include <__math/trigonometric_functions.h>
 #      include <__type_traits/enable_if.h>
-#      include <__type_traits/is_floating_point.h>
 #      include <__type_traits/is_integral.h>
-#      include <stdlib.h>
 
 // fpclassify relies on implementation-defined constants, so we can't move it to a detail header
 _LIBCPP_BEGIN_NAMESPACE_STD
@@ -431,19 +429,12 @@ using std::__math::isnormal;
 using std::__math::isunordered;
 #      endif // _LIBCPP_MSVCRT
 
-// abs
-//
-// handled in stdlib.h
-
-// div
-//
-// handled in stdlib.h
-
 // We have to provide double overloads for <math.h> to work on platforms that don't provide the full set of math
 // functions. To make the overload set work with multiple functions that take the same arguments, we make our overloads
 // templates. Functions are preferred over function templates during overload resolution, which means that our overload
 // will only be selected when the C library doesn't provide one.
 
+using std::__math::abs;
 using std::__math::acos;
 using std::__math::acosh;
 using std::__math::asin;
diff --git a/libcxx/include/stdlib.h b/libcxx/include/stdlib.h
index 39550f36bb6ed..8dfdfa416f088 100644
--- a/libcxx/include/stdlib.h
+++ b/libcxx/include/stdlib.h
@@ -106,23 +106,8 @@ extern "C++" {
 #        undef llabs
 #      endif
 
-// MSVCRT already has the correct prototype in <stdlib.h> if __cplusplus is defined
-#      if !defined(_LIBCPP_MSVCRT)
-[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI long abs(long __x) _NOEXCEPT { return __builtin_labs(__x); }
-[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI long long abs(long long __x) _NOEXCEPT { return __builtin_llabs(__x); }
-#      endif // !defined(_LIBCPP_MSVCRT)
-
-[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI float abs(float __lcpp_x) _NOEXCEPT {
-  return __builtin_fabsf(__lcpp_x); // Use builtins to prevent needing math.h
-}
-
-[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI double abs(double __lcpp_x) _NOEXCEPT {
-  return __builtin_fabs(__lcpp_x);
-}
-
-[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI long double abs(long double __lcpp_x) _NOEXCEPT {
-  return __builtin_fabsl(__lcpp_x);
-}
+#      include <__math/abs.h>
+using std::__math::abs;
 
 // div
 
diff --git a/libcxx/test/std/numerics/c.math/abs.verify.cpp b/libcxx/test/std/numerics/c.math/abs.verify.cpp
index dec80762b214f..cc30112f0c312 100644
--- a/libcxx/test/std/numerics/c.math/abs.verify.cpp
+++ b/libcxx/test/std/numerics/c.math/abs.verify.cpp
@@ -13,10 +13,10 @@ void f() {
     (void)std::abs(ui); // expected-error {{call to 'abs' is ambiguous}}
 
     unsigned char uc = -5;
-    (void)std::abs(uc); // expected-warning {{taking the absolute value of unsigned type 'unsigned char' has no effect}}
+    (void)std::abs(uc); // expected-warning 0-1 {{taking the absolute value of unsigned type 'unsigned char' has no effect}}
 
     unsigned short us = -5;
-    (void)std::abs(us); // expected-warning {{taking the absolute value of unsigned type 'unsigned short' has no effect}}
+    (void)std::abs(us); // expected-warning 0-1 {{taking the absolute value of unsigned type 'unsigned short' has no effect}}
 
     unsigned long ul = -5;
     (void)std::abs(ul); // expected-error {{call to 'abs' is ambiguous}}

[ldionne]

Thanks for the cleanup. I think this makes sense but I have a few questions. Also, this could break users since we're removing a transitive include of <stdlib.h> from <math.h>. That's not a reason not to do it, but pinging @llvm/libcxx-vendors for awareness.

[DKLoehr]

As a heads up, after this PR we found that some of the template/overload resolution for abs was different. In tensorflow, previously we had std::abs<float>. After this change, we had to change it to [](float f) { return std::abs(f); }, or use a static_cast to the right function type.

For more context, see https://crbug.com/427716648. Totally fixable, but it took a little while to figure out the fix.