[libcxx] Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to true if compiling with clang #152724

vhscampos · 2025-08-08T13:38:53Z

Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to 1 if compiling with clang.

Some tests involving functionality from uchar.h/cuchar fail when the platform does not provide support for the corresponding features. These have been xfailed on the relevant platforms.

This patch will enable the adoption of newer picolibc versions.

Starting from picolibc 1.8.9, the `char8_t` related functions are provided. This patch adds logic to detect the underlying picolibc version and define the `_LIBCPP_HAS_C8RTOMB_MBRTOC8 macro` accordingly.

llvmbot · 2025-08-08T13:39:29Z

@llvm/pr-subscribers-libcxx

Author: Victor Campos (vhscampos)

Changes

Starting from picolibc 1.8.9, the char8_t related functions are provided.

This patch adds logic to detect the underlying picolibc version and define the _LIBCPP_HAS_C8RTOMB_MBRTOC8 macro accordingly.

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

2 Files Affected:

(modified) libcxx/include/__config (+6-2)
(modified) libcxx/include/__configuration/platform.h (+4)

diff --git a/libcxx/include/__config b/libcxx/include/__config
index 77a71b6cf1cae..ab87ed4787b84 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -1027,8 +1027,12 @@ typedef __char32_t char32_t;
 #    else
 #      define _LIBCPP_HAS_C8RTOMB_MBRTOC8 0
 #    endif
-#  else
-#    define _LIBCPP_HAS_C8RTOMB_MBRTOC8 0
+#  elif defined (_LIBCPP_PICOLIBC_PREREQ)
+#    if _LIBCPP_PICOLIBC_PREREQ(1, 8, 9) && defined(__cpp_char8_t)
+#      define _LIBCPP_HAS_C8RTOMB_MBRTOC8 1
+#    else
+#      define _LIBCPP_HAS_C8RTOMB_MBRTOC8 0
+#    endif
 #  endif
 
 // There are a handful of public standard library types that are intended to
diff --git a/libcxx/include/__configuration/platform.h b/libcxx/include/__configuration/platform.h
index f3c199dee172b..4f581a352038f 100644
--- a/libcxx/include/__configuration/platform.h
+++ b/libcxx/include/__configuration/platform.h
@@ -47,6 +47,10 @@
 //       user code. Move code paths that need _NEWLIB_VERSION to another customization mechanism.
 #if __has_include(<picolibc.h>)
 #  include <picolibc.h>
+#  define _LIBCPP_PICOLIBC_VERSION_INT(maj, min, patch) (maj * 10000 + min * 100 + patch)
+#  define _LIBCPP_PICOLIBC_PREREQ(maj, min, patch)                                                                     \
+    _LIBCPP_PICOLIBC_VERSION_INT(__PICOLIBC__, __PICOLIBC_MINOR__, __PICOLIBC_PATCHLEVEL__) >=                         \
+        _LIBCPP_PICOLIBC_VERSION_INT(maj, min, patch)
 #endif
 
 #ifndef __BYTE_ORDER__

github-actions · 2025-08-08T13:42:47Z

✅ With the latest revision this PR passed the C/C++ code formatter.

PR llvm/llvm-project#152724 adds logic to define the `_LIBCPP_HAS_C8RTOMB_MBRTOC8` macro depending on the picolibc support for `char8_t` functions. With this change we don't need a downstream patch anymore.

philnik777

I think it would be better to refactor _LIBCPP_HAS_C8RTOMB_MBRTOC8 so that it's true unless we know it to be false (and also unconditionally make it true with Clang).

vhscampos · 2025-08-11T09:58:13Z

@philnik777 I don't understand the two suggestions you made. Can you please give more details? Thank you

philnik777 · 2025-08-11T10:02:04Z

@philnik777 I don't understand the two suggestions you made. Can you please give more details? Thank you

_LIBCPP_HAS_C8RTOMB_MBRTOC8 is currently false if we don't know whether a library provides it or not. IMO we should assume it's available unless we know for a fact it's not. With Clang we don't care at all whether it's available, since we have _LIBCPP_USING_IF_EXISTS, so we can just always define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true in that case.

