This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Don't test CharFile (/dev/null) on windows
ClosedPublic

Authored by mstorsjo on Mar 7 2021, 1:57 AM.

Diff Detail

Event Timeline

mstorsjo requested review of this revision.Mar 7 2021, 1:57 AM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 7 2021, 1:57 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
mstorsjo updated this revision to Diff 328872.Mar 7 2021, 8:40 AM

Reupload to retry CI

Quuxplusone added a subscriber: Quuxplusone.
Quuxplusone added inline comments.
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.status/status.pass.cpp
116

You might adjust this comment to // file_type::block is tested elsewhere, just so that people don't assume it's related to the #ifndef on the next line.
(And ditto in the other test file, as well.)

Have you heard of nul special file on Windows?
https://gcc.gnu.org/legacy-ml/gcc-patches/2005-05/msg01793.html
I haven't tested it yet but maybe using it on Win instead of /dev/null might be a good idea?

Have you heard of nul special file on Windows?
https://gcc.gnu.org/legacy-ml/gcc-patches/2005-05/msg01793.html
I haven't tested it yet but maybe using it on Win instead of /dev/null might be a good idea?

Good point, but neither our implementation nor the MS STL one actually identify files as file_type::character, see https://github.com/microsoft/STL/blob/main/stl/inc/filesystem#L1882 which says:

character, // not used in this implementation; theoretically some special files like CON might qualify, but querying for this is extremely expensive and unlikely to be useful in practice
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.status/status.pass.cpp
116

Sure, I can do that.

curdeius accepted this revision.Mar 7 2021, 11:39 AM

LGTM. Could you put a similar comment (but concerning only Windows of course) near file_type::character declaration as well?

This revision is now accepted and ready to land.Mar 7 2021, 11:39 AM

LGTM. Could you put a similar comment (but concerning only Windows of course) near file_type::character declaration as well?

In the header? Sure, I can make a separate patch for that.