[libcxx-commits] [PATCH] D91137: [5/N] [libcxx] Convert paths to/from the right narrow code page for narrow strings on windows

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Dec 14 15:28:17 PST 2020


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

LGTM except nits!



================
Comment at: libcxx/include/filesystem:1467
+      "u8path(Iter, Iter) requires Iter have a value_type of type 'char'"
+#ifndef _LIBCPP_NO_HAS_CHAR8_T
+      " or 'char8_t'"
----------------
I don't think `#ifdefing` the `static_assert` message adds anything. It just makes things harder to read IMO. I know this was done the same way elsewhere, but I think we should actually remove the ifdefs in those other places instead.


================
Comment at: libcxx/src/filesystem/operations.cpp:21
+#if defined(_LIBCPP_WIN32API)
+#define WIN32_LEAN_AND_MEAN
+#define NOMINMAX
----------------
Please indent includes like:

```
#if defined(_LIBCPP_WIN32API`
#   define WIN32_LEAN_AND_MEAN
#   include <stuff>
#else
#   include <more stuff>
#endif
```


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

https://reviews.llvm.org/D91137



More information about the libcxx-commits mailing list