[libcxx] Define _LIBCPP_HAS_C8RTOMB_MBRTOC8 to true if compiling with clang

[vhscampos]

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.

[llvmbot]

@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]

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

[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]

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

[philnik777]

@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]

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]

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]

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

[vhscampos]

@philnik777 Can you please have another look?

[vhscampos]

@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]

LGTM % nit.

[vhscampos]

@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]

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]

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

[vhscampos]

@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]

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.

[philnik777]

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.

[vhscampos]

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