Add codecvt*<char16_t, char8_t> and codecvt*<char32_t, char8_t>.
Deprecate codecvt<char(16|32)_t, char>.
Enable disabled tests.
Update _LIBCPP_STD_VER to use 20 for C++20. Add _LIBCPP_DEPRECATED_IN_CXX20 macro.
Details
- Reviewers
ldionne mclow.lists EricWF - Group Reviewers
Restricted Project Restricted Project - Commits
- rGa984dcaf7c21: [libc++] [P0482] [C++20] Implement missing bits for codecvt and codecvt_byname.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Buildkite shows build failures with undefined references.
That's because libc++ (.so, .a) is compiled using -std=c++17 and I used #ifndef _LIBCPP_NO_HAS_CHAR8_T to guard the char8_t-related things.
_LIBCPP_NO_HAS_CHAR8_T is defined as:
#if _LIBCPP_STD_VER <= 17 || !defined(__cpp_char8_t) #define _LIBCPP_NO_HAS_CHAR8_T #endif
So, in the failing cases, src/ compiles with C++17 and so without char8_t, but include/ and test/ compile with C++20.
I haven't seen these failures locally before because I was compiling everything in C++20 mode (CXX_STANDARD 20 in libcxx/CMakeLists.txt).
I see 2 possible solutions:
- Use CXX_STANDARD 20, but it might be an overkill.
- Change guards in locale.cpp to just #ifdef __cpp_char8_t instead of #ifndef _LIBCPP_NO_HAS_CHAR8_T.
WDYT?
I think that is the right way to go. I think it makes sense to build the library with the latest standard available.
Furthermore, once we have moved people over to the runtimes build and we know that folks are building the library with a recent compiler, we should even be able to assume that the compiler provides char8_t.
libcxx/include/__locale | ||
---|---|---|
1252 | I think it should be marked as deprecated whenever the standard says it is. |
- Use C++20 for libc++. Should we require standard? Or should we change to CXX_STANDARD_REQUIRED NO?
- Remove TODOs.
I'll create a new revision with only the bump of the C++ standard version, because that provokes some failures it seems.
Does this pass check-cxx-abilist? It needs to. I think you may need to update the ABI lists.
I would suggest you wait for me to submit D92194 and then rebase on top of it. That'll provide you with additional coverage for the ABI lists. My goal with D92194 was specifically to make sure the CI would catch issues with ABI lists in changes like yours :-). Requesting changes so it shows up right in the queue.
Can you confirm that the issues you were seeing on older macOSes are that the tests are now exercising char8_t codecvt, so you're seeing runtime errors since those are not installed in older dylibs? I'm trying to make sure we understand the issue and don't silent something important.
Yes, all the 33 test failures in https://buildkite.com/llvm-project/libcxx-ci/builds/578#4979bae1-6f90-4d3a-a17e-9b9981aaedbf
were due to missing symbols, e.g.:
dyld: Symbol not found: __ZNKSt3__17codecvtIDiDu11__mbstate_tE10do_unshiftERS1_PDuS4_RS4_ Referenced from: /private/tmp/buildkite-agent/builds/libcxx-mbp-local-1/llvm-project/libcxx-ci/build/x86_64-apple-system-backdeployment-10.9/projects/libcxx/test/std/localization/locale.categories/category.ctype/locale.codecvt.byname/Output/ctor_char32_t_char8_t.pass.cpp.dir/t.tmp.exe Expected in: /private/tmp/buildkite-agent/builds/libcxx-mbp-local-1/llvm-project/libcxx-ci/build/x86_64-apple-system-backdeployment-10.9/macos-roots/macOS/libc++/10.9/libc++.1.dylib
- [abi] Update ABI list for linux (and changelog).
- Rebase to include changes from D92194
- Fix linux ABI list.
- Re-generate ABI list (after a failed check) and get them as artifacts, so as to ease the development for contributors not having access to necessary platform.
@ldionne, I've modified the run-buildbot script (and added *.abilist to artifact_paths in buildkite-pipeline.yml) so as to run generate-cxx-abilist if check-cxx-abilist failed.
The build will fail anyway but the regenerated ABI lists will be present among the artifacts.
That's for people, like me, that don't have access to one of the platforms on which we check ABI lists.
If you prefer, I'll spin off another patch to keep things apart.
Nice! I love how we're building on top of the CI infrastructure to make our life simpler.
If you commit this before me, I'll rebase D92212 on top.
I think it should be marked as deprecated whenever the standard says it is.