Skip to content

[libc] Use proxy headers for limits.h values #102378

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 1 commit into from
Aug 7, 2024
Merged

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Aug 7, 2024

Summary:
This defines some constants that may conflict with the system when in
overlay mode. Use the proxy header instead.

Fixes: #102368

@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2024

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
This defines some constants that may conflict with the system when in
overlay mode. Use the proxy header instead.

Fixes: #102368


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

1 Files Affected:

  • (modified) libc/src/__support/CPP/limits.h (+2-2)
diff --git a/libc/src/__support/CPP/limits.h b/libc/src/__support/CPP/limits.h
index 686abcaea9880..34b7b7df262f4 100644
--- a/libc/src/__support/CPP/limits.h
+++ b/libc/src/__support/CPP/limits.h
@@ -9,10 +9,10 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_CPP_LIMITS_H
 #define LLVM_LIBC_SRC___SUPPORT_CPP_LIMITS_H
 
-#include "include/llvm-libc-macros/limits-macros.h" // CHAR_BIT
+#include "hdr/limits-macros.h" // CHAR_BIT
 #include "src/__support/CPP/type_traits/is_integral.h"
 #include "src/__support/CPP/type_traits/is_signed.h"
-#include "src/__support/macros/attributes.h"       // LIBC_INLINE
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
 #include "src/__support/macros/config.h"
 #include "src/__support/macros/properties/types.h" // LIBC_TYPES_HAS_INT128
 

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

Overall LGTM but also remember to update the cmake and the bazel. Here's the diff that fixes the bazel:

diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 253b89216a88..e52bf2782638 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -150,6 +150,12 @@ libc_support_library(
     hdrs = ["hdr/stdio_macros.h"],
 )
 
+
+libc_support_library(
+    name = "hdr_limits_macros",
+    hdrs = ["hdr/limits_macros.h"],
+)
+
 ############################ Type Proxy Header Files ###########################
 
 libc_support_library(
@@ -362,7 +368,7 @@ libc_support_library(
         "__support_cpp_type_traits",
         "__support_macros_attributes",
         ":__support_macros_properties_types",
-        ":llvm_libc_macros_limits_macros",
+        ":hdr_limits_macros",
     ],
 )

@@ -9,10 +9,10 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_CPP_LIMITS_H
#define LLVM_LIBC_SRC___SUPPORT_CPP_LIMITS_H

#include "include/llvm-libc-macros/limits-macros.h" // CHAR_BIT
#include "hdr/limits-macros.h" // CHAR_BIT
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include "hdr/limits-macros.h" // CHAR_BIT
#include "hdr/limits_macros.h" // CHAR_BIT

Summary:
This defines some constants that may conflict with the system when in
overlay mode. Use the proxy header instead.

Fixes: llvm#102368
@jhuber6 jhuber6 requested review from rupprecht and keith as code owners August 7, 2024 21:58
@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Aug 7, 2024
@jhuber6 jhuber6 merged commit cf416e0 into llvm:main Aug 7, 2024
5 of 6 checks passed
@jhuber6 jhuber6 deleted the proxy branch September 23, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel libc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libc fails to build on error "Assumed value of MB_LEN_MAX wrong"
3 participants