[libcxx] Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to true if compiling with clang by vhscampos · Pull Request #152724 · llvm/llvm-project

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 or the supporting C library does not provide support for the corresponding features. These have been xfailed.

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.

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

libcxx/utils/libcxx/test/features.py

libcxx/include/__config

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

- 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.

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

- Add android to xfail. - Test char8_t unconditionally.

vhscampos · 2025-10-10T12:58:09Z

@philnik777 @ldionne
The premerge CI jobs found some outstanding issues. The PR still has some issues to iron out. Here's the summary:

armv7, armv8, aarch64: Linux, Arm targets, clang, glibc < 2.36. Macro is true, so char8functions are declared in libcxx despite not available in glibc. error: reference to unresolved using declaration.
stage1 (generic-gcc, gcc-16, g++-15): Linux, x86_64, gcc, glibc < 2.36. Macro is false, so char8functions are NOT declared in libcxx. error: ‘mbrtoc8’ is not a member of ‘std’.
stage1 (generic-cxx26, clang-22, clang++-22): Linux, x86_64, clang, glibc < 2.36. Macro is true, so char8functions are declared in libcxx despite not available in glibc. error: reference to unresolved using declaration.
stage 1 (generic-modules, clang-22, clang++-22): Linux, x86_64, clang, glibc < 2.36. Macro is true, so char8functions are declared in libcxx despite not available in glibc. error: reference to unresolved using declaration.

I don't know how to specify xfails to capture the conditions of these failures, or if it's even possible. If I xfail it on Linux across the board, I think it will unexpectedly pass on downstream runners that have a newer glibc.

I'm lost here. Do you have any recommendations to make progress on this?

DavidSpickett · 2025-10-10T13:44:06Z

If you can write out in pseudocode what your ideal XFAIL would be, I can try to figure it out. I've spent a lot of time in these test frameworks, for better or worse.

DavidSpickett · 2025-10-10T13:49:55Z

For inspecting glibc,

llvm-project/libcxx/utils/libcxx/test/features.py

Line 278 in 1e84cb5

# Check for Glibc < 2.27, where the ru_RU.UTF-8 locale had

is a feature check, and there is a function glibc_version_less_than that could help.

But I get the impression it has to be more than just "glibc >= some verison".

- Add new lit feature to check if glibc is used and if it has char8_t support. - Use this new lit feature for xfails.

vhscampos · 2025-10-16T09:32:25Z

@DavidSpickett Thanks for your comment. Inspired by your idea I created a new lit feature to check the glibc version if glibc is indeed the C library in use. Now I believe the xfails are correct. The failing CI jobs are unrelated.

@philnik777 for your attention, please. If can have another look to see if everything is ok.

vhscampos · 2025-10-24T08:36:37Z

Since the PR had received approval before my latest changes, I will merge it. If you find any issues with it after the merge, I’m happy to revert it and make any necessary changes.

…C8RTOMB_MBRTOC8 With the merging of llvm/llvm-project#152724 upstream, the downstream patch is not required anymore.

…C8RTOMB_MBRTOC8 (#603) With the merging of llvm/llvm-project#152724 upstream, the downstream patch is not required anymore.

…iling with clang" (#165268) Reverts #152724 The PR was merged with broken pre-commit CI.

philnik777 · 2025-10-27T16:09:47Z

This patch was reverted in #165268 because it was merged with a broken pre-commit CI. Please re-open the PR and make sure the CI is happy before merging. If you have problem with getting the CI happy feel free to ask.

…rue if compiling with clang" (#165268) Reverts llvm/llvm-project#152724 The PR was merged with broken pre-commit CI.

…th clang (llvm#152724) Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to `1` if compiling with clang. Some tests involving functionality from `uchar.h`/`cuchar` fail when the platform or the supporting C library does not provide support for the corresponding features. These have been xfailed. This patch will enable the adoption of newer picolibc versions.

…iling with clang" (llvm#165268) Reverts llvm#152724 The PR was merged with broken pre-commit CI.

vhscampos · 2025-10-28T17:18:08Z

My apologies. I was very careful, but obviously not enough. I am working on the patch again.

…th clang (llvm#152724) Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to `1` if compiling with clang. Some tests involving functionality from `uchar.h`/`cuchar` fail when the platform or the supporting C library does not provide support for the corresponding features. These have been xfailed. This patch will enable the adoption of newer picolibc versions.

…iling with clang" (llvm#165268) Reverts llvm#152724 The PR was merged with broken pre-commit CI.

…th clang (llvm#152724) Define `_LIBCPP_HAS_C8RTOMB_MBRTOC8` to `1` if compiling with clang. Some tests involving functionality from `uchar.h`/`cuchar` fail when the platform or the supporting C library does not provide support for the corresponding features. These have been xfailed. This patch will enable the adoption of newer picolibc versions.

…iling with clang" (llvm#165268) Reverts llvm#152724 The PR was merged with broken pre-commit CI.

[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

Closed

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

Closed

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

libcxx/test/std/depr/depr.c.headers/uchar_h_char8_t.compile.pass.cpp Outdated Show resolved Hide resolved

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

Libcpp has c8 rtomb mbrtoc8 experimental 2 #160856

Closed

Changes:

0eea082

- Add android to xfail. - Test char8_t unconditionally.

vhscampos added 2 commits October 14, 2025 14:01

Changes:

1b0da9c

- Add new lit feature to check if glibc is used and if it has char8_t support. - Use this new lit feature for xfails.

Merge branch 'main' into picolibc-_LIBCPP_HAS_C8RTOMB_MBRTOC8

ce47ec1

vhscampos merged commit d522b1b into llvm:main Oct 24, 2025
28 of 31 checks passed

vhscampos deleted the picolibc-_LIBCPP_HAS_C8RTOMB_MBRTOC8 branch October 24, 2025 08:39

vhscampos mentioned this pull request Oct 24, 2025

[ATfE] Remove llvm-project downstream patch that defined _LIBCPP_HAS_C8RTOMB_MBRTOC8 arm/arm-toolchain#603

Merged

vinay-deshmukh mentioned this pull request Oct 25, 2025

[libc++] Make <map> std::map constexpr as part of P3372R3 #134330

Open

philnik777 mentioned this pull request Oct 27, 2025

Revert "[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if compiling with clang" #165268

Merged

philnik777 added a commit that referenced this pull request Oct 27, 2025

Revert "[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if comp…

fc1f3f3

…iling with clang" (#165268) Reverts #152724 The PR was merged with broken pre-commit CI.

dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025

Revert "[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if comp…

3b14ce9

…iling with clang" (llvm#165268) Reverts llvm#152724 The PR was merged with broken pre-commit CI.

Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025

Revert "[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if comp…

6444704

…iling with clang" (llvm#165268) Reverts llvm#152724 The PR was merged with broken pre-commit CI.

aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025

Revert "[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if comp…

4f1f849

…iling with clang" (llvm#165268) Reverts llvm#152724 The PR was merged with broken pre-commit CI.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philnik777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vhscampos commented Oct 10, 2025

Uh oh!

DavidSpickett commented Oct 10, 2025

Uh oh!

DavidSpickett commented Oct 10, 2025

Uh oh!

vhscampos commented Oct 16, 2025

Uh oh!

Uh oh!

vhscampos commented Oct 24, 2025

Uh oh!

philnik777 commented Oct 27, 2025

Uh oh!

vhscampos commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vhscampos commented Aug 8, 2025 •

edited

Loading

github-actions bot commented Aug 8, 2025 •

edited

Loading