vhscampos · 2025-08-11T14:19:22Z

I've refactored in a way that the macro is true "by default" as you suggested.

However, with regards to always setting it to true if compiling with clang, that causes a libcxx test to fail (link:

llvm-project/libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp

Line 26 in 372d86d

#if !defined(TEST_HAS_NO_C8RTOMB_MBRTOC8)

)

The value of macro TEST_HAS_NO_C8RTOMB_MBRTOC8 depends on _LIBCPP_HAS_C8RTOMB_MBRTOC8 (link:

llvm-project/libcxx/test/support/test_macros.h

Line 453 in 372d86d

# define TEST_HAS_NO_C8RTOMB_MBRTOC8

), so if we always set it to true for clang, this test fails if the functions aren't defined in the C library.

I couldn't think of a way to modify the test in order to have the macro true for clang.

philnik777 · 2025-08-12T12:06:14Z

I've refactored in a way that the macro is true "by default" as you suggested.

However, with regards to always setting it to true if compiling with clang, that causes a libcxx test to fail (link:

llvm-project/libcxx/test/std/strings/c.strings/cuchar.compile.pass.cpp

Line 26 in 372d86d

#if !defined(TEST_HAS_NO_C8RTOMB_MBRTOC8)

)

The value of macro TEST_HAS_NO_C8RTOMB_MBRTOC8 depends on _LIBCPP_HAS_C8RTOMB_MBRTOC8 (link:

llvm-project/libcxx/test/support/test_macros.h

Line 453 in 372d86d

# define TEST_HAS_NO_C8RTOMB_MBRTOC8

), so if we always set it to true for clang, this test fails if the functions aren't defined in the C library.

I couldn't think of a way to modify the test in order to have the macro true for clang.

We should probably just XFAIL the test on platforms that haven't fully implemented the interface. @ldionne any thoughts?

vhscampos · 2025-08-12T15:10:23Z

Handling every case is brittle because I can't run the CI jobs locally, e.g. on AIX. Sorry for the high traffic.

vhscampos · 2025-09-09T09:35:57Z

@philnik777 Can you please have another look?

libcxx/include/__config

- Define macro unconditionally when the compiler is clang. - Remove preprocessor logic that isn't needed anymore. - Define new libcxx test feature for the AIX platform. - Mark tests as xfail in the platforms whose C library doesn't support the char8_t functions.

vhscampos · 2025-09-17T13:15:11Z

@philnik777 I've changed it around to unconditionally define the macro for clang, and then xfail tests in platforms whose C library doesn't provide support.

The CI premerge failures AFAIK are unrelated to the patch.

philnik777 · 2025-09-18T08:20:50Z

libcxx/test/std/strings/c.strings/no_c8rtomb_mbrtoc8.verify.cpp

 //
 //===----------------------------------------------------------------------===//

 // UNSUPPORTED: c++03


This test can be deleted IMO. It seems to provide very little value.

philnik777 · 2025-09-18T08:21:20Z

