[libc++] Add support for picolibc and newlib in RUNTIMES_USE_LIBC #147956

voltur01 · 2025-07-10T13:08:14Z

This replaces detection of picolibc in libc++ (by checking for and including picolibc.h) with using RUNTIMES_USE_LIBC build time option intriduced in #134893 RUNTIMES_USE_LIBC is extended to accept picolibc and newlib.

Detection of picolibc via the header is kept as a deprecated feature to avoid breaking builds.

libc++ is updated to use dedicated LIBCXX_LIBC_NEWLIB macro to check for newlib specific conditions instead of less informative _NEWLIB_VERSION

This replaces detection of picolibc in libc++ (by checking for and including picolibc.h) with using RUNTIMES_USE_LIBC build time option intriduced in llvm#134893 RUNTIMES_USE_LIBC is extended to accept picolibc and newlib. Detection of picolibc via the header is kept as a deprecated feature to avoid breaking builds. libc++ is updated to use dedicated LIBCXX_LIBC_NEWLIB macro to check for newlib specific conditions instead of less informative _NEWLIB_VERSION

llvmbot · 2025-07-10T13:08:48Z

@llvm/pr-subscribers-libcxx

Author: Volodymyr Turanskyy (voltur01)

Changes

This replaces detection of picolibc in libc++ (by checking for and including picolibc.h) with using RUNTIMES_USE_LIBC build time option intriduced in #134893 RUNTIMES_USE_LIBC is extended to accept picolibc and newlib.

Detection of picolibc via the header is kept as a deprecated feature to avoid breaking builds.

libc++ is updated to use dedicated LIBCXX_LIBC_NEWLIB macro to check for newlib specific conditions instead of less informative _NEWLIB_VERSION

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

22 Files Affected:

(modified) libcxx/CMakeLists.txt (+15)
(modified) libcxx/include/__config_site.in (+5)
(modified) libcxx/include/__configuration/platform.h (+3-4)
(modified) libcxx/include/__cxx03/__fwd/ios.h (+1-1)
(modified) libcxx/include/__cxx03/__locale (+1-1)
(modified) libcxx/include/__cxx03/__locale_dir/locale_base_api.h (+1-1)
(modified) libcxx/include/__cxx03/fstream (+1-1)
(modified) libcxx/include/__cxx03/locale (+1-1)
(modified) libcxx/include/__cxx03/regex (+2-4)
(modified) libcxx/include/__fwd/ios.h (+1-1)
(modified) libcxx/include/__locale (+1-1)
(modified) libcxx/include/__locale_dir/messages.h (+1-1)
(modified) libcxx/include/fstream (+2-2)
(modified) libcxx/include/regex (+2-4)
(modified) libcxx/src/include/config_elast.h (+1-1)
(modified) libcxx/src/locale.cpp (+1-1)
(modified) libcxx/test/libcxx/system_reserved_names.gen.py (+2-2)
(modified) libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp (+1-1)
(modified) libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp (+1-1)
(modified) libcxx/test/support/platform_support.h (+1-1)
(modified) libcxx/utils/ci/run-buildbot (+1)
(modified) runtimes/cmake/Modules/HandleLibC.cmake (+2-2)

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 85514cc7547a9..5c0ea39473ec4 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -481,6 +481,21 @@ include(config-ix)
 include(HandleLibC) # Setup the C library flags
 include(HandleLibCXXABI) # Setup the ABI library flags
 
+# Set C library in use to define respective macro in __config_site
+# RUNTIMES_USE_LIBC was checked in HandleLibC to be one of accepted values
+if (RUNTIMES_USE_LIBC STREQUAL "llvm-libc")
+  set(LIBCXX_LIBC_LLVMLIBC 1)
+elseif (RUNTIMES_USE_LIBC STREQUAL "picolibc")
+  set(LIBCXX_LIBC_PICOLIBC 1)
+  # picolibc is derived from newlib and behaves the same in regards to libc++
+  # so setting both here:
+  # * LIBCXX_LIBC_NEWLIB is used now
+  # * LIBCXX_LIBC_PICOLIBC can be used for further customizations later
+  set(LIBCXX_LIBC_NEWLIB 1) 
+elseif (RUNTIMES_USE_LIBC STREQUAL "newlib")
+  set(LIBCXX_LIBC_NEWLIB 1)
+endif()
+
 # FIXME(EricWF): See the FIXME on LIBCXX_ENABLE_PEDANTIC.
 # Remove the -pedantic flag and -Wno-pedantic and -pedantic-errors
 # so they don't get transformed into -Wno and -errors respectively.
diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index fc01aaf2d8746..f01400bd351ec 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -42,6 +42,11 @@
 // Hardening.
 #cmakedefine _LIBCPP_HARDENING_MODE_DEFAULT @_LIBCPP_HARDENING_MODE_DEFAULT@
 
+// C libraries
+#cmakedefine LIBCXX_LIBC_LLVMLIBC
+#cmakedefine LIBCXX_LIBC_PICOLIBC
+#cmakedefine LIBCXX_LIBC_NEWLIB
+
 // __USE_MINGW_ANSI_STDIO gets redefined on MinGW
 #ifdef __clang__
 #  pragma clang diagnostic push
diff --git a/libcxx/include/__configuration/platform.h b/libcxx/include/__configuration/platform.h
index f3c199dee172b..cb97c97c51ae4 100644
--- a/libcxx/include/__configuration/platform.h
+++ b/libcxx/include/__configuration/platform.h
@@ -42,11 +42,10 @@
 #  endif
 #endif
 
-// This is required in order for _NEWLIB_VERSION to be defined in places where we use it.
-// TODO: We shouldn't be including arbitrarily-named headers from libc++ since this can break valid
-//       user code. Move code paths that need _NEWLIB_VERSION to another customization mechanism.
+// TODO: Remove this deprecated behavior after LLVM 22 release
+// To build libc++ with picolibc provide RUNTIMES_USE_LIBC=picolibc
 #if __has_include(<picolibc.h>)
-#  include <picolibc.h>
+#  define LIBCXX_LIBC_NEWLIB
 #endif
 
 #ifndef __BYTE_ORDER__
diff --git a/libcxx/include/__cxx03/__fwd/ios.h b/libcxx/include/__cxx03/__fwd/ios.h
index dc03e8c6bab2f..5776ad90ea623 100644
--- a/libcxx/include/__cxx03/__fwd/ios.h
+++ b/libcxx/include/__cxx03/__fwd/ios.h
@@ -31,7 +31,7 @@ using wios = basic_ios<wchar_t>;
 template <class _CharT, class _Traits>
 class _LIBCPP_PREFERRED_NAME(ios) _LIBCPP_IF_WIDE_CHARACTERS(_LIBCPP_PREFERRED_NAME(wios)) basic_ios;
 
-#if defined(_NEWLIB_VERSION)
+#if defined(LIBCXX_LIBC_NEWLIB)
 // On newlib, off_t is 'long int'
 using streamoff = long int; // for char_traits in <string>
 #else
diff --git a/libcxx/include/__cxx03/__locale b/libcxx/include/__cxx03/__locale
index d5faa89b99fc0..d32ac05d28187 100644
--- a/libcxx/include/__cxx03/__locale
+++ b/libcxx/include/__cxx03/__locale
@@ -384,7 +384,7 @@ public:
   static const mask xdigit       = _ISXDIGIT;
   static const mask blank        = _ISBLANK;
   static const mask __regex_word = 0x8000;
-#elif defined(_NEWLIB_VERSION)
+#elif defined(LIBCXX_LIBC_NEWLIB)
   // Same type as Newlib's _ctype_ array in newlib/libc/include/ctype.h.
   typedef char mask;
   // In case char is signed, static_cast is needed to avoid warning on
diff --git a/libcxx/include/__cxx03/__locale_dir/locale_base_api.h b/libcxx/include/__cxx03/__locale_dir/locale_base_api.h
index a20f0952f52c3..ccb8201e8ab1d 100644
--- a/libcxx/include/__cxx03/__locale_dir/locale_base_api.h
+++ b/libcxx/include/__cxx03/__locale_dir/locale_base_api.h
@@ -17,7 +17,7 @@
 #  include <__cxx03/__locale_dir/locale_base_api/android.h>
 #elif defined(__sun__)
 #  include <__cxx03/__locale_dir/locale_base_api/solaris.h>
