[libcxx-commits] [PATCH] D90222: [1/N] [libcxx] Implement c++2a char8_t input/output of std::filesystem::path

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Dec 2 03:21:04 PST 2020

mstorsjo added inline comments.

Comment at: libcxx/include/filesystem:1236
+      // Proposed in P1423
+      is_same<typename __is_pathable<_Source>::__char_type, char8_t>::value ||
curdeius wrote:
> Unnecessary comment? You should maybe mark [[ https://wg21.link/P1483 | P1483]] as In Progress.

Comment at: libcxx/include/filesystem:1241
       "u8path(Source const&) requires Source have a character type of type "
   return path(__s);
curdeius wrote:
> You should update the message too.
Oh, good catch, thanks!

Comment at: libcxx/test/support/filesystem_test_helper.h:433
-#define MKSTR(Str) {Str, TEST_CONCAT(L, Str), TEST_CONCAT(u, Str), TEST_CONCAT(U, Str)}
+#define MKSTR(Str) {Str, TEST_CONCAT(L, Str), TEST_CONCAT(u8, Str), TEST_CONCAT(u, Str), TEST_CONCAT(U, Str)}
curdeius wrote:
> Shouldn't you guard it on `__cpp_char8_t` instead of using `dummy` below?
Yeah, it's possible, with something like this:
#if TEST_STD_VER > 17 && defined(__cpp_char8_t)
#define CHAR8_ONLY(x) x,
#define CHAR8_ONLY
#define MKSTR(Str) {... CHAR8_ONLY(TEST_CONCAT(u8, Str)) TEST_CONCAT(u, Str), ...
The trailing comma is a bit tricky; it has to be included as part of the expansion of `CHAR8_ONLY()`, but this way it seems like it would work.

If you prefer that form, I can change it that way.



More information about the libcxx-commits mailing list