libcxx/utils/libcxx/test/features.py

            """,
        ),
    ),
+    Feature(name="aix", when=lambda cfg: "_AIX" in compilerMacros(cfg)),


Is there a reason you can't simply use the target triple?

philnik777 · 2025-09-18T08:29:24Z

libcxx/include/__config

+#  elif defined(__clang__)
+#    define _LIBCPP_HAS_C8RTOMB_MBRTOC8 1


I think I'd rather have this as the first option to make it clear the rest is purely for GCC support.

philnik777 · 2025-09-18T08:33:02Z

libcxx/test/std/depr/depr.c.headers/uchar_h.compile.pass.cpp

+// The following platforms do not provide mbrtoc8 and c8rtomb so the tests fail
+// XFAIL: aix
+// XFAIL: freebsd
+// XFAIL: windows


Could we move the mbrtoc8 and c8rtomb into a separate test and XFAIL that? Same for the other test.

- Move clang's case to the top. - Extract char8_t part of the tests into its own files. - Remove test that has little use. - Undo the creation of a feature just for the AIX platform.

philnik777

LGTM % nit.

philnik777 · 2025-09-19T09:59:09Z

libcxx/test/std/depr/depr.c.headers/uchar_h_char8_t.compile.pass.cpp

+
+#include "test_macros.h"
+
+#if !defined(TEST_HAS_NO_C8RTOMB_MBRTOC8)


I don't think we need to keep this around. We're XFAILing the test for the platforms where it doesn't exist anyways.

[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 for picolibc

cd932a1

Starting from picolibc 1.8.9, the `char8_t` related functions are provided. This patch adds logic to detect the underlying picolibc version and define the `_LIBCPP_HAS_C8RTOMB_MBRTOC8 macro` accordingly.

vhscampos requested a review from a team as a code owner August 8, 2025 13:38

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 8, 2025

Fix clang-format issue

b606497

vhscampos mentioned this pull request Aug 8, 2025

[ATfE] Remove downstream patch that defined _LIBCPP_HAS_C8RTOMB_MBRTOC8 arm/arm-toolchain#476

Closed

This was referenced Aug 8, 2025

[ATfE] Remove downstream patch that defined _LIBCPP_HAS_C8RTOMB_MBRTOC8 arm/arm-toolchain#477

Draft

[libcxx] Update CI picolibc version to v1.8.10 #152731

Draft

philnik777 reviewed Aug 11, 2025

View reviewed changes

Refactor the preprocessor logic

8d87631

vhscampos added 2 commits August 12, 2025 14:15

Handle Apple and MSVC explictly

91ea91e

Handle AIX

def1139

vhscampos added 2 commits August 13, 2025 12:12

Handle FreeBSD and Microsoft C Runtime library

5d7491a

Detect the UCRT library

27129d4

philnik777 reviewed Sep 15, 2025

View reviewed changes

libcxx/include/__config Outdated Show resolved Hide resolved

libcxx/include/__config Outdated Show resolved Hide resolved

vhscampos added 2 commits September 16, 2025 16:13

List of changes:

205652c

- Define macro unconditionally when the compiler is clang. - Remove preprocessor logic that isn't needed anymore. - Define new libcxx test feature for the AIX platform. - Mark tests as xfail in the platforms whose C library doesn't support the char8_t functions.

Add xfail for picolibc

1735435

vhscampos changed the title ~~[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 for picolibc~~ [libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if compiling with clang Sep 17, 2025

vhscampos requested a review from philnik777 September 17, 2025 13:19

philnik777 requested changes Sep 18, 2025

View reviewed changes

List of changes:

678c833

- Move clang's case to the top. - Extract char8_t part of the tests into its own files. - Remove test that has little use. - Undo the creation of a feature just for the AIX platform.

philnik777 approved these changes Sep 19, 2025

View reviewed changes

frederick-vs-ja mentioned this pull request Sep 26, 2025

Libcpp has c8 rtomb mbrtoc8 experimental 2 #160856

Draft

		# elif defined(__clang__)
		# define _LIBCPP_HAS_C8RTOMB_MBRTOC8 1


		#include "test_macros.h"

		#if !defined(TEST_HAS_NO_C8RTOMB_MBRTOC8)

[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if compiling with clang #152724

Are you sure you want to change the base?

[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if compiling with clang #152724

Uh oh!

Conversation

vhscampos commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 8, 2025

Uh oh!

github-actions bot commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philnik777 left a comment

Choose a reason for hiding this comment

Uh oh!

vhscampos commented Aug 11, 2025

Uh oh!

philnik777 commented Aug 11, 2025

Uh oh!

vhscampos commented Aug 11, 2025

Uh oh!

philnik777 commented Aug 12, 2025

Uh oh!

vhscampos commented Aug 12, 2025

Uh oh!

vhscampos commented Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

vhscampos commented Sep 17, 2025

Uh oh!

philnik777 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 left a comment

Choose a reason for hiding this comment

Uh oh!

philnik777 Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[libcxx] Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to true if compiling with clang #152724

[libcxx] Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to true if compiling with clang #152724

vhscampos commented Aug 8, 2025 •

edited

Loading

github-actions bot commented Aug 8, 2025 •

edited

Loading