-#elif defined(_NEWLIB_VERSION)
+#elif defined(LIBCXX_LIBC_NEWLIB)
 #  include <__cxx03/__locale_dir/locale_base_api/newlib.h>
 #elif defined(__OpenBSD__)
 #  include <__cxx03/__locale_dir/locale_base_api/openbsd.h>
diff --git a/libcxx/include/__cxx03/fstream b/libcxx/include/__cxx03/fstream
index 44bdabc4602b5..f7290878bbc07 100644
--- a/libcxx/include/__cxx03/fstream
+++ b/libcxx/include/__cxx03/fstream
@@ -209,7 +209,7 @@ typedef basic_fstream<wchar_t> wfstream;
 _LIBCPP_PUSH_MACROS
 #include <__cxx03/__undef_macros>
 
-#if defined(_LIBCPP_MSVCRT) || defined(_NEWLIB_VERSION)
+#if defined(_LIBCPP_MSVCRT) || defined(LIBCXX_LIBC_NEWLIB)
 #  define _LIBCPP_HAS_NO_OFF_T_FUNCTIONS
 #endif
 
diff --git a/libcxx/include/__cxx03/locale b/libcxx/include/__cxx03/locale
index 64162f5a4ff2c..0de1380601855 100644
--- a/libcxx/include/__cxx03/locale
+++ b/libcxx/include/__cxx03/locale
@@ -220,7 +220,7 @@ template <class charT> class messages_byname;
 
 #  if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
 // Most unix variants have catopen.  These are the specific ones that don't.
-#    if !defined(__BIONIC__) && !defined(_NEWLIB_VERSION) && !defined(__EMSCRIPTEN__)
+#    if !defined(__BIONIC__) && !defined(LIBCXX_LIBC_NEWLIB) && !defined(__EMSCRIPTEN__)
 #      define _LIBCPP_HAS_CATOPEN 1
 #      include <nl_types.h>
 #    endif
diff --git a/libcxx/include/__cxx03/regex b/libcxx/include/__cxx03/regex
index b96d59d3a252a..f282c983a7392 100644
--- a/libcxx/include/__cxx03/regex
+++ b/libcxx/include/__cxx03/regex
@@ -984,7 +984,7 @@ public:
   typedef _CharT char_type;
   typedef basic_string<char_type> string_type;
   typedef locale locale_type;
-#if defined(__BIONIC__) || defined(_NEWLIB_VERSION)
+#if defined(__BIONIC__) || defined(LIBCXX_LIBC_NEWLIB)
   // Originally bionic's ctype_base used its own ctype masks because the
   // builtin ctype implementation wasn't in libc++ yet. Bionic's ctype mask
   // was only 8 bits wide and already saturated, so it used a wider type here
@@ -993,9 +993,7 @@ public:
   // implementation, but this was not updated to match. Since then Android has
   // needed to maintain a stable libc++ ABI, and this can't be changed without
   // an ABI break.
-  // We also need this workaround for newlib since _NEWLIB_VERSION is not
-  // defined yet inside __config, so we can't set the
-  // _LIBCPP_PROVIDES_DEFAULT_RUNE_TABLE macro. Additionally, newlib is
+  // We also need this workaround for newlib since newlib is
   // often used for space constrained environments, so it makes sense not to
   // duplicate the ctype table.
   typedef uint16_t char_class_type;
diff --git a/libcxx/include/__fwd/ios.h b/libcxx/include/__fwd/ios.h
index 831624f4b1c57..4db2c5da3861d 100644
--- a/libcxx/include/__fwd/ios.h
+++ b/libcxx/include/__fwd/ios.h
@@ -31,7 +31,7 @@ using wios = basic_ios<wchar_t>;
 template <class _CharT, class _Traits>
 class _LIBCPP_PREFERRED_NAME(ios) _LIBCPP_IF_WIDE_CHARACTERS(_LIBCPP_PREFERRED_NAME(wios)) basic_ios;
 
