[libcxx-commits] [PATCH] D101731: [libcxx] [test] Fix fs.op.last_write_time for Windows

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp:40
+#endif
 #include <sys/stat.h>
 
----------------
Mordante wrote:
> Is the stat header required on Windows? If not would it be possible to include this header conditional and remove the `compat` namespace?
Ok, that seems to work. I'd generally consider that (trying to avoid that a specific gets included) a bit more brittle, but as long as we have CI, I guess it's fine.

The actual implementation uses a namespace like this though, but as long as we can do without one here, the test remains simpler.


================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp:92
+}
+} // namespace compat
+#else
----------------
Mordante wrote:
> Please move this line outside the `#if` section
If we'd keep the namespace, I wouldn't want to move the ending brace of the namespace outside of the ifdef as long as the starting brace is within the ifdef, that'd be awfully imbalanced. But we can get rid of it altogether.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101731