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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/print:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
Are we going to have a lot of stuff inside `<print>`? If so, it would probably make sense to introduce this utility under `__print/encode.h` instead.


================
Comment at: libcxx/include/print:44
+template <class _Tp>
+concept __utf16_code_unit = same_as<_Tp, char16_t> _LIBCPP_IF_WIDE_CHARACTERS(|| same_as<_Tp, wchar_t>);
+template <class _Tp>
----------------
Can we use `#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS` instead? I'd like to reduce usage of `_LIBCPP_IF_WIDE_CHARACTERS` and eventually remove it since it's a really unusual macro.


================
Comment at: libcxx/test/libcxx/input.output/iostream.format/print.fun/transcoding.pass.cpp:9-10
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// TODO FMT Fix this test using GCC, it currently times out.
+// UNSUPPORTED: gcc-12
+
----------------
Does this really time out on GCC 12? What's special about this test that makes it timeout?


================
Comment at: libcxx/test/libcxx/input.output/iostream.format/print.fun/transcoding.pass.cpp:18-22
+#include <algorithm>
+#include <cassert>
+#include <print>
+#include <string_view>
+#include <array>
----------------
Those don't seem sorted :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150031