public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
@ 2021-06-07  2:16 Tom Honermann
  2021-06-23 17:27 ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Honermann @ 2021-06-07  2:16 UTC (permalink / raw)
  To: libstdc++

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

This patch completes implementation of the C++20 proposal P0482R6 [1] by 
adding declarations of std::c8rtomb() and std::mbrtoc8() if provided by 
the C library.

Autoconf changes determine if the C library declares c8rtomb and mbrtoc8 
at global scope when uchar.h is included and compiled with -fchar8_t; 
_GLIBCXX_USE_UCHAR_CHAR8_T is defined if so.  The <cuchar> header 
re-declares these functions in the std namespace only if available and 
the C++20 __cpp_char8_t feature test macro is defined.

Patches to glibc to implement c8rtomb and mbrtoc8 have been submitted [2].

A new test is provided.  The test passes trivially if the C library does 
not provide these functions.  Otherwise it ensures that the functions 
are declared when <cuchar> is included and -fchar8_t support is enabled.

Tested on Linux x86_64.

libstdc++-v3/ChangeLog:

2021-05-31  Tom Honermann  <tom@honermann.net>

         * acinclude.m4 (_GLIBCXX_USE_UCHAR_CHAR8_T) Define if uchar.h
           provides c8rtomb() and mbrtoc8().
         * config.h.in: Re-generate.
         * configure: Re-generate.
         * include/c_compatibility/uchar.h: Declare ::c8rtomb and ::mbrtoc8.
         * include/c_global/cuchar: Declare std::c8rtomb and std::mbrtoc8.
         * include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8.
         * testsuite/21_strings/headers/cuchar/functions_std.cc: New test.

Tom.

[1]: WG21 P0482R6
      "char8_t: A type for UTF-8 characters and strings (Revision 6)"
      https://wg21.link/p0482r6

[2]: C++20 P0482R6 and C2X N2653: support for char8_t, mbrtoc8(), and 
c8rtomb().
      [Patch 0]: 
https://sourceware.org/pipermail/libc-alpha/2021-June/127230.html
      [Patch 1]: 
https://sourceware.org/pipermail/libc-alpha/2021-June/127231.html
      [Patch 2]: 
https://sourceware.org/pipermail/libc-alpha/2021-June/127232.html
      [Patch 3]: 
https://sourceware.org/pipermail/libc-alpha/2021-June/127233.html




[-- Attachment #2: p0482r6-n2653.patch --]
[-- Type: text/x-patch, Size: 6687 bytes --]

commit bffd3c8d94b7978d296df4b0e2fa77cc5887efe6
Author: Tom Honermann <tom@honermann.net>
Date:   Sat Feb 13 07:53:34 2021 -0500

    P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library.
    
    This change completes implementation of the C++20 proposal P0482R6 by
    adding declarations of std::c8rtomb() and std::mbrtoc8() if provided
    by the C library.
    
    Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
    at global scope when uchar.h is included and compiled with -fchar8_t;
    _GLIBCXX_USE_UCHAR_CHAR8_T is defined if so.  The <cuchar> header
    re-declares these functions in the std namespace only if available and
    the C++20 __cpp_char8_t feature test macro is defined.
    
    A new test is provided.  The test passes trivially if the C library does
    not provide these functions.  Otherwise it ensures that the functions
    are declared when <cuchar> is included and -fchar8_t support is enabled.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 90ecc4a87a2..355c21a03b5 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2063,6 +2063,28 @@ AC_DEFUN([GLIBCXX_CHECK_UCHAR_H], [
 	      namespace std in <cuchar>.])
   fi
 
+  CXXFLAGS="$CXXFLAGS -fchar8_t"
+  if test x"$ac_has_uchar_h" = x"yes"; then
+    AC_MSG_CHECKING([for char8_t support in <uchar.h>])
+    AC_TRY_COMPILE([#include <uchar.h>
+		    char8_t c8;
+		    namespace test
+		    {
+		      using ::c8rtomb;
+		      using ::mbrtoc8;
+		    }
+		   ],
+		   [], [ac_uchar_char8_t=yes], [ac_uchar_char8_t=no])
+  else
+    ac_uchar_char8_t=no
+  fi
+  AC_MSG_RESULT($ac_uchar_char8_t)
+  if test x"$ac_uchar_char8_t" = x"yes"; then
+    AC_DEFINE(_GLIBCXX_USE_UCHAR_CHAR8_T, 1,
+	      [Define if char8_t functions in <uchar.h> should be imported into
+	      namespace std in <cuchar>.])
+  fi
+
   CXXFLAGS="$ac_save_CXXFLAGS"
   AC_LANG_RESTORE
 ])
diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
index e545488386a..745de9687a4 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -991,6 +991,10 @@
 /* Define if obsolescent tmpnam is available in <stdio.h>. */
 #undef _GLIBCXX_USE_TMPNAM
 
+/* Define if char8_t functions in <uchar.h> should be imported into namespace
+   std in <cuchar>. */
+#undef _GLIBCXX_USE_UCHAR_CHAR8_T
+
 /* Define if utime is available in <utime.h>. */
 #undef _GLIBCXX_USE_UTIME
 
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 6f08b65c8ba..8311419a127 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -19115,6 +19115,45 @@ $as_echo "#define _GLIBCXX_USE_C11_UCHAR_CXX11 1" >>confdefs.h
 
   fi
 
+  CXXFLAGS="$CXXFLAGS -fchar8_t"
+  if test x"$ac_has_uchar_h" = x"yes"; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for char8_t support in <uchar.h>" >&5
+$as_echo_n "checking for char8_t support in <uchar.h>... " >&6; }
+    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <uchar.h>
+		    char8_t c8;
+		    namespace test
+		    {
+		      using ::c8rtomb;
+		      using ::mbrtoc8;
+		    }
+
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  ac_uchar_char8_t=yes
+else
+  ac_uchar_char8_t=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+  else
+    ac_uchar_char8_t=no
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_uchar_char8_t" >&5
+$as_echo "$ac_uchar_char8_t" >&6; }
+  if test x"$ac_uchar_char8_t" = x"yes"; then
+
+$as_echo "#define _GLIBCXX_USE_UCHAR_CHAR8_T 1" >>confdefs.h
+
+  fi
+
   CXXFLAGS="$ac_save_CXXFLAGS"
   ac_ext=c
 ac_cpp='$CPP $CPPFLAGS'
diff --git a/libstdc++-v3/include/c_compatibility/uchar.h b/libstdc++-v3/include/c_compatibility/uchar.h
index 1fe8a22f78a..21812bb44b4 100644
--- a/libstdc++-v3/include/c_compatibility/uchar.h
+++ b/libstdc++-v3/include/c_compatibility/uchar.h
@@ -33,6 +33,11 @@
 
 #ifdef _GLIBCXX_NAMESPACE_C
 
+#if _GLIBCXX_USE_CHAR8_T && _GLIBCXX_USE_UCHAR_CHAR8_T
+using std::mbrtoc8;
+using std::c8rtomb;
+#endif
+
 #if _GLIBCXX_USE_C11_UCHAR_CXX11
 using std::mbrtoc16;
 using std::c16rtomb;
