[libcxx-commits] [PATCH] D148542: [libc++] cuchar redeclares ::mbstate_t when it's in its own clang module

[Ian Anderson via Phabricator via libcxx-commits]

iana updated this revision to Diff 517674.
iana marked 4 inline comments as done.
iana added a comment.

Review changes, fix failed test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148542

Files:
  libcxx/include/__mbstate_t.h
  libcxx/include/uchar.h
  libcxx/include/wchar.h
  libcxx/test/std/strings/string.view/string.view.comparison/comparison.verify.cpp


Index: libcxx/test/std/strings/string.view/string.view.comparison/comparison.verify.cpp
===================================================================
--- libcxx/test/std/strings/string.view/string.view.comparison/comparison.verify.cpp
+++ libcxx/test/std/strings/string.view/string.view.comparison/comparison.verify.cpp
@@ -17,6 +17,7 @@
 // [string.view]/4
 // Mandates: R denotes a comparison category type ([cmp.categories]).
 
+#include <cwchar>
 #include <string_view>
 
 #include "test_macros.h"
Index: libcxx/include/wchar.h
===================================================================
--- libcxx/include/wchar.h
+++ libcxx/include/wchar.h
@@ -122,6 +122,8 @@
 
 #  if __has_include_next(<wchar.h>)
 #    include_next <wchar.h>
+#  else
+#    include <__mbstate_t.h> // make sure we have mbstate_t regardless of the existence of <wchar.h>
 #  endif
 
 // Determine whether we have const-correct overloads for wcschr and friends.
Index: libcxx/include/uchar.h
===================================================================
--- libcxx/include/uchar.h
+++ libcxx/include/uchar.h
@@ -42,10 +42,12 @@
 
 // Some platforms don't implement <uchar.h> and we don't want to give a hard
 // error on those platforms. When the platform doesn't provide <uchar.h>, at
-// least include <stddef.h> so we get the declaration for size_t.
+// least include <stddef.h> so we get the declaration for size_t, and try to
+// get the declaration of mbstate_t too.
 #if __has_include_next(<uchar.h>)
 # include_next <uchar.h>
 #else
+# include <__mbstate_t.h>
 # include <stddef.h>
 #endif
 
Index: libcxx/include/__mbstate_t.h
===================================================================
--- libcxx/include/__mbstate_t.h
+++ libcxx/include/__mbstate_t.h
@@ -16,29 +16,27 @@
 #  pragma GCC system_header
 #endif
 
-// TODO(ldionne):
-// The goal of this header is to provide mbstate_t without having to pull in
-// <wchar.h> or <uchar.h>. This is necessary because we need that type even
-// when we don't have (or try to provide) support for wchar_t, because several
-// types like std::fpos are defined in terms of mbstate_t.
+// The goal of this header is to provide mbstate_t without requiring all of
+// <uchar.h> or <wchar.h>. It's also used by the libc++ versions of <uchar.h>
+// and <wchar.h> to get mbstate_t when the C library doesn't provide <uchar.h>
+// or <wchar.h>, hence the #include_next of those headers instead of #include.
+// (e.g. if <wchar.h> isn't present in the C library, the libc++ <wchar.h>
+// will include this header. This header needs to not turn around and cyclically
+// include <wchar.h>, but fall through to <uchar.h>.)
 //
-// This is a gruesome hack, but I don't know how to make it cleaner for
-// the time being.
+// This does not define std::mbstate_t -- this only brings in the declaration
+// in the global namespace.
 
-#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
-#   include <wchar.h> // for mbstate_t
-#elif __has_include(<bits/types/mbstate_t.h>)
+#if __has_include(<bits/types/mbstate_t.h>)
 #   include <bits/types/mbstate_t.h> // works on most Unixes
 #elif __has_include(<sys/_types/_mbstate_t.h>)
 #   include <sys/_types/_mbstate_t.h> // works on Darwin
+#elif !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS) && __has_include_next(<wchar.h>)
+#   include_next <wchar.h> // fall back to the C standard provider of mbstate_t
+#elif __has_include_next(<uchar.h>)
+#   include_next <uchar.h> // <uchar.h> is also required to make mbstate_t visible
 #else
-#   error "The library was configured without support for wide-characters, but we don't know how to get the definition of mbstate_t without <wchar.h> on your platform."
+#   error "We don't know how to get the definition of mbstate_t without <wchar.h> on your platform."
 #endif
 
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-using ::mbstate_t _LIBCPP_USING_IF_EXISTS;
-
-_LIBCPP_END_NAMESPACE_STD
-
 #endif // _LIBCPP___MBSTATE_T_H


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148542.517674.patch
Type: text/x-patch
Size: 3923 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230427/3300f777/attachment-0001.bin>