[libcxx-commits] [PATCH] D90222: [1/N] [libcxx] Implement c++2a char8_t input/output of std::filesystem::path

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/filesystem:1006
+#ifndef _LIBCPP_NO_HAS_CHAR8_T
+  _LIBCPP_INLINE_VISIBILITY _VSTD::u8string u8string() const { return _VSTD::u8string(__pn_.begin(), __pn_.end()); }
+#else
----------------
mstorsjo wrote:
> ldionne wrote:
> > That's an ABI break in C++20. Do we think it's reasonable?
> > 
> > I think we at least need to add it to the release notes.
> This method is marked with the ` _LIBCPP_INLINE_VISIBILITY` attribute, which afaik is supposed to exclude it from the ABI altogether - exactly for this reason?
> 
> The fact that it's a source compat break, and whether to worry about it or not, is acknowledged and discussed in P1423.
I was trying to come up with examples where a library was already compiled with an inlined version of `path::u8string` that returns a `std::string`, and then link against another binary where `path::u8string()` returns a `std::u8string` instead. But I couldn't find something that wasn't very tortuous, so I think this is fine from the ABI perspective.

Can we please add a release note because this is a source break?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90222