-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[libc++] Support using Newlib as libc with locale support. #167962
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
Conversation
Tested on Newlib v4.5.0. The newlib.h was mostly copied from linux.h within the same directory. Newlib locale support was initially added in D5385 [1], but removed in revision cfde4fb [2], because of revision 78b6374 [3]. [1]: https://reviews.llvm.org/D5385 [2]: llvm#113721 [3]: https://reviews.llvm.org/D49927
|
@llvm/pr-subscribers-libcxx Author: Chenguang Wang (wecing) ChangesTested on Newlib v4.5.0. The newlib.h was mostly copied from linux.h within the same directory. Newlib locale support was initially added in D5385 1, but removed in revision cfde4fb 2, because of revision 78b6374 3. Full diff: https://github.com/llvm/llvm-project/pull/167962.diff 6 Files Affected:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index a119850cd808e..09026cbe1e4b8 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -285,6 +285,7 @@ endif()
# Feature options -------------------------------------------------------------
option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF)
+option(LIBCXX_HAS_NEWLIB_LIBC "Build libc++ with support for the Newlib C library" OFF)
option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
option(LIBCXX_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
option(LIBCXX_HAS_EXTERNAL_THREAD_API
@@ -741,6 +742,7 @@ config_define(${LIBCXX_HAS_PTHREAD_API} _LIBCPP_HAS_THREAD_API_PTHREAD)
config_define(${LIBCXX_HAS_EXTERNAL_THREAD_API} _LIBCPP_HAS_THREAD_API_EXTERNAL)
config_define(${LIBCXX_HAS_WIN32_THREAD_API} _LIBCPP_HAS_THREAD_API_WIN32)
config_define(${LIBCXX_HAS_MUSL_LIBC} _LIBCPP_HAS_MUSL_LIBC)
+config_define(${LIBCXX_HAS_NEWLIB_LIBC} _LIBCPP_HAS_NEWLIB_LIBC)
config_define_if(LIBCXX_NO_VCRUNTIME _LIBCPP_NO_VCRUNTIME)
config_define(${LIBCXX_ENABLE_FILESYSTEM} _LIBCPP_HAS_FILESYSTEM)
config_define(${LIBCXX_ENABLE_RANDOM_DEVICE} _LIBCPP_HAS_RANDOM_DEVICE)
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 131ba99357d62..4870f050709de 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -533,6 +533,7 @@ set(files
__locale_dir/support/fuchsia.h
__locale_dir/support/linux.h
__locale_dir/support/netbsd.h
+ __locale_dir/support/newlib.h
__locale_dir/support/no_locale/characters.h
__locale_dir/support/no_locale/strtonum.h
__locale_dir/support/windows.h
diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index b68c0c8258366..0f8e83d87c9c5 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -17,6 +17,7 @@
#cmakedefine01 _LIBCPP_HAS_MONOTONIC_CLOCK
#cmakedefine01 _LIBCPP_HAS_TERMINAL
#cmakedefine01 _LIBCPP_HAS_MUSL_LIBC
+#cmakedefine01 _LIBCPP_HAS_NEWLIB_LIBC
#cmakedefine01 _LIBCPP_HAS_THREAD_API_PTHREAD
#cmakedefine01 _LIBCPP_HAS_THREAD_API_EXTERNAL
#cmakedefine01 _LIBCPP_HAS_THREAD_API_WIN32
diff --git a/libcxx/include/__locale_dir/locale_base_api.h b/libcxx/include/__locale_dir/locale_base_api.h
index 8c8f00061d1ed..f83af881db215 100644
--- a/libcxx/include/__locale_dir/locale_base_api.h
+++ b/libcxx/include/__locale_dir/locale_base_api.h
@@ -123,6 +123,8 @@
# include <__locale_dir/support/fuchsia.h>
# elif defined(__linux__)
# include <__locale_dir/support/linux.h>
+# elif _LIBCPP_HAS_NEWLIB_LIBC
+# include <__locale_dir/support/newlib.h>
# else
// TODO: This is a temporary definition to bridge between the old way we defined the locale base API
diff --git a/libcxx/include/__locale_dir/support/newlib.h b/libcxx/include/__locale_dir/support/newlib.h
new file mode 100644
index 0000000000000..f24ff458b8bd3
--- /dev/null
+++ b/libcxx/include/__locale_dir/support/newlib.h
@@ -0,0 +1,269 @@
+//===-----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___LOCALE_DIR_SUPPORT_NEWLIB_H
+#define _LIBCPP___LOCALE_DIR_SUPPORT_NEWLIB_H
+
+#include <__config>
+#include <clocale>
+#include <cstdio>
+#include <cstdlib>
+#include <ctype.h>
+#include <stdarg.h>
+#include <string.h>
+#include <time.h>
+#if _LIBCPP_HAS_WIDE_CHARACTERS
+# include <cwchar>
+# include <wctype.h>
+#endif
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+namespace __locale {
+
+struct __locale_guard {
+ _LIBCPP_HIDE_FROM_ABI __locale_guard(locale_t& __loc) : __old_loc_(::uselocale(__loc)) {}
+
+ _LIBCPP_HIDE_FROM_ABI ~__locale_guard() {
+ if (__old_loc_)
+ ::uselocale(__old_loc_);
+ }
+
+ locale_t __old_loc_;
+
+ __locale_guard(__locale_guard const&) = delete;
+ __locale_guard& operator=(__locale_guard const&) = delete;
+};
+
+//
+// Locale management
+//
+
+using __locale_t _LIBCPP_NODEBUG = locale_t;
+using __lconv_t _LIBCPP_NODEBUG = lconv;
+
+inline _LIBCPP_HIDE_FROM_ABI __locale_t __newlocale(int __category_mask, const char* __locale, __locale_t __base) {
+ return ::newlocale(__category_mask, __locale, __base);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI void __freelocale(__locale_t __loc) { ::freelocale(__loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI char* __setlocale(int __category, char const* __locale) {
+ return ::setlocale(__category, __locale);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI __lconv_t* __localeconv(__locale_t& __loc) {
+ __locale_guard __current(__loc);
+ return std::localeconv();
+}
+
+#define _LIBCPP_COLLATE_MASK LC_COLLATE_MASK
+#define _LIBCPP_CTYPE_MASK LC_CTYPE_MASK
+#define _LIBCPP_MONETARY_MASK LC_MONETARY_MASK
+#define _LIBCPP_NUMERIC_MASK LC_NUMERIC_MASK
+#define _LIBCPP_TIME_MASK LC_TIME_MASK
+#define _LIBCPP_MESSAGES_MASK LC_MESSAGES_MASK
+#define _LIBCPP_ALL_MASK LC_ALL_MASK
+#define _LIBCPP_LC_ALL LC_ALL
+
+//
+// Strtonum functions
+//
+
+inline _LIBCPP_HIDE_FROM_ABI float __strtof(const char* __nptr, char** __endptr, __locale_t __loc) {
+ return ::strtof_l(__nptr, __endptr, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI double __strtod(const char* __nptr, char** __endptr, __locale_t __loc) {
+ return ::strtod_l(__nptr, __endptr, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI long double __strtold(const char* __nptr, char** __endptr, __locale_t __loc) {
+ return ::strtold_l(__nptr, __endptr, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI long long __strtoll(const char* __nptr, char** __endptr, int __base, __locale_t __loc) {
+ return ::strtoll_l(__nptr, __endptr, __base, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI unsigned long long
+__strtoull(const char* __nptr, char** __endptr, int __base, __locale_t __loc) {
+ return ::strtoull_l(__nptr, __endptr, __base, __loc);
+}
+
+//
+// Character manipulation functions
+//
+
+inline _LIBCPP_HIDE_FROM_ABI int __isdigit(int __c, __locale_t __loc) { return isdigit_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __isxdigit(int __c, __locale_t __loc) { return isxdigit_l(__c, __loc); }
+
+#if defined(_LIBCPP_BUILDING_LIBRARY)
+inline _LIBCPP_HIDE_FROM_ABI int __toupper(int __c, __locale_t __loc) { return toupper_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __tolower(int __c, __locale_t __loc) { return tolower_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __strcoll(const char* __s1, const char* __s2, __locale_t __loc) {
+ return strcoll_l(__s1, __s2, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __strxfrm(char* __dest, const char* __src, size_t __n, __locale_t __loc) {
+ return strxfrm_l(__dest, __src, __n, __loc);
+}
+
+# if _LIBCPP_HAS_WIDE_CHARACTERS
+inline _LIBCPP_HIDE_FROM_ABI int __iswctype(wint_t __c, wctype_t __type, __locale_t __loc) {
+ return iswctype_l(__c, __type, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswspace(wint_t __c, __locale_t __loc) { return iswspace_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswprint(wint_t __c, __locale_t __loc) { return iswprint_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswcntrl(wint_t __c, __locale_t __loc) { return iswcntrl_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswupper(wint_t __c, __locale_t __loc) { return iswupper_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswlower(wint_t __c, __locale_t __loc) { return iswlower_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswalpha(wint_t __c, __locale_t __loc) { return iswalpha_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswblank(wint_t __c, __locale_t __loc) { return iswblank_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswdigit(wint_t __c, __locale_t __loc) { return iswdigit_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswpunct(wint_t __c, __locale_t __loc) { return iswpunct_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __iswxdigit(wint_t __c, __locale_t __loc) { return iswxdigit_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI wint_t __towupper(wint_t __c, __locale_t __loc) { return towupper_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI wint_t __towlower(wint_t __c, __locale_t __loc) { return towlower_l(__c, __loc); }
+
+inline _LIBCPP_HIDE_FROM_ABI int __wcscoll(const wchar_t* __ws1, const wchar_t* __ws2, __locale_t __loc) {
+ return wcscoll_l(__ws1, __ws2, __loc);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __wcsxfrm(wchar_t* __dest, const wchar_t* __src, size_t __n, __locale_t __loc) {
+ return wcsxfrm_l(__dest, __src, __n, __loc);
+}
+# endif // _LIBCPP_HAS_WIDE_CHARACTERS
+
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__strftime(char* __s, size_t __max, const char* __format, const struct tm* __tm, __locale_t __loc) {
+ return strftime_l(__s, __max, __format, __tm, __loc);
+}
+
+//
+// Other functions
+//
+
+inline _LIBCPP_HIDE_FROM_ABI decltype(MB_CUR_MAX) __mb_len_max(__locale_t __loc) {
+ __locale_guard __current(__loc);
+ return MB_CUR_MAX;
+}
+
+# if _LIBCPP_HAS_WIDE_CHARACTERS
+inline _LIBCPP_HIDE_FROM_ABI wint_t __btowc(int __c, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::btowc(__c);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI int __wctob(wint_t __c, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::wctob(__c);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__wcsnrtombs(char* __dest, const wchar_t** __src, size_t __nwc, size_t __len, mbstate_t* __ps, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return ::wcsnrtombs(__dest, __src, __nwc, __len, __ps); // non-standard
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __wcrtomb(char* __s, wchar_t __wc, mbstate_t* __ps, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::wcrtomb(__s, __wc, __ps);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__mbsnrtowcs(wchar_t* __dest, const char** __src, size_t __nms, size_t __len, mbstate_t* __ps, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return ::mbsnrtowcs(__dest, __src, __nms, __len, __ps); // non-standard
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__mbrtowc(wchar_t* __pwc, const char* __s, size_t __n, mbstate_t* __ps, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::mbrtowc(__pwc, __s, __n, __ps);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI int __mbtowc(wchar_t* __pwc, const char* __pmb, size_t __max, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::mbtowc(__pwc, __pmb, __max);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t __mbrlen(const char* __s, size_t __n, mbstate_t* __ps, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::mbrlen(__s, __n, __ps);
+}
+
+inline _LIBCPP_HIDE_FROM_ABI size_t
+__mbsrtowcs(wchar_t* __dest, const char** __src, size_t __len, mbstate_t* __ps, __locale_t __loc) {
+ __locale_guard __current(__loc);
+ return std::mbsrtowcs(__dest, __src, __len, __ps);
+}
+# endif // _LIBCPP_HAS_WIDE_CHARACTERS
+#endif // _LIBCPP_BUILDING_LIBRARY
+
+#ifndef _LIBCPP_COMPILER_GCC // GCC complains that this can't be always_inline due to C-style varargs
+_LIBCPP_HIDE_FROM_ABI
+#endif
+inline _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 4, 5) int __snprintf(
+ char* __s, size_t __n, __locale_t __loc, const char* __format, ...) {
+ va_list __va;
+ va_start(__va, __format);
+ __locale_guard __current(__loc);
+ int __res = std::vsnprintf(__s, __n, __format, __va);
+ va_end(__va);
+ return __res;
+}
+
+#ifndef _LIBCPP_COMPILER_GCC // GCC complains that this can't be always_inline due to C-style varargs
+_LIBCPP_HIDE_FROM_ABI
+#endif
+inline _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 3, 4) int __asprintf(
+ char** __s, __locale_t __loc, const char* __format, ...) {
+ va_list __va;
+ va_start(__va, __format);
+ __locale_guard __current(__loc);
+ int __res = ::vasprintf(__s, __format, __va); // non-standard
+ va_end(__va);
+ return __res;
+}
+
+#ifndef _LIBCPP_COMPILER_GCC // GCC complains that this can't be always_inline due to C-style varargs
+_LIBCPP_HIDE_FROM_ABI
+#endif
+inline _LIBCPP_ATTRIBUTE_FORMAT(__scanf__, 3, 4) int __sscanf(
+ const char* __s, __locale_t __loc, const char* __format, ...) {
+ va_list __va;
+ va_start(__va, __format);
+ __locale_guard __current(__loc);
+ int __res = std::vsscanf(__s, __format, __va);
+ va_end(__va);
+ return __res;
+}
+
+} // namespace __locale
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___LOCALE_DIR_SUPPORT_NEWLIB_H
diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index 7ca57f6455dd8..f547e33a17dc8 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -1591,6 +1591,7 @@ module std [system] {
textual header "__locale_dir/support/fuchsia.h"
textual header "__locale_dir/support/linux.h"
textual header "__locale_dir/support/netbsd.h"
+ textual header "__locale_dir/support/newlib.h"
textual header "__locale_dir/support/no_locale/characters.h"
textual header "__locale_dir/support/no_locale/strtonum.h"
textual header "__locale_dir/support/windows.h"
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jroelofs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, but you should get @ldionne’s blessing.
_NEWLIB_VERSION is only visible when any libc header is included. I ran into a weird case where during libc++ compilation, __fwd/ios.h did not see _NEWLIB_VERSION and defined off_t as `long long`, but in the actual user program, _NEWLIB_VERSION was visible, so the program tried to use a `long int` instead of `long long` specialization of a template function that is provided by libc++.a, and caused linking failure. The new cmake option was also used in another PR that I created; see llvm#167962.
|
Friendly ping. @ldionne, may you take a look? |
a6643f2 implemented a better version of the cmake variables we need; this commit switches to using that, and removes the old version I implemented.
philnik777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you point out the changes compared to linux.h?
There are no substantial differences; it would work too if we just Originally I wanted to fork So, to make it short: do you think it's fine to simply change this: To this: Logically it doesn't quite make sense; it feels weird to include |
Many functions copied from linux.h are no longer needed after llvm@693f700. Also removed some whitespace changes to make newlib.h closer to linux.h.
|
The current difference between newlib.h and linux.h (this PR was synced to be based on upstream commit e110abc): $ diff libcxx/include/__locale_dir/support/linux.h libcxx/include/__locale_dir/support/newlib.h
9,10c9,10
< #ifndef _LIBCPP___LOCALE_DIR_SUPPORT_LINUX_H
< #define _LIBCPP___LOCALE_DIR_SUPPORT_LINUX_H
---
> #ifndef _LIBCPP___LOCALE_DIR_SUPPORT_NEWLIB_H
> #define _LIBCPP___LOCALE_DIR_SUPPORT_NEWLIB_H
15d14
< #include <__utility/forward.h>
244c243
< #endif // _LIBCPP___LOCALE_DIR_SUPPORT_LINUX_H
---
> #endif // _LIBCPP___LOCALE_DIR_SUPPORT_NEWLIB_H |
|
If there is no actual difference I think it's fine to just update the |
| # elif defined(__linux__) | ||
| # include <__locale_dir/support/linux.h> | ||
| # elif _LIBCPP_LIBC_NEWLIB | ||
| # include <__locale_dir/support/newlib.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Newlib ever used on a platform where __linux__ is defined? It seems to me like we might be checking the wrong thing by checking elif defined(__linux__) above, perhaps what we want is elif defined(_GLIBC)?
Also CC @reactive-firewall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, I recently realized that the -Dxxx approach might actually be okay, because it just exposes things that are compiled and included in libc.a anyways.
- Regarding bare-metal I generally agree;
- TL;DR - the
__linux__flag just declares the presence of the linux headers, which may or may not be relevant to the libc (which by standard (e.g. ISO 9989: ) has no requirement on the linux headers, e.g., musl and BSD libc can compile without the linux headers) so baremetal builds should not expect it to describe libc.
- TL;DR - the
EDIT: @ldionne
- It seems to me like we might be checking the wrong thing by checking
elif defined(__linux__)above, perhaps what we want iselif defined(_GLIBC)?- IIUC newlib has its requirements on a small subset of OS provided functions (with minimal implementations suggested in the docs), and is well suited to building without the
__linux__being defined when bootstraping a stage0 toolset, (possibly even for an eventual linux system). AFAIK, newlib does seem to provide thestrto*_lfunctions unconditionally (albeit not sure where in their codebase 🤷) - musl libc has no requirement on linux headers even when building for linux, rather they guard things based on macros like
_GNU_SOURCEor_BSD_SOURCE - IIUC glibc defines linux somewhere (I know GNU compilers pre-define it, albeit GNU lib != GNU compiler), so while misleading, may be correct in the specific case for identifying "official" (e.g. non-llvm) linux builds. However,
_GLIBCwould be more intuitive and granular for identifying the particular standard c library (e.g. glibc, stdlibc, libc, libsystem_c etc.)
- IIUC newlib has its requirements on a small subset of OS provided functions (with minimal implementations suggested in the docs), and is well suited to building without the
In summation: elif defined(_GLIBC) seems more granular and maintainable regarding near-baremetal (e.g. just minimal libc) use-cases.
🙇 Hopefully this helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review and insights, @ldionne and @reactive-firewall !
AFAIK, newlib does seem to provide the strto*_l functions unconditionally (albeit not sure where in their codebase 🤷)
For the record, they are defined in the non-_l .c files, e.g. newlib/libc/stdlib/strtol.c for strtol_l.
perhaps what we want is
elif defined(_GLIBC)?
Per my discussion with @philnik777 above, linux.h and newlib.h are essentially the same. So it sounds like we need something like this:
# elif defined(_GLIBC) || _LIBCPP_LIBC_NEWLIB
# include <__locale_dir/support/minimal_libc.h>
And rename linux.h to minimal_libc.h?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's going to sound weird, but for the moment I think I'd rather keep the changes minimal even if it incurs some code duplication. So for this patch, I prefer the current approach with
# elif _LIBCPP_LIBC_NEWLIB
# include <__locale_dir/support/newlib.h>
The reason is that I already want to formalize the way we define the C library we use, so there's some refactoring I'll need to do in that area. I also want to further refactor some of the localization base API stuff to better support LLVM-libc. So for the time being, please just follow the status quo closely and I'll think about how to refactor this in the future to reduce duplication a bit.
ldionne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my other comment, LGTM. I'd rather refactor this myself with the bigger picture than break the consistency right now.
Tested on Newlib v4.5.0. The newlib.h was mostly copied from linux.h within the same directory. Newlib locale support was initially added in http://llvm.org/D5385, but removed in llvm#113721 , because of revision http://llvm.org/D49927.
Tested on Newlib v4.5.0. The newlib.h was mostly copied from linux.h within the same directory.
Newlib locale support was initially added in D5385 1, but removed in revision cfde4fb 2, because of revision 78b6374 3.