Skip to content

[libc] Remove direct math.h includes from src #84991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

michaelrj-google
Copy link
Contributor

A downstream overlay mode user ran into issues with the isnan macro not
working in our sources with a specific libc configuration. This patch
replaces the last direct includes of math.h with our internal
math_macros.h, along with the necessary build system changes.

A downstream overlay mode user ran into issues with the isnan macro not
working in our sources with a specific libc configuration. This patch
replaces the last direct includes of math.h with our internal
math_macros.h, along with the necessary build system changes.
@llvmbot llvmbot added the libc label Mar 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2024

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

A downstream overlay mode user ran into issues with the isnan macro not
working in our sources with a specific libc configuration. This patch
replaces the last direct includes of math.h with our internal
math_macros.h, along with the necessary build system changes.


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

8 Files Affected:

  • (modified) libc/src/__support/FPUtil/FEnvImpl.h (+1-1)
  • (modified) libc/src/__support/FPUtil/ManipulationFunctions.h (+1-1)
  • (modified) libc/src/__support/FPUtil/NearestIntegerOperations.h (+1-1)
  • (modified) libc/src/math/generic/math_utils.h (+1-1)
  • (modified) libc/test/src/math/ILogbTest.h (+1-1)
  • (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+15-1)
  • (modified) utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel (+1)
  • (modified) utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel (+1)
diff --git a/libc/src/__support/FPUtil/FEnvImpl.h b/libc/src/__support/FPUtil/FEnvImpl.h
index 6810659733de2c..4a84d1dc086c07 100644
--- a/libc/src/__support/FPUtil/FEnvImpl.h
+++ b/libc/src/__support/FPUtil/FEnvImpl.h
@@ -14,8 +14,8 @@
 #include "src/__support/macros/properties/architectures.h"
 #include "src/errno/libc_errno.h"
 
+#include "include/llvm-libc-macros/math-macros.h"
 #include <fenv.h>
-#include <math.h>
 
 #if defined(LIBC_TARGET_ARCH_IS_AARCH64)
 #if defined(__APPLE__)
diff --git a/libc/src/__support/FPUtil/ManipulationFunctions.h b/libc/src/__support/FPUtil/ManipulationFunctions.h
index f148d984f5f35c..1aa1efcb4eb7dd 100644
--- a/libc/src/__support/FPUtil/ManipulationFunctions.h
+++ b/libc/src/__support/FPUtil/ManipulationFunctions.h
@@ -22,7 +22,7 @@
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
 
-#include <math.h>
+#include "include/llvm-libc-macros/math-macros.h"
 
 namespace LIBC_NAMESPACE {
 namespace fputil {
diff --git a/libc/src/__support/FPUtil/NearestIntegerOperations.h b/libc/src/__support/FPUtil/NearestIntegerOperations.h
index 19ae75ea788912..da3d80ceb097e5 100644
--- a/libc/src/__support/FPUtil/NearestIntegerOperations.h
+++ b/libc/src/__support/FPUtil/NearestIntegerOperations.h
@@ -16,7 +16,7 @@
 #include "src/__support/CPP/type_traits.h"
 #include "src/__support/common.h"
 
-#include <math.h>
+#include "include/llvm-libc-macros/math-macros.h"
 
 namespace LIBC_NAMESPACE {
 namespace fputil {
diff --git a/libc/src/math/generic/math_utils.h b/libc/src/math/generic/math_utils.h
index e884fe2deae284..396a70d7713b2c 100644
--- a/libc/src/math/generic/math_utils.h
+++ b/libc/src/math/generic/math_utils.h
@@ -14,7 +14,7 @@
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
 
-#include <math.h>
+#include "include/llvm-libc-macros/math-macros.h"
 
 #include <stdint.h>
 
diff --git a/libc/test/src/math/ILogbTest.h b/libc/test/src/math/ILogbTest.h
index 3e2db33e2c0524..dcc9d554eb3c28 100644
--- a/libc/test/src/math/ILogbTest.h
+++ b/libc/test/src/math/ILogbTest.h
@@ -9,11 +9,11 @@
 #ifndef LLVM_LIBC_TEST_SRC_MATH_ILOGBTEST_H
 #define LLVM_LIBC_TEST_SRC_MATH_ILOGBTEST_H
 
+#include "include/llvm-libc-macros/math-macros.h"
 #include "src/__support/CPP/limits.h" // INT_MAX
 #include "src/__support/FPUtil/FPBits.h"
 #include "src/__support/FPUtil/ManipulationFunctions.h"
 #include "test/UnitTest/Test.h"
-#include <math.h>
 
 class LlvmLibcILogbTest : public LIBC_NAMESPACE::testing::Test {
 public:
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 5f6c43cd6af7c1..8fec108df3124a 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -62,7 +62,17 @@ config_setting(
     flag_values = {":mpfr": "system"},
 )
 
-############################## Support libraries #############################
+################################# Include Files ################################
+
+libc_support_library(
+    name = "internal_includes",
+    hdrs = glob([
+        "include/llvm-libc-macros/*.h",
+        "include/llvm-libc/types/*",
+    ]),
+)
+
+############################### Support libraries ##############################
 
 libc_support_library(
     name = "__support_macros_properties_architectures",
@@ -669,6 +679,7 @@ libc_support_library(
         ":__support_macros_properties_architectures",
         ":__support_macros_sanitizer",
         ":errno",
+        ":internal_includes",
     ],
 )
 
@@ -738,6 +749,7 @@ libc_support_library(
         ":__support_fputil_normal_float",
         ":__support_macros_optimization",
         ":__support_uint128",
+        ":internal_includes",
     ],
 )
 
@@ -751,6 +763,7 @@ libc_support_library(
         ":__support_fputil_fp_bits",
         ":__support_fputil_rounding_mode",
         ":__support_macros_attributes",
+        ":internal_includes",
     ],
 )
 
@@ -1172,6 +1185,7 @@ libc_support_library(
         "__support_cpp_type_traits",
         ":__support_common",
         ":errno",
+        ":internal_includes",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
index 6837b9880d5a41..7c8ad71853e5e4 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
@@ -64,6 +64,7 @@ libc_test(
     name = "integer_to_string_test",
     srcs = ["integer_to_string_test.cpp"],
     deps = [
+        "//libc:__support_cpp_limits",
         "//libc:__support_cpp_span",
         "//libc:__support_cpp_string_view",
         "//libc:__support_integer_literals",
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
index 63e18b83710918..391b509854f998 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
@@ -296,6 +296,7 @@ libc_support_library(
         "//libc:__support_cpp_limits",
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_manipulation_functions",
+        "//libc:internal_includes",
         "//libc/test/UnitTest:LibcUnitTest",
     ],
 )

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind removing the newlines separating this include from the previous blocks of includes, then sorting the result?

For example:

  • libc/src/__support/FPUtil/FEnvImpl.h
  • libc/src/__support/FPUtil/ManipulationFunctions.h
  • libc/src/__support/FPUtil/NearestIntegerOperations.h
  • libc/src/math/generic/math_utils.h

now all have unnecessary newlines before the inclusion of math-macros.h.

@michaelrj-google michaelrj-google merged commit caba6d1 into llvm:main Mar 14, 2024
@michaelrj-google michaelrj-google deleted the libcMathMacroIncludes branch March 14, 2024 21:15
michaelrj-google added a commit that referenced this pull request Mar 14, 2024
michaelrj-google added a commit that referenced this pull request Mar 14, 2024
@michaelrj-google michaelrj-google restored the libcMathMacroIncludes branch March 14, 2024 22:11
michaelrj-google added a commit that referenced this pull request Mar 18, 2024
Reland of #84991

A downstream overlay mode user ran into issues with the isnan macro not
working in our sources with a specific libc configuration. This patch
replaces the last direct includes of math.h with our internal
math_macros.h, along with the necessary build system changes.
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
Reland of llvm#84991

A downstream overlay mode user ran into issues with the isnan macro not
working in our sources with a specific libc configuration. This patch
replaces the last direct includes of math.h with our internal
math_macros.h, along with the necessary build system changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants