Skip to content

[libcxx] Do not include langinfo.h when using the LLVM C library #106634

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 31, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Aug 29, 2024

Summary:
The langinfo.h header is a POSIX extension, so ideally we would be
able to build the C++ library without it. Currently the LLVM C library
doesn't support / provide it. This allows us to build the C++ library
with locales enabled. We can either disable it here, or just provide
stubs that do nothing as in #106620.

@jhuber6 jhuber6 requested a review from a team as a code owner August 29, 2024 21:24
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 29, 2024

@llvm/pr-subscribers-libcxx

Author: Joseph Huber (jhuber6)

Changes

Summary:
The langinfo.h header is a POSIX extension, so ideally we would be
able to build the C++ library without it. Currently the LLVM C library
doesn't support / provide it. This allows us to build the C++ library
with locales enabled. We can either disable it here, or just provide
stubs that do nothing as in #106620.


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

1 Files Affected:

  • (modified) libcxx/src/locale.cpp (+2-1)
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index 4efdc63c096611..06c4f414ed9fa9 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -34,7 +34,8 @@
 #  define _CTYPE_DISABLE_MACROS
 #endif
 
-#if !defined(_LIBCPP_MSVCRT) && !defined(__MINGW32__) && !defined(__BIONIC__) && !defined(__NuttX__)
+#if !defined(_LIBCPP_MSVCRT) && !defined(__MINGW32__) && !defined(__BIONIC__) && !defined(__NuttX__) ||                \
+    defined(__LLVM_LIBC__)
 #  include <langinfo.h>
 #endif
 

@jhuber6 jhuber6 changed the title [libc] Do not include langinfo.h when using the LLVM C library [libcxx] Do not include langinfo.h when using the LLVM C library Aug 29, 2024
Comment on lines 37 to 39
#if !defined(_LIBCPP_MSVCRT) && !defined(__MINGW32__) && !defined(__BIONIC__) && !defined(__NuttX__) || \
defined(__LLVM_LIBC__)
# include <langinfo.h>
#endif
Copy link
Member

Choose a reason for hiding this comment

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

#if __has_include?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following what the other targets do, but I could replace it with a has include if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, guess we'll see if this passes CI.

Summary:
The `langinfo.h` header is a POSIX extension, so ideally we would be
able to build the C++ library without it. Currently the LLVM C library
doesn't support / provide it. This allows us to build the C++ library
with locales enabled. We can either disable it here, or just provide
stubs that do nothing as in llvm#106620.
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM pending CI.

@jhuber6 jhuber6 merged commit 109bff1 into llvm:main Aug 31, 2024
60 checks passed
@jhuber6 jhuber6 deleted the lang branch August 31, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants