[24/N] [libcxx] Make generic_*string return paths with forward slashes on windows

[mstorsjo]

Summary

This matches what MS STL returns; in std::filesystem, forward slashes is considered a generic dir separator that is valid on all platforms.

[mstorsjo]

Updated to use _VSTD:: instead of std::.

[mstorsjo]

Can you @curdeius have a look at this one?

[curdeius]

libcxx/include/filesystem

1252

Hmmm. Why can't it be implemented in terms of generic_string?
I.e.: { return generic_string<typename __u8_string::value_type>(); }

1253–1254

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>.

[mstorsjo]

libcxx/include/filesystem

1252

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.

1253–1254

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...

[curdeius]

LGTM. I'd like just to see a TODO note or a ticket for this double traversal.

libcxx/include/filesystem

1252

Indeed. Thanks for explanation.

1253–1254

OK. Maybe adding a TODO note, or creating a bugzilla ticket for this, would at least keep trace of this. WDYT?

[mstorsjo]

In D91181#2462705, @curdeius wrote:

LGTM. I'd like just to see a TODO note or a ticket for this double traversal.

Thanks for reviewing!

libcxx/include/filesystem

1253–1254

I can add a note in a comment acknowledging this - I think that's the suitable amount of bureacracy for the issue.

[mstorsjo]

Rebased, added a comment pointing out suboptimal cases of iterating over the strings twice.

[mstorsjo]

Rebased, rerunning CI.

[mstorsjo]

Rebased

[mstorsjo]

Retrigger CI

libcxx/include/filesystem

   _LIBCPP_INLINE_VISIBILITY
   const value_type* c_str() const noexcept { return __pn_.c_str(); }
 
   _LIBCPP_INLINE_VISIBILITY operator string_type() const { return __pn_; }
 
 #if defined(_LIBCPP_WIN32API)
   _LIBCPP_INLINE_VISIBILITY _VSTD::wstring wstring() const { return __pn_; }
 
-  _VSTD::wstring generic_wstring() const { return __pn_; }
+  _VSTD::wstring generic_wstring() const {
+    _VSTD::wstring __s;
+    __s.resize(__pn_.size());
+    _VSTD::replace_copy(__pn_.begin(), __pn_.end(), __s.begin(), '\\', '/');
+    return __s;
+  }
 
 #if !defined(_LIBCPP_HAS_NO_LOCALIZATION)
   template <class _ECharT, class _Traits = char_traits<_ECharT>,
             class _Allocator = allocator<_ECharT> >
   basic_string<_ECharT, _Traits, _Allocator>
   string(const _Allocator& __a = _Allocator()) const {
     using _Str = basic_string<_ECharT, _Traits, _Allocator>;
     _Str __s(__a);

     return string<char32_t>();
   }
 
   // generic format observers
   template <class _ECharT, class _Traits = char_traits<_ECharT>,
             class _Allocator = allocator<_ECharT> >
   basic_string<_ECharT, _Traits, _Allocator>
   generic_string(const _Allocator& __a = _Allocator()) const {
-    return string<_ECharT, _Traits, _Allocator>(__a);
+    using _Str = basic_string<_ECharT, _Traits, _Allocator>;
+    _Str __s = string<_ECharT, _Traits, _Allocator>(__a);
+    // Note: This (and generic_u8string below) is slightly suboptimal as
+    // it iterates twice over the string; once to convert it to the right
+    // character type, and once to replace path delimiters.
+    _VSTD::replace(__s.begin(), __s.end(),
+                 static_cast<_ECharT>('\\'), static_cast<_ECharT>('/'));
+    return __s;
   }
 
   _VSTD::string generic_string() const { return generic_string<char>(); }
   _VSTD::u16string generic_u16string() const { return generic_string<char16_t>(); }
   _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();
+    _VSTD::replace(__s.begin(), __s.end(), '\\', '/');
+    return __s;
+  }
 #endif /* !_LIBCPP_HAS_NO_LOCALIZATION */
 #else /* _LIBCPP_WIN32API */
 
   _LIBCPP_INLINE_VISIBILITY _VSTD::string string() const { return __pn_; }
 #ifndef _LIBCPP_NO_HAS_CHAR8_T
   _LIBCPP_INLINE_VISIBILITY _VSTD::u8string u8string() const { return _VSTD::u8string(__pn_.begin(), __pn_.end()); }
 #else
   _LIBCPP_INLINE_VISIBILITY _VSTD::string u8string() const { return __pn_; }

libcxx/test/std/input.output/filesystems/class.path/path.member/path.generic.obs/named_overloads.pass.cpp

 #include <cassert>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 #include "count_new.h"
 #include "min_allocator.h"
 #include "filesystem_test_helper.h"
 
-MultiStringType longString = MKSTR("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ/123456789/abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ");
+MultiStringType input = MKSTR("c:\\foo\\bar");
+#ifdef _WIN32
+// On windows, the generic_* accessors return a path with forward slashes
+MultiStringType ref = MKSTR("c:/foo/bar");
+#else
+// On posix, the input string is returned as-is
+MultiStringType ref = MKSTR("c:\\foo\\bar");
+#endif
 
 int main(int, char**)
 {
   using namespace fs;
-  auto const& MS = longString;
-  const char* value = longString;
+  auto const& MS = ref;
+  const char* value = input;
   const path p(value);
   {
     std::string s = p.generic_string();
-    assert(s == value);
+    assert(s == (const char*)MS);
   }
   {
 #if TEST_STD_VER > 17 && defined(__cpp_char8_t)
     ASSERT_SAME_TYPE(decltype(p.generic_u8string()), std::u8string);
     std::u8string s = p.generic_u8string();
     assert(s == (const char8_t*)MS);
 #else
     ASSERT_SAME_TYPE(decltype(p.generic_u8string()), std::string);