[libcxx-commits] [PATCH] D130946: [libc++][cuchar] Declare std::c8rtomb and std::mbrtoc8 in <cuchar> if available.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/cuchar:54-57
+#  if !defined(_LIBCPP_HAS_NO_C8RTOMB_MBRTOC8)
+using ::mbrtoc8 _LIBCPP_USING_IF_EXISTS;
+using ::c8rtomb _LIBCPP_USING_IF_EXISTS;
+#  endif
----------------
tahonermann wrote:
> philnik wrote:
> > tahonermann wrote:
> > > philnik wrote:
> > > > I think this should also be guarded with `_LIBCPP_STD_VER >= 20`.
> > > That is an option, but leads to scenarios where the declarations are available in the global namespace via `uchar.h` but not in the `std` namespace via `<cuchar>`. I thought it made more sense to keep these in sync.
> > > 
> > > At present, libc++ omits the other `char8_t` related declarations (e.g., `u8string`) in C++20 mode when `char8_t` support is disabled (of course) but likewise omits them in pre-C++20 modes when `char8_t` support is enabled. What I would prefer to do (in a separate patch) is to match the libstdcxx behavior of declaring all of the `char8_t` related declarations based on `char8_t` enablement rather than on the C++ standard version. This would give users more flexibility for evaluating  and addressing`char8_t` related migration concerns before migrating to C++20.
> > Hmm. Having the declarations in `uchar.h` but not in `cuchar` would indeed be quite weird. OTOH having the declarations pre-C++20 would be an extension in the C library, which we might not want to support. I also suspect that you will find quite strong opposition to enabling `char8_t` pre-C++20. We are quite anti-extensions and have even removed a number of them over the last years. @ldionne Do you have any thoughts here?
> @ldionne, could you please share your thoughts on whether the `std::c8rtomb()` and `std::mbrtoc8()` declarations should be restricted to C++20 and later or provided consistent with the corresponding C declarations in the global namespace?
My preference would be to be strict (and hence not include them before C++20). That being said, it seems like we have a strong precedent everywhere in our C compatibility headers to include functions inside `std::` whenever they are provided by the underlying C library. So I think the current approach is OK.

Is there a reason why we even need `_LIBCPP_HAS_NO_C8RTOMB_MBRTOC8` at all? We have `_LIBCPP_USING_IF_EXIST` to eliminate the need for exactly this sort of stuff. Of course GCC doesn't implement it, however it would still work when compiling using Clang on a platform that uses glibc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130946