diff --git a/libstdc++-v3/include/c_global/cuchar b/libstdc++-v3/include/c_global/cuchar
index 57047d74218..aab62e57bc8 100644
--- a/libstdc++-v3/include/c_global/cuchar
+++ b/libstdc++-v3/include/c_global/cuchar
@@ -48,10 +48,34 @@
 #include <bits/c++config.h>
 #include <cwchar>
 
-#if _GLIBCXX_USE_C11_UCHAR_CXX11
+#if _GLIBCXX_USE_C11_UCHAR_CXX11 || _GLIBCXX_USE_CHAR8_T
 
 #include <uchar.h>
 
+#endif
+
+
+// Support for mbrtoc8 and c8rtomb is conditioned on support by the C library.
+#if _GLIBCXX_USE_CHAR8_T && _GLIBCXX_USE_UCHAR_CHAR8_T
+
+#undef mbrtoc8
+#undef c8rtomb
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  using ::mbrtoc8;
+  using ::c8rtomb;
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
+
+#endif // _GLIBCXX_USE_CHAR8_T && _GLIBCXX_USE_UCHAR_CHAR8_T
+
+
+#if _GLIBCXX_USE_C11_UCHAR_CXX11
+
 // Get rid of those macros defined in <uchar.h> in lieu of real functions.
 #undef mbrtoc16
 #undef c16rtomb
diff --git a/libstdc++-v3/include/c_std/cuchar b/libstdc++-v3/include/c_std/cuchar
index 7ce413e1bb0..13d8b773c49 100644
--- a/libstdc++-v3/include/c_std/cuchar
+++ b/libstdc++-v3/include/c_std/cuchar
@@ -48,10 +48,35 @@
 #include <bits/c++config.h>
 #include <cwchar>
 
-#if _GLIBCXX_USE_C11_UCHAR_CXX11
+#if _GLIBCXX_USE_C11_UCHAR_CXX11 || _GLIBCXX_USE_CHAR8_T
 
 #include <uchar.h>
 
+#endif
+
+
+// Support for mbrtoc8 and c8rtomb is conditioned on support by the C library.
+#if _GLIBCXX_USE_CHAR8_T && _GLIBCXX_USE_UCHAR_CHAR8_T
+
+// Get rid of those macros defined in <uchar.h> in lieu of real functions.
+#undef mbrtoc8
+#undef c8rtomb
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  using ::mbrtoc8;
+  using ::c8rtomb;
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
+
+#endif // _GLIBCXX_USE_CHAR8_T && _GLIBCXX_USE_UCHAR_CHAR8_T
+
+
+#if _GLIBCXX_USE_C11_UCHAR_CXX11
+
 // Get rid of those macros defined in <uchar.h> in lieu of real functions.
 #undef mbrtoc16
 #undef c16rtomb
