[libcxx-commits] [PATCH] D150015: [libc++] Consistently enable __CORRECT_ISO_CPP_WCHAR_H_PROTO in mbstate.

Jordan Rupprecht via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed May 10 16:01:30 PDT 2023


rupprecht added inline comments.


================
Comment at: libcxx/include/__mbstate_t.h:30-33
+// We define this here to support older versions of glibc <wchar.h> that do
+// not define this for clang. This is also set in libc++'s <wchar.h> header,
+// and we need to do so here too to avoid a different function signature given
+// a different include order.
----------------
iana wrote:
> Can you also mention why this isn't done only in the `#elif !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS) && __has_include_next(<wchar.h>)` branch? It looks like that macro should only affect <wchar.h>, but I think you were saying that it's also needed for <bits/types/mbstate_t.h> I think?
(just noticed this comment too, right after submitting)

I don't think it matters much one way or the other -- I'm fairly certain that `_LIBCPP_HAS_NO_WIDE_CHARACTERS` only affects wchar.h, and none of the other include/include_next entries here will transitively include wchar.h, so it would be safe to put it within the if/elif chain. It's slightly more readable to have it outside of the block IMHO, and I don't think there's any harm in having it there -- it should be safe to even have it in libc++'s `__config` header.

But anyway, proposed D150322 to do that. Happy to land that if you'd prefer that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150015



More information about the libcxx-commits mailing list