[libcxx-commits] [PATCH] D150031: [libc++][format] Adds a UTF transcoder.

Sat Jun 24 04:26:35 PDT 2023

Mordante marked 3 inline comments as done.
Mordante added a comment.

In D150031#4445220 <https://reviews.llvm.org/D150031#4445220>, @tahonermann wrote:

> Sorry for the delay, again. I think this is looking good. I noticed one issue that I think needs to be addressed and otherwise just want to confirm that lack of support for encoding UTF-8 is intentional for now.

No problem, thanks for the review!

================
Comment at: libcxx/include/print:39-43
+// The names of these concepts are modelled after P2728R0, but the
+// implementation is not. char16_t may contain 32-bits so depending on the
+// number of bits is an issue.
+template <class _Tp>
+concept __utf8_code_unit = same_as<_Tp, char8_t> || same_as<_Tp, char>;
----------------
tahonermann wrote:
> As far as I can tell, the `__utf8_code_unit` concept is not used, nor is there a UTF-8 overload of `__encode()`. That is ok if intentional, I'm just mentioning it in case it isn't.
> 
> The use of `char8_t` should be predicated on `_LIBCPP_HAS_NO_CHAR8_T` not being defined since `char8_t` support can be disabled with `-fno-char8_t`.
Nice catch. It's indeed not used at the moment. I'll remove it for now. When Zach's paper is approved I will add the concepts based on the wording of the paper.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150031