diff --git a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std.cc b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std.cc
new file mode 100644
index 00000000000..192b5a0481d
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std.cc
@@ -0,0 +1,12 @@
+// { dg-do compile }
+// { dg-options "-fchar8_t" }
+
+#include <cuchar>
+
+namespace gnu
+{
+#if _GLIBCXX_USE_UCHAR_CHAR8_T
+  using std::mbrtoc8;
+  using std::c8rtomb;
+#endif
+}



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2021-06-07  2:16 [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library Tom Honermann
@ 2021-06-23 17:27 ` Jonathan Wakely
  2021-06-24 19:35   ` Tom Honermann
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2021-06-23 17:27 UTC (permalink / raw)
  To: Tom Honermann; +Cc: libstdc++

N.B. please CC gcc-patches as well when sending patches to this list.

On Mon, 7 Jun 2021 at 03:16, Tom Honermann via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> This patch completes implementation of the C++20 proposal P0482R6 [1] by
> adding declarations of std::c8rtomb() and std::mbrtoc8() if provided by
> the C library.
>
> Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
> at global scope when uchar.h is included and compiled with -fchar8_t;
> _GLIBCXX_USE_UCHAR_CHAR8_T is defined if so.  The <cuchar> header
> re-declares these functions in the std namespace only if available and
> the C++20 __cpp_char8_t feature test macro is defined.


Thanks, Tom. I'm surprised that there's no __cplusplus dependency
here. Some C libraries aren't going to declare these new functions
unconditionally, only for C2x and C++20 modes. Your new configure
check tests for them with -std=c++11 -fchar8_t (and the implicit
-D_GNU_SOURCE on GNU/Linux) which doesn't guarantee they'll be
available.

This isn't a problem for your glibc patches, because you do:

+#if defined _CHAR8_T_SOURCE || defined __cpp_char8_t
+# define __GLIBC_USE_CHAR8_T   1

But consider a hypothetical libc that only define the new functions
for C2x/C++20, ignoring the __cpp_char8_t set by -fchar8_t. For such a
libc the configure test using -std=c++11 -fchar8_t will fail, and
libstdc++ won't declare the functions in namespace std even though
they are in libc.

Shouldn't the configure test use -std=c++20 instead to check that
they're available for C++20 mode, and then in <cuchar> guard the using
declarations for ::c8rtomb and ::mbrtoc8 with #if __cplusplus >=
202002L ?

That would mean they're not declared in <cuchar> pre-C++20 when using
-fchar8_t, but that seems a lesser problem than having them not
declared in C++20 when they are actually present in libc. Maybe to
solve that we need two configure macros (or one macro with multiple
values), one that says the new functions are available for C++20, and
another that says they are also available pre-C++20 if __cpp_char8_t
is defined. Then <cuchar> can do something like:

#if (__cplusplus >= 202002 && _GLIBCXX_USE_UCHAR_CXX20) \
  || (__cpp_char8_t && _GLIBCXX_USE_UCHAR_CHAR8_T)

Messy. Hmm.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2021-06-23 17:27 ` Jonathan Wakely
@ 2021-06-24 19:35   ` Tom Honermann
  2021-06-24 19:39     ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Honermann @ 2021-06-24 19:35 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++

On 6/23/21 1:27 PM, Jonathan Wakely wrote:
> N.B. please CC gcc-patches as well when sending patches to this list.
Ah, yes, I think you've reminded me of that before.
>
> On Mon, 7 Jun 2021 at 03:16, Tom Honermann via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> This patch completes implementation of the C++20 proposal P0482R6 [1] by
>> adding declarations of std::c8rtomb() and std::mbrtoc8() if provided by
>> the C library.
>>
>> Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
>> at global scope when uchar.h is included and compiled with -fchar8_t;
>> _GLIBCXX_USE_UCHAR_CHAR8_T is defined if so.  The <cuchar> header
>> re-declares these functions in the std namespace only if available and
>> the C++20 __cpp_char8_t feature test macro is defined.
>
> Thanks, Tom. I'm surprised that there's no __cplusplus dependency
> here. Some C libraries aren't going to declare these new functions
> unconditionally, only for C2x and C++20 modes. Your new configure
> check tests for them with -std=c++11 -fchar8_t (and the implicit
> -D_GNU_SOURCE on GNU/Linux) which doesn't guarantee they'll be
> available.
>
> This isn't a problem for your glibc patches, because you do:
>
> +#if defined _CHAR8_T_SOURCE || defined __cpp_char8_t
> +# define __GLIBC_USE_CHAR8_T   1
>
> But consider a hypothetical libc that only define the new functions
> for C2x/C++20, ignoring the __cpp_char8_t set by -fchar8_t. For such a
> libc the configure test using -std=c++11 -fchar8_t will fail, and
> libstdc++ won't declare the functions in namespace std even though
> they are in libc.
>
> Shouldn't the configure test use -std=c++20 instead to check that
> they're available for C++20 mode, and then in <cuchar> guard the using
> declarations for ::c8rtomb and ::mbrtoc8 with #if __cplusplus >=
> 202002L ?
>
> That would mean they're not declared in <cuchar> pre-C++20 when using
> -fchar8_t, but that seems a lesser problem than having them not
> declared in C++20 when they are actually present in libc. Maybe to
> solve that we need two configure macros (or one macro with multiple
> values), one that says the new functions are available for C++20, and
> another that says they are also available pre-C++20 if __cpp_char8_t
> is defined. Then <cuchar> can do something like:
>
> #if (__cplusplus >= 202002 && _GLIBCXX_USE_UCHAR_CXX20) \
>    || (__cpp_char8_t && _GLIBCXX_USE_UCHAR_CHAR8_T)
>
> Messy. Hmm.

Yup, you hit all the points I struggled with when coming up with an 
initial solution.  In the end, I settled on use of the C++ feature test 
macro being the only solution that worked in all cases, but it does 
imply that C libraries consistently follow suit.

Multiple probes seems like a reasonable option.  I think it is fair to 
assume that, if the declarations are present for -std=c++11 -fchar8_t, 
then they will also be present for later language standards.  So, if 
that probe fails, we can then probe again using just -std=c++20.

The conditional above would encounter errors for -std=c++20 -fno-char8_t 
for libc libraries that condition the declarations on the feature test 
macro.  I think it is reasonable to only enable the std namespace 
declarations if the __cpp_char8_t feature test macro is defined.

#if __cpp_char8_t \
     && (_GLIBCXX_USE_UCHAR_CXX11_CHAR8_T\
         || (__cplusplus >= 202002 &&_GLIBCXX_USE_UCHAR_CXX20))

Still messy, but I think this covers all the bases.

Tom.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2021-06-24 19:35   ` Tom Honermann
@ 2021-06-24 19:39     ` Jonathan Wakely
  2021-06-25  2:56       ` Tom Honermann
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2021-06-24 19:39 UTC (permalink / raw)
  To: Tom Honermann; +Cc: Jonathan Wakely, libstdc++

On Thu, 24 Jun 2021 at 20:35, Tom Honermann via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On 6/23/21 1:27 PM, Jonathan Wakely wrote:
> > N.B. please CC gcc-patches as well when sending patches to this list.
> Ah, yes, I think you've reminded me of that before.
> >
> > On Mon, 7 Jun 2021 at 03:16, Tom Honermann via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> >> This patch completes implementation of the C++20 proposal P0482R6 [1] by
> >> adding declarations of std::c8rtomb() and std::mbrtoc8() if provided by
> >> the C library.
> >>
> >> Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
> >> at global scope when uchar.h is included and compiled with -fchar8_t;
> >> _GLIBCXX_USE_UCHAR_CHAR8_T is defined if so.  The <cuchar> header
> >> re-declares these functions in the std namespace only if available and
> >> the C++20 __cpp_char8_t feature test macro is defined.
> >
> > Thanks, Tom. I'm surprised that there's no __cplusplus dependency
> > here. Some C libraries aren't going to declare these new functions
> > unconditionally, only for C2x and C++20 modes. Your new configure
> > check tests for them with -std=c++11 -fchar8_t (and the implicit
> > -D_GNU_SOURCE on GNU/Linux) which doesn't guarantee they'll be
> > available.
> >
> > This isn't a problem for your glibc patches, because you do:
> >
> > +#if defined _CHAR8_T_SOURCE || defined __cpp_char8_t
> > +# define __GLIBC_USE_CHAR8_T   1
> >
> > But consider a hypothetical libc that only define the new functions
> > for C2x/C++20, ignoring the __cpp_char8_t set by -fchar8_t. For such a
> > libc the configure test using -std=c++11 -fchar8_t will fail, and
> > libstdc++ won't declare the functions in namespace std even though
> > they are in libc.
> >
> > Shouldn't the configure test use -std=c++20 instead to check that
> > they're available for C++20 mode, and then in <cuchar> guard the using
> > declarations for ::c8rtomb and ::mbrtoc8 with #if __cplusplus >=
> > 202002L ?
> >
> > That would mean they're not declared in <cuchar> pre-C++20 when using
> > -fchar8_t, but that seems a lesser problem than having them not
> > declared in C++20 when they are actually present in libc. Maybe to
> > solve that we need two configure macros (or one macro with multiple
> > values), one that says the new functions are available for C++20, and
> > another that says they are also available pre-C++20 if __cpp_char8_t
> > is defined. Then <cuchar> can do something like:
> >
> > #if (__cplusplus >= 202002 && _GLIBCXX_USE_UCHAR_CXX20) \
> >    || (__cpp_char8_t && _GLIBCXX_USE_UCHAR_CHAR8_T)
> >
> > Messy. Hmm.
>
> Yup, you hit all the points I struggled with when coming up with an
> initial solution.  In the end, I settled on use of the C++ feature test
> macro being the only solution that worked in all cases, but it does
> imply that C libraries consistently follow suit.

We could just go with your patch for now, and see what happens when
other C libraries start to add the new functions. But if you are
willing to work on a revised patch with the extra conditions, that
would be good.

> Multiple probes seems like a reasonable option.  I think it is fair to
> assume that, if the declarations are present for -std=c++11 -fchar8_t,
> then they will also be present for later language standards.  So, if
> that probe fails, we can then probe again using just -std=c++20.

Sounds good.

> The conditional above would encounter errors for -std=c++20 -fno-char8_t
> for libc libraries that condition the declarations on the feature test
> macro.  I think it is reasonable to only enable the std namespace
> declarations if the __cpp_char8_t feature test macro is defined.
>
> #if __cpp_char8_t \
>      && (_GLIBCXX_USE_UCHAR_CXX11_CHAR8_T\
>          || (__cplusplus >= 202002 &&_GLIBCXX_USE_UCHAR_CXX20))
>
> Still messy, but I think this covers all the bases.

I did consider the -fno-char8_t case and decided I didn't care for
those people, but your suggestion allows it to work for them, without
making it _too_ much messier.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2021-06-24 19:39     ` Jonathan Wakely
@ 2021-06-25  2:56       ` Tom Honermann
  0 siblings, 0 replies; 10+ messages in thread
From: Tom Honermann @ 2021-06-25  2:56 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++

On 6/24/21 3:39 PM, Jonathan Wakely wrote:
> On Thu, 24 Jun 2021 at 20:35, Tom Honermann via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> On 6/23/21 1:27 PM, Jonathan Wakely wrote:
>>> N.B. please CC gcc-patches as well when sending patches to this list.
>> Ah, yes, I think you've reminded me of that before.
>>> On Mon, 7 Jun 2021 at 03:16, Tom Honermann via Libstdc++
>>> <libstdc++@gcc.gnu.org> wrote:
>>>> This patch completes implementation of the C++20 proposal P0482R6 [1] by
>>>> adding declarations of std::c8rtomb() and std::mbrtoc8() if provided by
>>>> the C library.
>>>>
>>>> Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
>>>> at global scope when uchar.h is included and compiled with -fchar8_t;
>>>> _GLIBCXX_USE_UCHAR_CHAR8_T is defined if so.  The <cuchar> header
>>>> re-declares these functions in the std namespace only if available and
>>>> the C++20 __cpp_char8_t feature test macro is defined.
>>> Thanks, Tom. I'm surprised that there's no __cplusplus dependency
>>> here. Some C libraries aren't going to declare these new functions
>>> unconditionally, only for C2x and C++20 modes. Your new configure
>>> check tests for them with -std=c++11 -fchar8_t (and the implicit
>>> -D_GNU_SOURCE on GNU/Linux) which doesn't guarantee they'll be
>>> available.
>>>
>>> This isn't a problem for your glibc patches, because you do:
>>>
>>> +#if defined _CHAR8_T_SOURCE || defined __cpp_char8_t
>>> +# define __GLIBC_USE_CHAR8_T   1
>>>
>>> But consider a hypothetical libc that only define the new functions
>>> for C2x/C++20, ignoring the __cpp_char8_t set by -fchar8_t. For such a
>>> libc the configure test using -std=c++11 -fchar8_t will fail, and
>>> libstdc++ won't declare the functions in namespace std even though
>>> they are in libc.
>>>
>>> Shouldn't the configure test use -std=c++20 instead to check that
>>> they're available for C++20 mode, and then in <cuchar> guard the using
>>> declarations for ::c8rtomb and ::mbrtoc8 with #if __cplusplus >=
>>> 202002L ?
>>>
>>> That would mean they're not declared in <cuchar> pre-C++20 when using
>>> -fchar8_t, but that seems a lesser problem than having them not
>>> declared in C++20 when they are actually present in libc. Maybe to
>>> solve that we need two configure macros (or one macro with multiple
>>> values), one that says the new functions are available for C++20, and
>>> another that says they are also available pre-C++20 if __cpp_char8_t
>>> is defined. Then <cuchar> can do something like:
>>>
>>> #if (__cplusplus >= 202002 && _GLIBCXX_USE_UCHAR_CXX20) \
>>>     || (__cpp_char8_t && _GLIBCXX_USE_UCHAR_CHAR8_T)
>>>
>>> Messy. Hmm.
>> Yup, you hit all the points I struggled with when coming up with an
>> initial solution.  In the end, I settled on use of the C++ feature test
>> macro being the only solution that worked in all cases, but it does
>> imply that C libraries consistently follow suit.
> We could just go with your patch for now, and see what happens when
> other C libraries start to add the new functions. But if you are
> willing to work on a revised patch with the extra conditions, that
> would be good.
I'm happy to do that; it isn't much effort.
>
>> Multiple probes seems like a reasonable option.  I think it is fair to
>> assume that, if the declarations are present for -std=c++11 -fchar8_t,
>> then they will also be present for later language standards.  So, if
>> that probe fails, we can then probe again using just -std=c++20.
> Sounds good.
>
>> The conditional above would encounter errors for -std=c++20 -fno-char8_t
>> for libc libraries that condition the declarations on the feature test
>> macro.  I think it is reasonable to only enable the std namespace
>> declarations if the __cpp_char8_t feature test macro is defined.
>>
>> #if __cpp_char8_t \
>>       && (_GLIBCXX_USE_UCHAR_CXX11_CHAR8_T\
>>           || (__cplusplus >= 202002 &&_GLIBCXX_USE_UCHAR_CXX20))
>>
>> Still messy, but I think this covers all the bases.
> I did consider the -fno-char8_t case and decided I didn't care for
> those people, but your suggestion allows it to work for them, without
> making it _too_ much messier.

Yup.  And I don't want to give those people more reasons for being 
unhappy with me ;)

I'll follow up with a revised patch soon.  Thank you for the review!

Tom.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2022-01-10 21:38     ` Jonathan Wakely
@ 2022-01-11 20:13       ` Tom Honermann
  0 siblings, 0 replies; 10+ messages in thread
From: Tom Honermann @ 2022-01-11 20:13 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 5009 bytes --]

On 1/10/22 4:38 PM, Jonathan Wakely wrote:
> On Mon, 10 Jan 2022 at 21:24, Tom Honermann via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> On 1/10/22 8:23 AM, Jonathan Wakely wrote:
>>>
>>> On Sat, 8 Jan 2022 at 00:42, Tom Honermann via Libstdc++
>>> <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
>>>
>>>      This patch completes implementation of the C++20 proposal P0482R6
>>>      [1] by
>>>      adding declarations of std::c8rtomb() and std::mbrtoc8() in
>>>      <cuchar> if
>>>      provided by the C library in <uchar.h>.
>>>
>>>      This patch addresses feedback provided in response to a previous
>>>      patch
>>>      submission [2].
>>>
>>>      Autoconf changes determine if the C library declares c8rtomb and
>>>      mbrtoc8
>>>      at global scope when uchar.h is included and compiled with either
>>>      -fchar8_t or -std=c++20. New
>>>      _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
>>>      and _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros
>>>      reflect the probe results. The <cuchar> header declares these
>>>      functions
>>>      in the std namespace only if available and the _GLIBCXX_USE_CHAR8_T
>>>      configuration macro is defined (by default it is defined if the C++20
>>>      __cpp_char8_t feature test macro is defined)
>>>
>>>      Patches to glibc to implement c8rtomb and mbrtoc8 have been
>>>      submitted [3].
>>>
>>>      New tests validate the presence of these declarations. The tests pass
>>>      trivially if the C library does not provide these functions.
>>>      Otherwise
>>>      they ensure that the functions are declared when <cuchar> is included
>>>      and either -fchar8_t or -std=c++20 is enabled.
>>>
>>>      Tested on Linux x86_64.
>>>
>>>      libstdc++-v3/ChangeLog:
>>>
>>>      2022-01-07  Tom Honermann  <tom@honermann.net
>>>      <mailto:tom@honermann.net>>
>>>
>>>              * acinclude.m4 Define config macros if uchar.h provides
>>>              c8rtomb() and mbrtoc8().
>>>              * config.h.in <http://config.h.in>: Re-generate.
>>>              * configure: Re-generate.
>>>              * include/c_compatibility/uchar.h: Declare ::c8rtomb and
>>>              ::mbrtoc8.
>>>              * include/c_global/cuchar: Declare std::c8rtomb and
>>>              std::mbrtoc8.
>>>              * include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8.
>>>              * testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc:
>>>              New test.
>>>              *
>>>      testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc:
>>>              New test.
>>>
>>>
>>>
>>> Thanks, Tom, this looks good and I'll get it committed for GCC 12.
>> Thank you!
>>> My only concern is that the new tests depend on an internal macro:
>>>
>>> +#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
>>> +  using std::mbrtoc8;
>>> +  using std::c8rtomb;
>>>
>>> I prefer if tests are written as "user code" when possible, and not
>>> using our internal macros. That isn't always possible, and in this
>>> case would require adding new effective-target keyword to
>>> testsuite/lib/libstdc++.exp just for use in these two tests. I don't
>>> think we should bother with that.
>> I went with this approach solely due to my unfamiliarity with the test
>> system. I knew there should be a way to conditionally make the test
>> "pass" as unsupported or as an expected failure, but didn't know how to
>> go about implementing that. I don't mind following up with an additional
>> patch if such a change is desirable. I took a look at
>> testsuite/lib/libstdc++.exp and it looks like it may be pretty straight
>> forward to add effective-target support. It would probably be a good
>> learning experience for me. I'll prototype and report back.
> Yes, it's very easy to do. Take a look at the
> check_effective_target_blah procs in that file, especially the later
> ones that use v3_check_preprocessor_condition. You can use that to
> define an effective target keyword for any preprocessor condition
> (such as the new macros you're adding).
>
> Then the test can do:
> // { dg-do compile { target blah } }
> which will make it UNSUPPORTED if the effective target proc doesn't return true.
> See https://gcc.gnu.org/onlinedocs/gccint/Selectors.html#Selectors for
> the docs on target selectors.
>
> I'm just not sure it's worth adding a new keyword for just two tests.

Thank you for the implementation direction; this was quite easy!

Patch attached (to be applied after the original one).

libstdc++-v3/ChangeLog:

2022-01-11  Tom Honermann  <tom@honermann.net>

	* testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc:
	Modify to use new c8rtomb_mbrtoc8_cxx20 effective target.
	* testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc:
	Modify to use new c8rtomb_mbrtoc8_fchar8_t effective target.
	* testsuite/lib/libstdc++.exp: Add new effective targets.

If you decide that the new keywords aren't worth adding, no worries; my 
feelings won't be hurt :)

Tom.


[-- Attachment #2: p0482r6-n2653-eff-target.patch --]
[-- Type: text/x-patch, Size: 2648 bytes --]

commit 0542361fe8cb5da146097f86ca8ea8bca86421e0
Author: Tom Honermann <tom@honermann.net>
Date:   Tue Jan 11 14:57:51 2022 -0500

    Add effective target support for tests of C++20 c8rtomb and mbrtoc8.

diff --git a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc
index 7c152ed42b5..681c12127db 100644
--- a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc
+++ b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc
@@ -1,12 +1,10 @@
-// { dg-do compile }
+// { dg-do compile { target c8rtomb_mbrtoc8_cxx20 } }
 // { dg-options "-std=c++20" }
 
 #include <cuchar>
 
 namespace gnu
 {
-#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
   using std::mbrtoc8;
   using std::c8rtomb;
-#endif // _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
 }
diff --git a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc
index 1cfaf7427e5..836690f0349 100644
--- a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc
+++ b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc
@@ -1,12 +1,10 @@
-// { dg-do compile }
+// { dg-do compile { target c8rtomb_mbrtoc8_fchar8_t } }
 // { dg-options "-fchar8_t" }
 
 #include <cuchar>
 
 namespace gnu
 {
-#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
   using std::mbrtoc8;
   using std::c8rtomb;
-#endif // _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
 }
diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 1a43dd05ba7..7a2379c9982 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -1346,6 +1346,22 @@ proc check_effective_target_std_allocator_new { } {
     }]
 }
 
+# Return 1 if mbrtoc8 and c8rtomb are available for C++20, 0 otherwise.
+proc check_effective_target_c8rtomb_mbrtoc8_cxx20 { } {
+    return [check_v3_target_prop_cached et_c8rtomb_mbrtoc8_cxx20 {
+	set cond "_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20"
+	return [v3_check_preprocessor_condition c8rtomb_mbrtoc8_cxx20 $cond]
+    }]
+}
+
+# Return 1 if mbrtoc8 and c8rtomb are available for -fchar8_t, 0 otherwise.
+proc check_effective_target_c8rtomb_mbrtoc8_fchar8_t { } {
+    return [check_v3_target_prop_cached et_c8rtomb_mbrtoc8_fchar8_t {
+	set cond "_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T"
+	return [v3_check_preprocessor_condition c8rtomb_mbrtoc8_fchar8_t $cond]
+    }]
+}
+
 set additional_prunes ""
 
 if { [info exists env(GCC_RUNTEST_PARALLELIZE_DIR)] \

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2022-01-10 21:23   ` Tom Honermann
@ 2022-01-10 21:38     ` Jonathan Wakely
  2022-01-11 20:13       ` Tom Honermann
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2022-01-10 21:38 UTC (permalink / raw)
  To: Tom Honermann; +Cc: Jonathan Wakely, libstdc++, gcc-patches

On Mon, 10 Jan 2022 at 21:24, Tom Honermann via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On 1/10/22 8:23 AM, Jonathan Wakely wrote:
> >
> >
> > On Sat, 8 Jan 2022 at 00:42, Tom Honermann via Libstdc++
> > <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
> >
> >     This patch completes implementation of the C++20 proposal P0482R6
> >     [1] by
> >     adding declarations of std::c8rtomb() and std::mbrtoc8() in
> >     <cuchar> if
> >     provided by the C library in <uchar.h>.
> >
> >     This patch addresses feedback provided in response to a previous
> >     patch
> >     submission [2].
> >
> >     Autoconf changes determine if the C library declares c8rtomb and
> >     mbrtoc8
> >     at global scope when uchar.h is included and compiled with either
> >     -fchar8_t or -std=c++20. New
> >     _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
> >     and _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros
> >     reflect the probe results. The <cuchar> header declares these
> >     functions
> >     in the std namespace only if available and the _GLIBCXX_USE_CHAR8_T
> >     configuration macro is defined (by default it is defined if the C++20
> >     __cpp_char8_t feature test macro is defined)
> >
> >     Patches to glibc to implement c8rtomb and mbrtoc8 have been
> >     submitted [3].
> >
> >     New tests validate the presence of these declarations. The tests pass
> >     trivially if the C library does not provide these functions.
> >     Otherwise
> >     they ensure that the functions are declared when <cuchar> is included
> >     and either -fchar8_t or -std=c++20 is enabled.
> >
> >     Tested on Linux x86_64.
> >
> >     libstdc++-v3/ChangeLog:
> >
> >     2022-01-07  Tom Honermann  <tom@honermann.net
> >     <mailto:tom@honermann.net>>
> >
> >             * acinclude.m4 Define config macros if uchar.h provides
> >             c8rtomb() and mbrtoc8().
> >             * config.h.in <http://config.h.in>: Re-generate.
> >             * configure: Re-generate.
> >             * include/c_compatibility/uchar.h: Declare ::c8rtomb and
> >             ::mbrtoc8.
> >             * include/c_global/cuchar: Declare std::c8rtomb and
> >             std::mbrtoc8.
> >             * include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8.
> >             * testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc:
> >             New test.
> >             *
> >     testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc:
> >             New test.
> >
> >
> >
> > Thanks, Tom, this looks good and I'll get it committed for GCC 12.
> Thank you!
> >
> > My only concern is that the new tests depend on an internal macro:
> >
> > +#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
> > +  using std::mbrtoc8;
> > +  using std::c8rtomb;
> >
> > I prefer if tests are written as "user code" when possible, and not
> > using our internal macros. That isn't always possible, and in this
> > case would require adding new effective-target keyword to
> > testsuite/lib/libstdc++.exp just for use in these two tests. I don't
> > think we should bother with that.
> I went with this approach solely due to my unfamiliarity with the test
> system. I knew there should be a way to conditionally make the test
> "pass" as unsupported or as an expected failure, but didn't know how to
> go about implementing that. I don't mind following up with an additional
> patch if such a change is desirable. I took a look at
> testsuite/lib/libstdc++.exp and it looks like it may be pretty straight
> forward to add effective-target support. It would probably be a good
> learning experience for me. I'll prototype and report back.

Yes, it's very easy to do. Take a look at the
check_effective_target_blah procs in that file, especially the later
ones that use v3_check_preprocessor_condition. You can use that to
define an effective target keyword for any preprocessor condition
(such as the new macros you're adding).

Then the test can do:
// { dg-do compile { target blah } }
which will make it UNSUPPORTED if the effective target proc doesn't return true.
See https://gcc.gnu.org/onlinedocs/gccint/Selectors.html#Selectors for
the docs on target selectors.

I'm just not sure it's worth adding a new keyword for just two tests.


> >
> > I suppose strictly speaking we should not define __cpp_lib_char8_t
> > unless these two functions are present in libc. But I'm not sure we
> > want to change that now either.
>
> All of libstdc++, libc++, and MS STL have been defining
> __cpp_lib_char8_t despite the absence of these functions, so yeah, I
> don't think we want to change that.

OK, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2022-01-10 13:23 ` Jonathan Wakely
@ 2022-01-10 21:23   ` Tom Honermann
  2022-01-10 21:38     ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Honermann @ 2022-01-10 21:23 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 1/10/22 8:23 AM, Jonathan Wakely wrote:
>
>
> On Sat, 8 Jan 2022 at 00:42, Tom Honermann via Libstdc++ 
> <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote:
>
>     This patch completes implementation of the C++20 proposal P0482R6
>     [1] by
>     adding declarations of std::c8rtomb() and std::mbrtoc8() in
>     <cuchar> if
>     provided by the C library in <uchar.h>.
>
>     This patch addresses feedback provided in response to a previous
>     patch
>     submission [2].
>
>     Autoconf changes determine if the C library declares c8rtomb and
>     mbrtoc8
>     at global scope when uchar.h is included and compiled with either
>     -fchar8_t or -std=c++20. New
>     _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
>     and _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros
>     reflect the probe results. The <cuchar> header declares these
>     functions
>     in the std namespace only if available and the _GLIBCXX_USE_CHAR8_T
>     configuration macro is defined (by default it is defined if the C++20
>     __cpp_char8_t feature test macro is defined)
>
>     Patches to glibc to implement c8rtomb and mbrtoc8 have been
>     submitted [3].
>
>     New tests validate the presence of these declarations. The tests pass
>     trivially if the C library does not provide these functions.
>     Otherwise
>     they ensure that the functions are declared when <cuchar> is included
>     and either -fchar8_t or -std=c++20 is enabled.
>
>     Tested on Linux x86_64.
>
>     libstdc++-v3/ChangeLog:
>
>     2022-01-07  Tom Honermann  <tom@honermann.net
>     <mailto:tom@honermann.net>>
>
>             * acinclude.m4 Define config macros if uchar.h provides
>             c8rtomb() and mbrtoc8().
>             * config.h.in <http://config.h.in>: Re-generate.
>             * configure: Re-generate.
>             * include/c_compatibility/uchar.h: Declare ::c8rtomb and
>             ::mbrtoc8.
>             * include/c_global/cuchar: Declare std::c8rtomb and
>             std::mbrtoc8.
>             * include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8.
>             * testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc:
>             New test.
>             *
>     testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc:
>             New test.
>
>
>
> Thanks, Tom, this looks good and I'll get it committed for GCC 12.
Thank you!
>
> My only concern is that the new tests depend on an internal macro:
>
> +#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
> +  using std::mbrtoc8;
> +  using std::c8rtomb;
>
> I prefer if tests are written as "user code" when possible, and not 
> using our internal macros. That isn't always possible, and in this 
> case would require adding new effective-target keyword to 
> testsuite/lib/libstdc++.exp just for use in these two tests. I don't 
> think we should bother with that.
I went with this approach solely due to my unfamiliarity with the test 
system. I knew there should be a way to conditionally make the test 
"pass" as unsupported or as an expected failure, but didn't know how to 
go about implementing that. I don't mind following up with an additional 
patch if such a change is desirable. I took a look at 
testsuite/lib/libstdc++.exp and it looks like it may be pretty straight 
forward to add effective-target support. It would probably be a good 
learning experience for me. I'll prototype and report back.
>
> I suppose strictly speaking we should not define __cpp_lib_char8_t 
> unless these two functions are present in libc. But I'm not sure we 
> want to change that now either.

All of libstdc++, libc++, and MS STL have been defining 
__cpp_lib_char8_t despite the absence of these functions, so yeah, I 
don't think we want to change that.

Tom.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
  2022-01-08  0:42 Tom Honermann
@ 2022-01-10 13:23 ` Jonathan Wakely
  2022-01-10 21:23   ` Tom Honermann
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2022-01-10 13:23 UTC (permalink / raw)
  To: Tom Honermann; +Cc: libstdc++, gcc-patches

On Sat, 8 Jan 2022 at 00:42, Tom Honermann via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> This patch completes implementation of the C++20 proposal P0482R6 [1] by
> adding declarations of std::c8rtomb() and std::mbrtoc8() in <cuchar> if
> provided by the C library in <uchar.h>.
>
> This patch addresses feedback provided in response to a previous patch
> submission [2].
>
> Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
> at global scope when uchar.h is included and compiled with either
> -fchar8_t or -std=c++20. New _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
> and _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros
> reflect the probe results. The <cuchar> header declares these functions
> in the std namespace only if available and the _GLIBCXX_USE_CHAR8_T
> configuration macro is defined (by default it is defined if the C++20
> __cpp_char8_t feature test macro is defined)
>
> Patches to glibc to implement c8rtomb and mbrtoc8 have been submitted [3].
>
> New tests validate the presence of these declarations. The tests pass
> trivially if the C library does not provide these functions. Otherwise
> they ensure that the functions are declared when <cuchar> is included
> and either -fchar8_t or -std=c++20 is enabled.
>
> Tested on Linux x86_64.
>
> libstdc++-v3/ChangeLog:
>
> 2022-01-07  Tom Honermann  <tom@honermann.net>
>
>         * acinclude.m4 Define config macros if uchar.h provides
>         c8rtomb() and mbrtoc8().
>         * config.h.in: Re-generate.
>         * configure: Re-generate.
>         * include/c_compatibility/uchar.h: Declare ::c8rtomb and
>         ::mbrtoc8.
>         * include/c_global/cuchar: Declare std::c8rtomb and
>         std::mbrtoc8.
>         * include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8.
>         * testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc:
>         New test.
>         * testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc:
>         New test.
>


Thanks, Tom, this looks good and I'll get it committed for GCC 12.

My only concern is that the new tests depend on an internal macro:

+#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
+  using std::mbrtoc8;
+  using std::c8rtomb;

I prefer if tests are written as "user code" when possible, and not using
our internal macros. That isn't always possible, and in this case would
require adding new effective-target keyword to testsuite/lib/libstdc++.exp
just for use in these two tests. I don't think we should bother with that.

I suppose strictly speaking we should not define __cpp_lib_char8_t unless
these two functions are present in libc. But I'm not sure we want to change
that now either.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library
@ 2022-01-08  0:42 Tom Honermann
  2022-01-10 13:23 ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Honermann @ 2022-01-08  0:42 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2442 bytes --]

This patch completes implementation of the C++20 proposal P0482R6 [1] by 
adding declarations of std::c8rtomb() and std::mbrtoc8() in <cuchar> if 
provided by the C library in <uchar.h>.

This patch addresses feedback provided in response to a previous patch 
submission [2].

Autoconf changes determine if the C library declares c8rtomb and mbrtoc8 
at global scope when uchar.h is included and compiled with either 
-fchar8_t or -std=c++20. New _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T 
and _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros 
reflect the probe results. The <cuchar> header declares these functions 
in the std namespace only if available and the _GLIBCXX_USE_CHAR8_T 
configuration macro is defined (by default it is defined if the C++20 
__cpp_char8_t feature test macro is defined)

Patches to glibc to implement c8rtomb and mbrtoc8 have been submitted [3].

New tests validate the presence of these declarations. The tests pass 
trivially if the C library does not provide these functions. Otherwise 
they ensure that the functions are declared when <cuchar> is included 
and either -fchar8_t or -std=c++20 is enabled.

Tested on Linux x86_64.

libstdc++-v3/ChangeLog:

2022-01-07  Tom Honermann  <tom@honermann.net>

	* acinclude.m4 Define config macros if uchar.h provides
	c8rtomb() and mbrtoc8().
	* config.h.in: Re-generate.
	* configure: Re-generate.
	* include/c_compatibility/uchar.h: Declare ::c8rtomb and
	::mbrtoc8.
	* include/c_global/cuchar: Declare std::c8rtomb and
	std::mbrtoc8.
	* include/c_std/cuchar: Declare std::c8rtomb and std::mbrtoc8.
	* testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc:
	New test.
	* testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc:
	New test.

Tom.

[1]: WG21 P0482R6
      "char8_t: A type for UTF-8 characters and strings (Revision 6)"
      https://wg21.link/p0482r6

[2]: [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 
if provided by the C library
      https://gcc.gnu.org/pipermail/libstdc++/2021-June/052685.html

[3]: "C++20 P0482R6 and C2X N2653"
      [Patch 0/3]: 
https://sourceware.org/pipermail/libc-alpha/2022-January/135061.html
      [Patch 1/3]: 
https://sourceware.org/pipermail/libc-alpha/2022-January/135062.html
      [Patch 2/3]: 
https://sourceware.org/pipermail/libc-alpha/2022-January/135063.html
      [Patch 3/3]: 
https://sourceware.org/pipermail/libc-alpha/2022-January/135064.html

Tom.

[-- Attachment #2: p0482r6-n2653.patch --]
[-- Type: text/x-patch, Size: 10505 bytes --]

commit 3d40bc9bf5c79343ea5a6cc355539542f4b56c9b
Author: Tom Honermann <tom@honermann.net>
Date:   Sat Jan 1 17:26:31 2022 -0500

    P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library.
    
    This change completes implementation of the C++20 proposal P0482R6 by
    adding declarations of std::c8rtomb() and std::mbrtoc8() if provided
    by the C library.
    
    Autoconf changes determine if the C library declares c8rtomb and mbrtoc8
    at global scope when uchar.h is included and compiled with either -fchar8_t
    or -std=c++20 enabled; new _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T and
    _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 configuration macros are defined
    accordingly. The <cuchar> header declares these functions in the std
    namespace only if available and the _GLIBCXX_USE_CHAR8_T configuration
    macro is defined (by default it is defined if the C++20 __cpp_char8_t
    feature test macro is defined).
    
    New tests validate the presence of these declarations. The tests pass
    trivially if the C library does not provide these functions. Otherwise they
    ensure that the functions are declared when <cuchar> is included and
    either -fchar8_t or -std=c++20 is enabled.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 635168d7e25..85235005c7e 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2039,6 +2039,50 @@ AC_DEFUN([GLIBCXX_CHECK_UCHAR_H], [
 	      namespace std in <cuchar>.])
   fi
 
+  CXXFLAGS="$CXXFLAGS -fchar8_t"
+  if test x"$ac_has_uchar_h" = x"yes"; then
+    AC_MSG_CHECKING([for c8rtomb and mbrtoc8 in <uchar.h> with -fchar8_t])
+    AC_TRY_COMPILE([#include <uchar.h>
+		    namespace test
+		    {
+		      using ::c8rtomb;
+		      using ::mbrtoc8;
+		    }
+		   ],
+		   [], [ac_uchar_c8rtomb_mbrtoc8_fchar8_t=yes],
+		       [ac_uchar_c8rtomb_mbrtoc8_fchar8_t=no])
+  else
+    ac_uchar_c8rtomb_mbrtoc8_fchar8_t=no
+  fi
+  AC_MSG_RESULT($ac_uchar_c8rtomb_mbrtoc8_fchar8_t)
+  if test x"$ac_uchar_c8rtomb_mbrtoc8_fchar8_t" = x"yes"; then
+    AC_DEFINE(_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T, 1,
+	      [Define if c8rtomb and mbrtoc8 functions in <uchar.h> should be
+	      imported into namespace std in <cuchar> for -fchar8_t.])
+  fi
+
+  CXXFLAGS="$CXXFLAGS -std=c++20"
+  if test x"$ac_has_uchar_h" = x"yes"; then
+    AC_MSG_CHECKING([for c8rtomb and mbrtoc8 in <uchar.h> with -std=c++20])
+    AC_TRY_COMPILE([#include <uchar.h>
+		    namespace test
+		    {
+		      using ::c8rtomb;
+		      using ::mbrtoc8;
+		    }
+		   ],
+		   [], [ac_uchar_c8rtomb_mbrtoc8_cxx20=yes],
+		       [ac_uchar_c8rtomb_mbrtoc8_cxx20=no])
+  else
+    ac_uchar_c8rtomb_mbrtoc8_cxx20=no
+  fi
+  AC_MSG_RESULT($ac_uchar_c8rtomb_mbrtoc8_cxx20)
+  if test x"$ac_uchar_c8rtomb_mbrtoc8_cxx20" = x"yes"; then
+    AC_DEFINE(_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20, 1,
+	      [Define if c8rtomb and mbrtoc8 functions in <uchar.h> should be
+	      imported into namespace std in <cuchar> for C++20.])
+  fi
+
   CXXFLAGS="$ac_save_CXXFLAGS"
   AC_LANG_RESTORE
 ])
diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
index 10675fe2530..e09744961d1 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -1000,6 +1000,14 @@
 /* Define if obsolescent tmpnam is available in <stdio.h>. */
 #undef _GLIBCXX_USE_TMPNAM
 
+/* Define if c8rtomb and std functions in <uchar.h> should be
+   imported into namespace std in <cuchar> for -fchar8_t. */
+#undef _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
+
+/* Define if c8rtomb and std functions in <uchar.h> should be
+   imported into namespace std in <cuchar> for C++20. */
+#undef _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
+
 /* Define if utime is available in <utime.h>. */
 #undef _GLIBCXX_USE_UTIME
 
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index b1a0157d0b9..fae78ec1cc6 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -19130,6 +19130,82 @@ $as_echo "#define _GLIBCXX_USE_C11_UCHAR_CXX11 1" >>confdefs.h
 
   fi
 
+  CXXFLAGS="$CXXFLAGS -fchar8_t"
+  if test x"$ac_has_uchar_h" = x"yes"; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for c8rtomb and mbrtoc8 in <uchar.h> with -fchar8_t" >&5
+$as_echo_n "checking for c8rtomb and mbrtoc8 in <uchar.h> with -fchar8_t... " >&6; }
+    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <uchar.h>
+		    namespace test
+		    {
+		      using ::c8rtomb;
+		      using ::mbrtoc8;
+		    }
+
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  ac_uchar_c8rtomb_mbrtoc8_fchar8_t=yes
+else
+  ac_uchar_c8rtomb_mbrtoc8_fchar8_t=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+  else
+    ac_uchar_c8rtomb_mbrtoc8_fchar8_t=no
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_uchar_c8rtomb_mbrtoc8_fchar8_t" >&5
+$as_echo "$ac_uchar_c8rtomb_mbrtoc8_fchar8_t" >&6; }
+  if test x"$ac_uchar_c8rtomb_mbrtoc8_fchar8_t" = x"yes"; then
+
+$as_echo "#define _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T 1" >>confdefs.h
+
+  fi
+
+  CXXFLAGS="$CXXFLAGS -std=c++20"
+  if test x"$ac_has_uchar_h" = x"yes"; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for c8rtomb and mbrtoc8 in <uchar.h> with -std=c++20" >&5
+$as_echo_n "checking for c8rtomb and mbrtoc8 in <uchar.h> with -std=c++20... " >&6; }
+    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <uchar.h>
+		    namespace test
+		    {
+		      using ::c8rtomb;
+		      using ::mbrtoc8;
+		    }
+
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  ac_uchar_c8rtomb_mbrtoc8_cxx20=yes
+else
+  ac_uchar_c8rtomb_mbrtoc8_cxx20=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+  else
+    ac_uchar_c8rtomb_mbrtoc8_cxx20=no
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_uchar_c8rtomb_mbrtoc8_cxx20" >&5
+$as_echo "$ac_uchar_c8rtomb_mbrtoc8_cxx20" >&6; }
+  if test x"$ac_uchar_c8rtomb_mbrtoc8_cxx20" = x"yes"; then
+
+$as_echo "#define _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20 1" >>confdefs.h
+
+  fi
+
   CXXFLAGS="$ac_save_CXXFLAGS"
   ac_ext=c
 ac_cpp='$CPP $CPPFLAGS'
