[libcxx-commits] [PATCH] D91133: [2/N] [libcxx] [test] Add a test for conversions between wchar_t, utf8, char16_t, char32_t and windows native narrow code pages

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Dec 1 15:35:59 PST 2020

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

In D91133#2426694 <https://reviews.llvm.org/D91133#2426694>, @mstorsjo wrote:

> In D91133#2426605 <https://reviews.llvm.org/D91133#2426605>, @amccarth wrote:
>> These test look pretty thorough.  I think these will be useful in finding portability problems.
>> Given the TODO in the description, I'll hold off on accepting this revision.
> @ldionne Where should this kind of new test be placed? It doesn't test strictly one specific method, but tests some specific aspects of both constructors and observers.

I think `libcxx/test/std/input.output/filesystems/class.path/path.member/path.charconv.pass.cpp`, as you did, is fine.

Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.charconv.pass.cpp:12
+// <filesystem>
Can you add a comment explaining what this test is about?

Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.charconv.pass.cpp:26
+using namespace fs;
I would much rather we qualify names instead. `fs::` is not much longer, but it adds clarity.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list