-#if defined(_NEWLIB_VERSION)
+#if defined(LIBCXX_LIBC_NEWLIB)
 // On newlib, off_t is 'long int'
 using streamoff = long int; // for char_traits in <string>
 #else
diff --git a/libcxx/include/__locale b/libcxx/include/__locale
index 757a53951f66e..50422b5af9963 100644
--- a/libcxx/include/__locale
+++ b/libcxx/include/__locale
@@ -389,7 +389,7 @@ public:
   static const mask xdigit       = _ISXDIGIT;
   static const mask blank        = _ISBLANK;
   static const mask __regex_word = 0x8000;
-#  elif defined(_NEWLIB_VERSION)
+#  elif defined(LIBCXX_LIBC_NEWLIB)
   // Same type as Newlib's _ctype_ array in newlib/libc/include/ctype.h.
   typedef char mask;
   // In case char is signed, static_cast is needed to avoid warning on
diff --git a/libcxx/include/__locale_dir/messages.h b/libcxx/include/__locale_dir/messages.h
index c04bf04025ff0..4ebedc9de6d45 100644
--- a/libcxx/include/__locale_dir/messages.h
+++ b/libcxx/include/__locale_dir/messages.h
@@ -22,7 +22,7 @@
 
 #  if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
 // Most unix variants have catopen.  These are the specific ones that don't.