diff --git a/libstdc++-v3/include/c_compatibility/uchar.h b/libstdc++-v3/include/c_compatibility/uchar.h
index 1fe8a22f78a..b2346e70d2b 100644
--- a/libstdc++-v3/include/c_compatibility/uchar.h
+++ b/libstdc++-v3/include/c_compatibility/uchar.h
@@ -33,6 +33,14 @@
 
 #ifdef _GLIBCXX_NAMESPACE_C
 
+#if (_GLIBCXX_USE_CHAR8_T \
+     && (_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T \
+	 || (__cplusplus >= 202002 \
+	     && _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20)))
+using std::mbrtoc8;
+using std::c8rtomb;
+#endif // _GLIBCXX_USE_CHAR8_T
+
 #if _GLIBCXX_USE_C11_UCHAR_CXX11
 using std::mbrtoc16;
 using std::c16rtomb;
diff --git a/libstdc++-v3/include/c_global/cuchar b/libstdc++-v3/include/c_global/cuchar
index 57047d74218..943d2727878 100644
--- a/libstdc++-v3/include/c_global/cuchar
+++ b/libstdc++-v3/include/c_global/cuchar
@@ -48,10 +48,41 @@
 #include <bits/c++config.h>
 #include <cwchar>
 
-#if _GLIBCXX_USE_C11_UCHAR_CXX11
+#if (_GLIBCXX_USE_C11_UCHAR_CXX11 \
+     || (_GLIBCXX_USE_CHAR8_T \
+	 && (_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T \
+	     || (__cplusplus >= 202002 \
+		 && _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20))))
 
 #include <uchar.h>
 
