[libcxx-commits] [PATCH] D91135: [3/N] [libcxx] Make filesystem::path::value_type wchar_t on windows

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/filesystem:795
 
+#if defined(_LIBCPP_WIN32API)
+template <class _ECharT>
----------------
This code is valid even outside of Windows, right? If so, I'd rather we keep it in the header unconditionally. I don't mind if it's unused outside of Windows -- I think it's better to try to remove these `#ifdef`s as much as possible.


================
Comment at: libcxx/include/filesystem:1110-1122
+  _LIBCPP_INLINE_VISIBILITY _VSTD::string string() const {
+    return string<char>();
+  }
+  _LIBCPP_INLINE_VISIBILITY __u8_string u8string() const {
+    return string<__u8_string::value_type>();
+  }
+
----------------
Would it be possible to share these definitions between Windows and non-Windows to avoid code duplication?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91135