public inbox for gcc-patches@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
@ 2022-01-08  0:42 Tom Honermann
  2022-01-10 13:23 ` Jonathan Wakely
  0 siblings, 1 reply; 5+ 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] 5+ 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 [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library Tom Honermann
@ 2022-01-10 13:23 ` Jonathan Wakely
  2022-01-10 21:23   ` Tom Honermann
  0 siblings, 1 reply; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread

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

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-08  0:42 [PATCH] C++ P0482R6 char8_t: declare std::c8rtomb and std::mbrtoc8 if provided by the C library 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).