[libcxx-commits] [PATCH] D91181: [24/N] [libcxx] Make generic_*string return paths with forward slashes on windows

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/include/filesystem:1252
   _VSTD::u32string generic_u32string() const { return generic_string<char32_t>(); }
-  __u8_string generic_u8string() const { return u8string(); }
+  __u8_string generic_u8string() const {
+    __u8_string __s = u8string();
----------------
curdeius wrote:
> Hmmm. Why can't it be implemented in terms of `generic_string`?
> I.e.: `{ return generic_string<typename __u8_string::value_type>(); }`
Because before C++20 (before `char8_t`), there's no typename that specifically triggers UTF8 output; if requesting output as `char`, it's produced in the current windows narrow code page, which in most cases isn't UTF8.


================
Comment at: libcxx/include/filesystem:1253-1254
+  __u8_string generic_u8string() const {
+    __u8_string __s = u8string();
+    _VSTD::replace(__s.begin(), __s.end(), '\\', '/');
+    return __s;
----------------
curdeius wrote:
> If I'm not mistaken it means that you traverse the string twice, once in `u8string()` to convert it using `_CVT`, and then in `replace()`.
> I don't think it's a blocker, but just saying.
> Same above in `generic_string<_ECharT, _Traits, _Allocator>`.
Yep, that's a correct observation. Suboptimal, but I wouldn't want to complicate things even further (especially at this point of the patchset) for that cause - at this point, getting it to work (at all) and correctly is the only priority...


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

https://reviews.llvm.org/D91181