+#endif
+
+
+// Support for mbrtoc8 and c8rtomb is conditioned on support by the C library.
+#if (_GLIBCXX_USE_CHAR8_T \
+     && (_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T \
+	 || (__cplusplus >= 202002 \
+	     && _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20)))
+
+#undef mbrtoc8
+#undef c8rtomb
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  using ::mbrtoc8;
+  using ::c8rtomb;
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
+
+#endif // _GLIBCXX_USE_CHAR8_T
+
+
+#if _GLIBCXX_USE_C11_UCHAR_CXX11
+
 // Get rid of those macros defined in <uchar.h> in lieu of real functions.
 #undef mbrtoc16
 #undef c16rtomb
diff --git a/libstdc++-v3/include/c_std/cuchar b/libstdc++-v3/include/c_std/cuchar
index 7ce413e1bb0..56c11f7d24f 100644
--- a/libstdc++-v3/include/c_std/cuchar
+++ b/libstdc++-v3/include/c_std/cuchar
@@ -48,10 +48,42 @@
 #include <bits/c++config.h>
 #include <cwchar>
 
-#if _GLIBCXX_USE_C11_UCHAR_CXX11
+#if (_GLIBCXX_USE_C11_UCHAR_CXX11 \
+     || (_GLIBCXX_USE_CHAR8_T \
+	 && (_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T \
+	     || (__cplusplus >= 202002 \
+		 && _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20))))
 
 #include <uchar.h>
 
