[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
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
CHANGES SINCE LAST ACTION
More information about the libcxx-commits