-#    if !defined(__BIONIC__) && !defined(_NEWLIB_VERSION) && !defined(__EMSCRIPTEN__)
+#    if !defined(__BIONIC__) && !defined(LIBCXX_LIBC_NEWLIB) && !defined(__EMSCRIPTEN__)
 #      define _LIBCPP_HAS_CATOPEN 1
 #      include <nl_types.h>
 #    else
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index c86f709bedb80..9b6b0267267e6 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -964,7 +964,7 @@ template <class _CharT, class _Traits>
 int basic_filebuf<_CharT, _Traits>::__fseek(FILE* __file, pos_type __offset, int __whence) {
 #    if defined(_LIBCPP_MSVCRT_LIKE)
   return _fseeki64(__file, __offset, __whence);
-#    elif defined(_NEWLIB_VERSION)
+#    elif defined(LIBCXX_LIBC_NEWLIB)
   return fseek(__file, __offset, __whence);
 #    else
   return ::fseeko(__file, __offset, __whence);
@@ -975,7 +975,7 @@ template <class _CharT, class _Traits>
 typename basic_filebuf<_CharT, _Traits>::pos_type basic_filebuf<_CharT, _Traits>::__ftell(FILE* __file) {
 #    if defined(_LIBCPP_MSVCRT_LIKE)
   return _ftelli64(__file);
-#    elif defined(_NEWLIB_VERSION)
+#    elif defined(LIBCXX_LIBC_NEWLIB)
   return ftell(__file);
 #    else
   return ftello(__file);
diff --git a/libcxx/include/regex b/libcxx/include/regex
index bbc21e244dd17..29ba9f6a06725 100644
--- a/libcxx/include/regex
+++ b/libcxx/include/regex
@@ -1004,7 +1004,7 @@ public:
   typedef _CharT char_type;
   typedef basic_string<char_type> string_type;
   typedef locale locale_type;
-#    if defined(__BIONIC__) || defined(_NEWLIB_VERSION)
+#    if defined(__BIONIC__) || defined(LIBCXX_LIBC_NEWLIB)
   // Originally bionic's ctype_base used its own ctype masks because the
   // builtin ctype implementation wasn't in libc++ yet. Bionic's ctype mask
   // was only 8 bits wide and already saturated, so it used a wider type here
@@ -1013,9 +1013,7 @@ public:
   // implementation, but this was not updated to match. Since then Android has
   // needed to maintain a stable libc++ ABI, and this can't be changed without
   // an ABI break.
-  // We also need this workaround for newlib since _NEWLIB_VERSION is not
-  // defined yet inside __config, so we can't set the
-  // _LIBCPP_PROVIDES_DEFAULT_RUNE_TABLE macro. Additionally, newlib is
+  // We also need this workaround for newlib since newlib is
   // often used for space constrained environments, so it makes sense not to
   // duplicate the ctype table.
   typedef uint16_t char_class_type;
diff --git a/libcxx/src/include/config_elast.h b/libcxx/src/include/config_elast.h
index 7edff2d9375d4..388286e41330f 100644
--- a/libcxx/src/include/config_elast.h
+++ b/libcxx/src/include/config_elast.h
@@ -23,7 +23,7 @@
 #  define _LIBCPP_ELAST ELAST
 #elif defined(__LLVM_LIBC__)
 // No _LIBCPP_ELAST needed for LLVM libc
-#elif defined(_NEWLIB_VERSION)
+#elif defined(LIBCXX_LIBC_NEWLIB)
 #  define _LIBCPP_ELAST __ELASTERROR
 #elif defined(__NuttX__)
 // No _LIBCPP_ELAST needed on NuttX
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index da735865c322c..da4d16c7963f9 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -933,7 +933,7 @@ const ctype<char>::mask* ctype<char>::classic_table() noexcept {
   return __pctype_func();
 #  elif defined(__EMSCRIPTEN__)
   return *__ctype_b_loc();
-#  elif defined(_NEWLIB_VERSION)
+#  elif defined(LIBCXX_LIBC_NEWLIB)
   // Newlib has a 257-entry table in ctype_.c, where (char)0 starts at [1].
   return _ctype_ + 1;
 #  elif defined(_AIX)
diff --git a/libcxx/test/libcxx/system_reserved_names.gen.py b/libcxx/test/libcxx/system_reserved_names.gen.py
index a4f2928eda332..5e4809d20fd82 100644
--- a/libcxx/test/libcxx/system_reserved_names.gen.py
+++ b/libcxx/test/libcxx/system_reserved_names.gen.py
@@ -78,7 +78,7 @@
 
 // Test that libc++ doesn't use names that collide with FreeBSD system macros.
 // newlib and picolibc also define these macros
-#if !defined(__FreeBSD__) && !defined(_NEWLIB_VERSION)
+#if !defined(__FreeBSD__) && !defined(LIBCXX_LIBC_NEWLIB)
 #  define __null_sentinel SYSTEM_RESERVED_NAME
 #  define __generic SYSTEM_RESERVED_NAME
 #endif
@@ -117,7 +117,7 @@
 #endif
 
 // Newlib & picolibc use __input as a parameter name of a64l & l64a
-#ifndef _NEWLIB_VERSION
+#ifndef LIBCXX_LIBC_NEWLIB
 # define __input SYSTEM_RESERVED_NAME
 #endif
 #define __output SYSTEM_RESERVED_NAME
diff --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
index 5425203304014..b4fa377b7f101 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
@@ -48,7 +48,7 @@ int main(int, char**)
         // responds with an empty message, which we probably want to
         // treat as a failure code otherwise, but we can detect that
         // with the preprocessor.
-#if defined(_NEWLIB_VERSION)
+#if defined(LIBCXX_LIBC_NEWLIB)
         const bool is_newlib = true;
 #else
         const bool is_newlib = false;
diff --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
index 255cbe75e2fa9..ca3a57006f0d4 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
@@ -59,7 +59,7 @@ int main(int, char**) {
     // responds with an empty message, which we probably want to
     // treat as a failure code otherwise, but we can detect that
     // with the preprocessor.
-#if defined(_NEWLIB_VERSION)
+#if defined(LIBCXX_LIBC_NEWLIB)
     const bool is_newlib = true;
 #else
     const bool is_newlib = false;
diff --git a/libcxx/test/support/platform_support.h b/libcxx/test/support/platform_support.h
index 99e60f60c5998..655641e858468 100644
--- a/libcxx/test/support/platform_support.h
+++ b/libcxx/test/support/platform_support.h
@@ -48,7 +48,7 @@
 # include <string.h> // strverscmp
 #endif
 
-#if defined(_NEWLIB_VERSION) && defined(__STRICT_ANSI__)
+#if defined(LIBCXX_LIBC_NEWLIB) && defined(__STRICT_ANSI__)
 // Newlib provides this, but in the header it's under __STRICT_ANSI__
 extern "C" {
   int mkstemp(char*);
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index d8b23be9a0323..9bd63d175a74c 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -230,6 +230,7 @@ function test-armv7m-picolibc() {
         -DLIBUNWIND_TEST_CONFIG="armv7m-picolibc-libunwind.cfg.in" \
         -DCMAKE_C_FLAGS="${flags}" \
         -DCMAKE_CXX_FLAGS="${flags}" \
+        -DRUNTIMES_USE_LIBC=picolibc \
         "${@}"
 
     step "Installing compiler-rt"
diff --git a/runtimes/cmake/Modules/HandleLibC.cmake b/runtimes/cmake/Modules/HandleLibC.cmake
index 51fbf04df7e3b..23f735aee6c02 100644
--- a/runtimes/cmake/Modules/HandleLibC.cmake
+++ b/runtimes/cmake/Modules/HandleLibC.cmake
@@ -10,10 +10,10 @@
 
 include_guard(GLOBAL)
 
-set(RUNTIMES_SUPPORTED_C_LIBRARIES system llvm-libc)
+set(RUNTIMES_SUPPORTED_C_LIBRARIES system llvm-libc picolibc newlib)
 set(RUNTIMES_USE_LIBC "system" CACHE STRING "Specify C library to use. Supported values are ${RUNTIMES_SUPPORTED_C_LIBRARIES}.")
 if (NOT "${RUNTIMES_USE_LIBC}" IN_LIST RUNTIMES_SUPPORTED_C_LIBRARIES)
-  message(FATAL_ERROR "Unsupported C library: '${RUNTIMES_CXX_ABI}'. Supported values are ${RUNTIMES_SUPPORTED_C_LIBRARIES}.")
+  message(FATAL_ERROR "Unsupported C library: '${RUNTIMES_USE_LIBC}'. Supported values are ${RUNTIMES_SUPPORTED_C_LIBRARIES}.")
 endif()
 
 # Link against a system-provided libc

pratlucas

Just a small comment, but LGTM otherwise.

libcxx/include/__configuration/platform.h

pratlucas

LGTM, but as per the contribution policy this PR probably needs approval from someone from the libc++ review group.

ldionne

Thanks for the patch! I have some comments but this makes sense to me.

ldionne · 2025-07-15T17:24:01Z

libcxx/CMakeLists.txt

@@ -481,6 +481,21 @@ include(config-ix)
 include(HandleLibC) # Setup the C library flags
 include(HandleLibCXXABI) # Setup the ABI library flags

+# Set C library in use to define respective macro in __config_site


I would suggest we do this instead (around line 753 where we handle other __config_site variables):

if (RUNTIMES_USE_LIBC STREQUAL "llvm-libc") config_define(_LIBCPP_LIBC_LLVMLIBC 1) elseif (RUNTIMES_USE_LIBC STREQUAL "picolibc") config_define(_LIBCPP_LIBC_PICOLIBC 1) elseif (RUNTIMES_USE_LIBC STREQUAL "newlib") config_define(_LIBCPP_LIBC_NEWLIB 1) endif()

That's more consistent with what we do for the other __config_site macros.

Thank you for review!

Good point, moved and made use of config_define() for consistency.

ldionne · 2025-07-15T17:25:04Z

libcxx/CMakeLists.txt

+  # picolibc is derived from newlib and behaves the same in regards to libc++
+  # so setting both here:
+  # * LIBCXX_LIBC_NEWLIB is used now
+  # * LIBCXX_LIBC_PICOLIBC can be used for further customizations later
+  set(LIBCXX_LIBC_NEWLIB 1) 


Do you absolutely need this right now (if so, why)? I think it would be cleaner to have a 1-1 mapping between the macros we define and the libc we're using, otherwise things become confusing when reading the source.

It was for completeness only - removed now.

ldionne · 2025-07-15T17:26:05Z

libcxx/CMakeLists.txt

@@ -481,6 +481,21 @@ include(config-ix)
 include(HandleLibC) # Setup the C library flags


General comment on the whole patch: Let's use _LIBCPP_foo instead of LIBCXX_foo for the macro names. That's what we do consistently everywhere, and we must use a reserved identifier.

Indeed, done.

ldionne · 2025-07-15T17:27:04Z

libcxx/include/__config_site.in

+#cmakedefine LIBCXX_LIBC_LLVMLIBC
+#cmakedefine LIBCXX_LIBC_PICOLIBC
+#cmakedefine LIBCXX_LIBC_NEWLIB


I'd use #cmakedefine01 here and then I'd check #if _LIBCPP_LIBC_NEWLIB instead of #if defined(_LIBCPP_LIBC_NEWLIB). This can prevent mistakes when coupled with -Wundef.

Very good suggestion, done - the checks are much cleaner now.

ldionne · 2025-07-15T17:28:54Z

libcxx/include/__configuration/platform.h

-//       user code. Move code paths that need _NEWLIB_VERSION to another customization mechanism.
-#if __has_include(<picolibc.h>)
-#  include <picolibc.h>
+// TODO: Remove this deprecated behavior after LLVM 22 release


Since this is something that vendors should change, I'm not sure there's a benefit from deprecating this before we remove it.

Agreed, I do not expect many people using it anyway - removed.

libcxx/include/__cxx03/__fwd/ios.h

runtimes/cmake/Modules/HandleLibC.cmake

- Use _LIBCXX_... reserved namespace macro names. - Move library definition code closer to the rest of __config_site definitions in CMake. - Remove unused LIBCXX_LIBC_NEWLIB definition. - Use #cmakedefine01 and thus avoid defined() in conditional statements. - Remove deprecation check.

arichardson · 2025-07-18T14:57:52Z

It looks to me like this will break existing builds that don't define the macro. I think for at least a while there should be a CMake check for picoline.h (and the newlib equivalent) to set these macros correctly and emit a warning to update the build config?

voltur01 · 2025-07-21T10:17:27Z

Thank you for the review @arichardson! There is a way to provide a warning like in this commit a876988 however I am not sure how many people are actually building with picolibc and will benefit from it.

I think this is up to maintainers to decide if the warning should be included or not.

arichardson · 2025-07-22T05:06:13Z

Thank you for the review @arichardson! There is a way to provide a warning like in this commit a876988 however I am not sure how many people are actually building with picolibc and will benefit from it.

I think this is up to maintainers to decide if the warning should be included or not.

Yeah I'm not sure how important this is. I am using the existing implicit support but I can easily update my build scripts.
I think it would be quite nice to detect this at CMake time and give a warning, but probably no need to add the checks in the headers themselves.
IMO this is a question for the libc++ maintainers

voltur01 requested a review from a team as a code owner July 10, 2025 13:08

llvmbot added cmake Build system in general and CMake in particular libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Jul 10, 2025

voltur01 mentioned this pull request Jul 10, 2025

[RFC] Set C library for libc++ to avoid including extra headers arm/arm-toolchain#416

Open

pratlucas reviewed Jul 11, 2025

View reviewed changes

libcxx/include/__configuration/platform.h Outdated Show resolved Hide resolved

Provide a warning if RUNTIMES_USE_LIBC is not used

a876988

pratlucas approved these changes Jul 15, 2025

View reviewed changes

ldionne requested changes Jul 15, 2025

View reviewed changes

		@@ -481,6 +481,21 @@ include(config-ix)
		include(HandleLibC) # Setup the C library flags

[libc++] Add support for picolibc and newlib in RUNTIMES_USE_LIBC #147956

Are you sure you want to change the base?

[libc++] Add support for picolibc and newlib in RUNTIMES_USE_LIBC #147956

Uh oh!

Conversation

voltur01 commented Jul 10, 2025

Uh oh!

llvmbot commented Jul 10, 2025

Uh oh!

pratlucas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pratlucas left a comment

Choose a reason for hiding this comment

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arichardson commented Jul 18, 2025

Uh oh!

voltur01 commented Jul 21, 2025

Uh oh!

arichardson commented Jul 22, 2025

Uh oh!

Uh oh!