[libcxx-commits] [PATCH] D91517: [libc++] [P0482] [C++20] Implement missing bits for codecvt and codecvt_byname.

Tue Nov 17 05:47:38 PST 2020

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

In D91517#2399385 <https://reviews.llvm.org/D91517#2399385>, @curdeius wrote:

> 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:
>
> 1. Use `CXX_STANDARD 20`, but it might be an overkill.

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

================
Comment at: libcxx/include/__locale:1253

+// TODO: Should _LIBCPP_DEPRECATED_IN_CXX20 be present only if char8_t is supported??
 template <>
----------------
I think it should be marked as deprecated whenever the standard says it is.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91517/new/

https://reviews.llvm.org/D91517