+#endif
+
+
+// Support for mbrtoc8 and c8rtomb is conditioned on support by the C library.
+#if (_GLIBCXX_USE_CHAR8_T \
+     && (_GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T \
+	 || (__cplusplus >= 202002 \
+	     && _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20)))
+
+// Get rid of those macros defined in <uchar.h> in lieu of real functions.
+#undef mbrtoc8
+#undef c8rtomb
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  using ::mbrtoc8;
+  using ::c8rtomb;
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
+
+#endif // _GLIBCXX_USE_CHAR8_T
+
+
+#if _GLIBCXX_USE_C11_UCHAR_CXX11
+
 // Get rid of those macros defined in <uchar.h> in lieu of real functions.
 #undef mbrtoc16
 #undef c16rtomb
diff --git a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc
new file mode 100644
index 00000000000..7c152ed42b5
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_cxx20.cc
@@ -0,0 +1,12 @@
+// { dg-do compile }
+// { dg-options "-std=c++20" }
+
+#include <cuchar>
+
+namespace gnu
+{
+#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
+  using std::mbrtoc8;
+  using std::c8rtomb;
+#endif // _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_CXX20
+}
diff --git a/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc
new file mode 100644
index 00000000000..1cfaf7427e5
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/headers/cuchar/functions_std_fchar8_t.cc
@@ -0,0 +1,12 @@
+// { dg-do compile }
+// { dg-options "-fchar8_t" }
+
+#include <cuchar>
+
+namespace gnu
+{
+#if _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
+  using std::mbrtoc8;
+  using std::c8rtomb;
+#endif // _GLIBCXX_USE_UCHAR_C8RTOMB_MBRTOC8_FCHAR8_T
+}

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-01-11 20:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-07  2:16 [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library Tom Honermann
2021-06-23 17:27 ` Jonathan Wakely
2021-06-24 19:35   ` Tom Honermann
2021-06-24 19:39     ` Jonathan Wakely
2021-06-25  2:56       ` Tom Honermann
2022-01-08  0:42 Tom Honermann
2022-01-10 13:23 ` Jonathan Wakely
2022-01-10 21:23   ` Tom Honermann
2022-01-10 21:38     ` Jonathan Wakely
2022-01-11 20:13       ` Tom Honermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).