public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17
@ 2017-04-23 18:54 Pedro Alves
  2017-04-23 21:38 ` Pedro Alves
  0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2017-04-23 18:54 UTC (permalink / raw)
  To: Jonathan Wakely, libstdc++, gcc-patches

Hi!

As I had suggested in PR c++/80265, here's a patch that uses
__builtin_constant_p to tell whether we can defer to a constexpr
algorithm, which avoids having to wait for compiler support.

Unfortunately I ran out of cycles today to run a full bootstrap/regtest
cycle, but constexpr_functions_c++17.cc passes cleanly on
x86_64 GNU/Linux at least.

If this looks like a reasonable approach, I can maybe try running
full tests on the gcc compile farm next week.

WDYT?

Thanks,
Pedro Alves

From 91606ca1f51309121e292559bcb6b2cfc126737b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sun, 23 Apr 2017 14:16:09 +0100
Subject: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits"
 for C++17

As discussed in PR c++/80265 (__builtin_{memcmp,memchr,strlen} are not
usable in constexpr functions), use __builtin_constant_p to tell
whether we can defer to a constexpr algorithm.

I used __always_inline__ just to be thorough.  It isn't really really
necessary as far as I could determine.

Changes like these:

	 if (__n == 0)
	   return 0;
 -	return wmemcmp(__s1, __s2, __n);
 +	else
 +	  return wmemcmp(__s1, __s2, __n);

are necessary otherwise G++ complains that we're calling a
non-constexpr function, which looks like a a manifestation of PR67026
to me.

libstdc++-v3:
2017-04-23  Pedro Alves  <palves@redhat.com>

	* doc/xml/manual/status_cxx2017.xml: Update C++17 constexpr
	char_traits status.
	* doc/html/*: Regenerate.

	* include/bits/char_traits.h (_GLIBCXX_ALWAYS_INLINE): Define if
	not already defined.
	(__cpp_lib_constexpr_char_traits): Uncomment.
	(__constant_string_p, __constant_char_array_p): New.
	(std::char_traits<char>, std::char_traits<wchar_t>): Add
	_GLIBCXX17_CONSTEXPR on compare, length, find and assign and use
	__constant_string_p, __constant_char_array_p and
	__builtin_constant_p to defer to __gnu_cxx::char_traits at compile
	time.

	* testsuite/21_strings/char_traits/requirements/
	constexpr_functions_c++17.cc: Uncomment
	__cpp_lib_constexpr_char_traits tests.  Uncomment
	test_compare<char>, test_length<char>, test_find<char>,
	test_compare<wchar_t>, test_length<wchar_t> and test_find<wchar_t>
	static_assert tests.
---
 libstdc++-v3/doc/xml/manual/status_cxx2017.xml     |   4 +-
 libstdc++-v3/include/bits/char_traits.h            | 120 ++++++++++++++++++---
 .../requirements/constexpr_functions_c++17.cc      |  16 +--
 3 files changed, 116 insertions(+), 24 deletions(-)

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
index 0e35f75..fed91f9 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
@@ -489,8 +489,8 @@ Feature-testing recommendations for C++</link>.
 	P0426R1
 	</link>
       </entry>
-      <entry align="center"> 7 (partial) </entry>
-      <entry><code> ??? </code></entry>
+      <entry align="center"> 7 </entry>
+      <entry><code> __cpp_lib_constexpr_char_traits >= 201611 </code></entry>
     </row>
 
     <row>
diff --git a/libstdc++-v3/include/bits/char_traits.h b/libstdc++-v3/include/bits/char_traits.h
index 75db5b8..cc636a9 100644
--- a/libstdc++-v3/include/bits/char_traits.h
+++ b/libstdc++-v3/include/bits/char_traits.h
@@ -40,6 +40,10 @@
 #include <bits/postypes.h>      // For streampos
 #include <cwchar>               // For WEOF, wmemmove, wmemset, etc.
 
+#ifndef _GLIBCXX_ALWAYS_INLINE
+#define _GLIBCXX_ALWAYS_INLINE inline __attribute__((__always_inline__))
+#endif
+
 namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -139,7 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       { return !eq_int_type(__c, eof()) ? __c : to_int_type(char_type()); }
     };
 
-// #define __cpp_lib_constexpr_char_traits 201611
+#define __cpp_lib_constexpr_char_traits 201611
 
   template<typename _CharT>
     _GLIBCXX14_CONSTEXPR int
@@ -212,6 +216,42 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+#if __cplusplus > 201402
+  /**
+   *  @brief Determine whether the characters of a NULL-terminated
+   *  string are known at compile time.
+   *  @param  __s  The string.
+   *
+   *  Assumes that _CharT is a built-in character type.
+   */
+  template<typename _CharT>
+    static _GLIBCXX_ALWAYS_INLINE constexpr bool
+    __constant_string_p(const _CharT* __s)
+    {
+      while (__builtin_constant_p(*__s) && *__s)
+	__s++;
+      return __builtin_constant_p(*__s);
+    }
+
+  /**
+   *  @brief Determine whether the characters of a character array are
+   *  known at compile time.
+   *  @param  __a  The character array.
+   *  @param  __n  Number of characters.
+   *
+   *  Assumes that _CharT is a built-in character type.
+   */
+  template<typename _CharT>
+    static _GLIBCXX_ALWAYS_INLINE constexpr bool
+    __constant_char_array_p(const _CharT* __a, size_t __n)
+    {
+      size_t __i = 0;
+      while (__builtin_constant_p(__a[__i]) && __i < __n)
+	__i++;
+      return __i == __n;
+    }
+#endif
+
   // 21.1
   /**
    *  @brief  Basis for explicit traits specializations.
@@ -256,21 +296,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		< static_cast<unsigned char>(__c2));
       }
 
-      static /* _GLIBCXX17_CONSTEXPR */ int
+      static _GLIBCXX17_CONSTEXPR int
       compare(const char_type* __s1, const char_type* __s2, size_t __n)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __constant_char_array_p(__s1, __n)
+	    && __constant_char_array_p(__s2, __n))
+	  return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n);
+#endif
 	if (__n == 0)
 	  return 0;
 	return __builtin_memcmp(__s1, __s2, __n);
       }
 
-      static /* _GLIBCXX17_CONSTEXPR */ size_t
+      static _GLIBCXX17_CONSTEXPR size_t
       length(const char_type* __s)
-      { return __builtin_strlen(__s); }
+      {
+#if __cplusplus > 201402
+	if (__constant_string_p(__s))
+	  return __gnu_cxx::char_traits<char_type>::length(__s);
+#endif
+	return __builtin_strlen(__s);
+      }
 
-      static /* _GLIBCXX17_CONSTEXPR */ const char_type*
+      static _GLIBCXX17_CONSTEXPR const char_type*
       find(const char_type* __s, size_t __n, const char_type& __a)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __builtin_constant_p(__a)
+	    && __constant_char_array_p(__s, __n))
+	  return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a);
+#endif
 	if (__n == 0)
 	  return 0;
 	return static_cast<const char_type*>(__builtin_memchr(__s, __a, __n));
@@ -292,9 +350,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return static_cast<char_type*>(__builtin_memcpy(__s1, __s2, __n));
       }
 
-      static char_type*
+      static _GLIBCXX17_CONSTEXPR char_type*
       assign(char_type* __s, size_t __n, char_type __a)
       {
+#if __cplusplus > 201402
+	if (__constant_string_p(__s)
+	    && __builtin_constant_p(__n)
+	    && __builtin_constant_p(__a))
+	  return __gnu_cxx::char_traits<char_type>::assign(__s,  __n, __a);
+#endif
 	if (__n == 0)
 	  return __s;
 	return static_cast<char_type*>(__builtin_memset(__s, __a, __n));
@@ -347,24 +411,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       lt(const char_type& __c1, const char_type& __c2) _GLIBCXX_NOEXCEPT
       { return __c1 < __c2; }
 
-      static /* _GLIBCXX17_CONSTEXPR */ int
+      static _GLIBCXX17_CONSTEXPR int
       compare(const char_type* __s1, const char_type* __s2, size_t __n)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __constant_char_array_p(__s1, __n)
+	    && __constant_char_array_p(__s2, __n))
+	  return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n);
+#endif
 	if (__n == 0)
 	  return 0;
-	return wmemcmp(__s1, __s2, __n);
+	else
+	  return wmemcmp(__s1, __s2, __n);
       }
 
-      static /* _GLIBCXX17_CONSTEXPR */ size_t
+      static _GLIBCXX17_CONSTEXPR size_t
       length(const char_type* __s)
-      { return wcslen(__s); }
+      {
+#if __cplusplus > 201402
+	if (__constant_string_p(__s))
+	  return __gnu_cxx::char_traits<char_type>::length(__s);
+	else
+#endif
+	  return wcslen(__s);
+      }
 
-      static /* _GLIBCXX17_CONSTEXPR */ const char_type*
+      static _GLIBCXX17_CONSTEXPR const char_type*
       find(const char_type* __s, size_t __n, const char_type& __a)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __builtin_constant_p(__a)
+	    && __constant_char_array_p(__s, __n))
+	  return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a);
+#endif
 	if (__n == 0)
 	  return 0;
-	return wmemchr(__s, __a, __n);
+	else
+	  return wmemchr(__s, __a, __n);
       }
 
       static char_type*
@@ -383,12 +468,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return wmemcpy(__s1, __s2, __n);
       }
 
-      static char_type*
+      static _GLIBCXX17_CONSTEXPR char_type*
       assign(char_type* __s, size_t __n, char_type __a)
       {
+#if __cplusplus > 201402
+	if (__constant_string_p(__s)
+	    && __builtin_constant_p(__n)
+	    && __builtin_constant_p(__a))
+	  return __gnu_cxx::char_traits<char_type>::assign(__s,  __n, __a);
+#endif
 	if (__n == 0)
 	  return __s;
-	return wmemset(__s, __a, __n);
+	else
+	  return wmemset(__s, __a, __n);
       }
 
       static _GLIBCXX_CONSTEXPR char_type
diff --git a/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc b/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
index 014caa0..efd280f 100644
--- a/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
+++ b/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
@@ -74,20 +74,20 @@ template<typename CT>
   }
 
 #ifndef __cpp_lib_constexpr_char_traits
-// #error Feature-test macro for constexpr char_traits is missing
+# error Feature-test macro for constexpr char_traits is missing
 #elif __cpp_lib_constexpr_char_traits != 201611
-// #error Feature-test macro for constexpr char_traits has the wrong value
+# error Feature-test macro for constexpr char_traits has the wrong value
 #endif
 
 static_assert( test_assign<std::char_traits<char>>() );
-// static_assert( test_compare<std::char_traits<char>>() );
-// static_assert( test_length<std::char_traits<char>>() );
-// static_assert( test_find<std::char_traits<char>>() );
+static_assert( test_compare<std::char_traits<char>>() );
+static_assert( test_length<std::char_traits<char>>() );
+static_assert( test_find<std::char_traits<char>>() );
 #ifdef _GLIBCXX_USE_WCHAR_T
 static_assert( test_assign<std::char_traits<wchar_t>>() );
-// static_assert( test_compare<std::char_traits<wchar_t>>() );
-// static_assert( test_length<std::char_traits<wchar_t>>() );
-// static_assert( test_find<std::char_traits<wchar_t>>() );
+static_assert( test_compare<std::char_traits<wchar_t>>() );
+static_assert( test_length<std::char_traits<wchar_t>>() );
+static_assert( test_find<std::char_traits<wchar_t>>() );
 #endif
 static_assert( test_assign<std::char_traits<char16_t>>() );
 static_assert( test_compare<std::char_traits<char16_t>>() );
-- 
2.5.5

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

* Re: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17
  2017-04-23 18:54 [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17 Pedro Alves
@ 2017-04-23 21:38 ` Pedro Alves
  2017-06-05 14:27   ` Jonathan Wakely
  0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2017-04-23 21:38 UTC (permalink / raw)
  To: Pedro Alves, Jonathan Wakely, libstdc++, gcc-patches

On 04/23/2017 06:54 PM, Pedro Alves wrote:
> Hi!
> 
> As I had suggested in PR c++/80265, here's a patch that uses
> __builtin_constant_p to tell whether we can defer to a constexpr
> algorithm, which avoids having to wait for compiler support.
> 
> Unfortunately I ran out of cycles today to run a full bootstrap/regtest
> cycle, but constexpr_functions_c++17.cc passes cleanly on
> x86_64 GNU/Linux at least.
> 
> If this looks like a reasonable approach, I can maybe try running
> full tests on the gcc compile farm next week.
> 
> WDYT?

Reading the patch via the list made me spot something
that was incorrect.

>  
> -      static char_type*
> +      static _GLIBCXX17_CONSTEXPR char_type*
>        assign(char_type* __s, size_t __n, char_type __a)
>        {
> +#if __cplusplus > 201402
> +	if (__constant_string_p(__s)

This should probably have been __constant_char_array_p, but now
that I look again at the paper, I see I got myself confused -- this
assign overload is not meant to be constexpr in the first place.

So here's an updated patch that drops that bit.  (Same testing
as before.)

From 71cf9b9a3ea79bd790d792c9a02e5a577f1af156 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sun, 23 Apr 2017 14:16:09 +0100
Subject: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits"
 for C++17

As discussed in PR c++/80265 (__builtin_{memcmp,memchr,strlen} are not
usable in constexpr functions), use __builtin_constant_p to tell
whether we can defer to a constexpr algorithm.

I used __always_inline__ just to be thorough.  It isn't really really
necessary as far as I could determine.

Changes like these:

	 if (__n == 0)
	   return 0;
 -	return wmemcmp(__s1, __s2, __n);
 +	else
 +	  return wmemcmp(__s1, __s2, __n);

are necessary otherwise G++ complains that we're calling a
non-constexpr function, which looks like a a manifestation of PR67026
to me.

libstdc++-v3:
2017-04-23  Pedro Alves  <palves@redhat.com>

	* doc/xml/manual/status_cxx2017.xml: Update C++17 constexpr
	char_traits status.
	* doc/html/*: Regenerate.

	* include/bits/char_traits.h (_GLIBCXX_ALWAYS_INLINE): Define if
	not already defined.
	(__cpp_lib_constexpr_char_traits): Uncomment.
	(__constant_string_p, __constant_char_array_p): New.
	(std::char_traits<char>, std::char_traits<wchar_t>): Add
	_GLIBCXX17_CONSTEXPR on compare, length and find and use
	__constant_string_p, __constant_char_array_p and
	__builtin_constant_p to defer to __gnu_cxx::char_traits at compile
	time.

	* testsuite/21_strings/char_traits/requirements/
	constexpr_functions_c++17.cc: Uncomment
	__cpp_lib_constexpr_char_traits tests.  Uncomment
	test_compare<char>, test_length<char>, test_find<char>,
	test_compare<wchar_t>, test_length<wchar_t> and test_find<wchar_t>
	static_assert tests.
---
 libstdc++-v3/doc/xml/manual/status_cxx2017.xml     |   4 +-
 libstdc++-v3/include/bits/char_traits.h            | 101 ++++++++++++++++++---
 .../requirements/constexpr_functions_c++17.cc      |  16 ++--
 3 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
index 0e35f75..fed91f9 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
@@ -489,8 +489,8 @@ Feature-testing recommendations for C++</link>.
 	P0426R1
 	</link>
       </entry>
-      <entry align="center"> 7 (partial) </entry>
-      <entry><code> ??? </code></entry>
+      <entry align="center"> 7 </entry>
+      <entry><code> __cpp_lib_constexpr_char_traits >= 201611 </code></entry>
     </row>
 
     <row>
diff --git a/libstdc++-v3/include/bits/char_traits.h b/libstdc++-v3/include/bits/char_traits.h
index 75db5b8..3ecc30e 100644
--- a/libstdc++-v3/include/bits/char_traits.h
+++ b/libstdc++-v3/include/bits/char_traits.h
@@ -40,6 +40,10 @@
 #include <bits/postypes.h>      // For streampos
 #include <cwchar>               // For WEOF, wmemmove, wmemset, etc.
 
+#ifndef _GLIBCXX_ALWAYS_INLINE
+#define _GLIBCXX_ALWAYS_INLINE inline __attribute__((__always_inline__))
+#endif
+
 namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -139,7 +143,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       { return !eq_int_type(__c, eof()) ? __c : to_int_type(char_type()); }
     };
 
-// #define __cpp_lib_constexpr_char_traits 201611
+#define __cpp_lib_constexpr_char_traits 201611
 
   template<typename _CharT>
     _GLIBCXX14_CONSTEXPR int
@@ -212,6 +216,42 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+#if __cplusplus > 201402
+  /**
+   *  @brief Determine whether the characters of a NULL-terminated
+   *  string are known at compile time.
+   *  @param  __s  The string.
+   *
+   *  Assumes that _CharT is a built-in character type.
+   */
+  template<typename _CharT>
+    static _GLIBCXX_ALWAYS_INLINE constexpr bool
+    __constant_string_p(const _CharT* __s)
+    {
+      while (__builtin_constant_p(*__s) && *__s)
+	__s++;
+      return __builtin_constant_p(*__s);
+    }
+
+  /**
+   *  @brief Determine whether the characters of a character array are
+   *  known at compile time.
+   *  @param  __a  The character array.
+   *  @param  __n  Number of characters.
+   *
+   *  Assumes that _CharT is a built-in character type.
+   */
+  template<typename _CharT>
+    static _GLIBCXX_ALWAYS_INLINE constexpr bool
+    __constant_char_array_p(const _CharT* __a, size_t __n)
+    {
+      size_t __i = 0;
+      while (__builtin_constant_p(__a[__i]) && __i < __n)
+	__i++;
+      return __i == __n;
+    }
+#endif
+
   // 21.1
   /**
    *  @brief  Basis for explicit traits specializations.
@@ -256,21 +296,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		< static_cast<unsigned char>(__c2));
       }
 
-      static /* _GLIBCXX17_CONSTEXPR */ int
+      static _GLIBCXX17_CONSTEXPR int
       compare(const char_type* __s1, const char_type* __s2, size_t __n)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __constant_char_array_p(__s1, __n)
+	    && __constant_char_array_p(__s2, __n))
+	  return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n);
+#endif
 	if (__n == 0)
 	  return 0;
 	return __builtin_memcmp(__s1, __s2, __n);
       }
 
-      static /* _GLIBCXX17_CONSTEXPR */ size_t
+      static _GLIBCXX17_CONSTEXPR size_t
       length(const char_type* __s)
-      { return __builtin_strlen(__s); }
+      {
+#if __cplusplus > 201402
+	if (__constant_string_p(__s))
+	  return __gnu_cxx::char_traits<char_type>::length(__s);
+#endif
+	return __builtin_strlen(__s);
+      }
 
-      static /* _GLIBCXX17_CONSTEXPR */ const char_type*
+      static _GLIBCXX17_CONSTEXPR const char_type*
       find(const char_type* __s, size_t __n, const char_type& __a)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __builtin_constant_p(__a)
+	    && __constant_char_array_p(__s, __n))
+	  return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a);
+#endif
 	if (__n == 0)
 	  return 0;
 	return static_cast<const char_type*>(__builtin_memchr(__s, __a, __n));
@@ -347,24 +405,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       lt(const char_type& __c1, const char_type& __c2) _GLIBCXX_NOEXCEPT
       { return __c1 < __c2; }
 
-      static /* _GLIBCXX17_CONSTEXPR */ int
+      static _GLIBCXX17_CONSTEXPR int
       compare(const char_type* __s1, const char_type* __s2, size_t __n)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __constant_char_array_p(__s1, __n)
+	    && __constant_char_array_p(__s2, __n))
+	  return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n);
+#endif
 	if (__n == 0)
 	  return 0;
-	return wmemcmp(__s1, __s2, __n);
+	else
+	  return wmemcmp(__s1, __s2, __n);
       }
 
-      static /* _GLIBCXX17_CONSTEXPR */ size_t
+      static _GLIBCXX17_CONSTEXPR size_t
       length(const char_type* __s)
-      { return wcslen(__s); }
+      {
+#if __cplusplus > 201402
+	if (__constant_string_p(__s))
+	  return __gnu_cxx::char_traits<char_type>::length(__s);
+	else
+#endif
+	  return wcslen(__s);
+      }
 
-      static /* _GLIBCXX17_CONSTEXPR */ const char_type*
+      static _GLIBCXX17_CONSTEXPR const char_type*
       find(const char_type* __s, size_t __n, const char_type& __a)
       {
+#if __cplusplus > 201402
+	if (__builtin_constant_p(__n)
+	    && __builtin_constant_p(__a)
+	    && __constant_char_array_p(__s, __n))
+	  return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a);
+#endif
 	if (__n == 0)
 	  return 0;
-	return wmemchr(__s, __a, __n);
+	else
+	  return wmemchr(__s, __a, __n);
       }
 
       static char_type*
diff --git a/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc b/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
index 014caa0..efd280f 100644
--- a/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
+++ b/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
@@ -74,20 +74,20 @@ template<typename CT>
   }
 
 #ifndef __cpp_lib_constexpr_char_traits
-// #error Feature-test macro for constexpr char_traits is missing
+# error Feature-test macro for constexpr char_traits is missing
 #elif __cpp_lib_constexpr_char_traits != 201611
-// #error Feature-test macro for constexpr char_traits has the wrong value
+# error Feature-test macro for constexpr char_traits has the wrong value
 #endif
 
 static_assert( test_assign<std::char_traits<char>>() );
-// static_assert( test_compare<std::char_traits<char>>() );
-// static_assert( test_length<std::char_traits<char>>() );
-// static_assert( test_find<std::char_traits<char>>() );
+static_assert( test_compare<std::char_traits<char>>() );
+static_assert( test_length<std::char_traits<char>>() );
+static_assert( test_find<std::char_traits<char>>() );
 #ifdef _GLIBCXX_USE_WCHAR_T
 static_assert( test_assign<std::char_traits<wchar_t>>() );
-// static_assert( test_compare<std::char_traits<wchar_t>>() );
-// static_assert( test_length<std::char_traits<wchar_t>>() );
-// static_assert( test_find<std::char_traits<wchar_t>>() );
+static_assert( test_compare<std::char_traits<wchar_t>>() );
+static_assert( test_length<std::char_traits<wchar_t>>() );
+static_assert( test_find<std::char_traits<wchar_t>>() );
 #endif
 static_assert( test_assign<std::char_traits<char16_t>>() );
 static_assert( test_compare<std::char_traits<char16_t>>() );
-- 
2.5.5




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

* Re: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17
  2017-04-23 21:38 ` Pedro Alves
@ 2017-06-05 14:27   ` Jonathan Wakely
  2017-06-12 22:28     ` Pedro Alves
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2017-06-05 14:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: libstdc++, gcc-patches

On 23/04/17 19:33 +0100, Pedro Alves wrote:
>On 04/23/2017 06:54 PM, Pedro Alves wrote:
>> Hi!
>>
>> As I had suggested in PR c++/80265, here's a patch that uses
>> __builtin_constant_p to tell whether we can defer to a constexpr
>> algorithm, which avoids having to wait for compiler support.
>>
>> Unfortunately I ran out of cycles today to run a full bootstrap/regtest
>> cycle, but constexpr_functions_c++17.cc passes cleanly on
>> x86_64 GNU/Linux at least.
>>
>> If this looks like a reasonable approach, I can maybe try running
>> full tests on the gcc compile farm next week.
>>
>> WDYT?
>
>Reading the patch via the list made me spot something
>that was incorrect.
>
>>
>> -      static char_type*
>> +      static _GLIBCXX17_CONSTEXPR char_type*
>>        assign(char_type* __s, size_t __n, char_type __a)
>>        {
>> +#if __cplusplus > 201402
>> +	if (__constant_string_p(__s)
>
>This should probably have been __constant_char_array_p, but now
>that I look again at the paper, I see I got myself confused -- this
>assign overload is not meant to be constexpr in the first place.
>
>So here's an updated patch that drops that bit.  (Same testing
>as before.)

Pedro, this is OK for trunk now we're in stage 1. Please go ahead and
commit it - thanks.

It's probably safe for gcc-7-branch too, but let's leave it on trunk
for a while first.


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

* Re: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17
  2017-06-05 14:27   ` Jonathan Wakely
@ 2017-06-12 22:28     ` Pedro Alves
  2017-06-13 15:57       ` Jonathan Wakely
  0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2017-06-12 22:28 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 06/05/2017 03:27 PM, Jonathan Wakely wrote:

> Pedro, this is OK for trunk now we're in stage 1. Please go ahead and
> commit it - thanks.

Thanks Jonathan.  I've pushed it in now.

> 
> It's probably safe for gcc-7-branch too, but let's leave it on trunk
> for a while first.

OK.

BTW, for extra thoroughness, to confirm we're handling both
const & non-const arrays correctly, I had written this testsuite
tweak too.  Would you like to have this in?

From 3f7adab8bab68955aafd760467bb860057140d40 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 12 Jun 2017 20:23:23 +0100
Subject: [PATCH] constexpr char_traits: Test non-const strings/arrays too

libstdc++-v3/ChangeLog
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
	(test_assign, test_compare, test_length, test_find): Test
	non-const strings/arrays too.
	(struct C): Add a generic conversion ctor.
---
 .../requirements/constexpr_functions_c++17.cc      | 173 ++++++++++++++++++---
 1 file changed, 150 insertions(+), 23 deletions(-)

diff --git a/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc b/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
index efd280f..c41b490 100644
--- a/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
+++ b/libstdc++-v3/testsuite/21_strings/char_traits/requirements/constexpr_functions_c++17.cc
@@ -25,10 +25,40 @@ template<typename CT>
   test_assign()
   {
     using char_type = typename CT::char_type;
-    char_type s1[2] = {};
-    const char_type s2[2] = {1, 0};
-    CT::assign(s1[0], s2[0]);
-    return s1[0] == char_type{1};
+
+    auto check = [](char_type* s1, const char_type* s2)
+      {
+	CT::assign(s1[0], s2[0]);
+	return s1[0] == char_type{1};
+      };
+
+    // const strings.
+
+    {
+      char_type s1[2] = {};
+      const char_type s2[2] = {1, 0};
+      if (!check (s1, s2))
+	return false;
+    }
+
+    // non-const strings.
+
+    {
+      char_type s1[2] = {};
+      char_type s2[2] = {1, 0};
+      if (!check (s1, s2))
+	return false;
+    }
+
+    {
+      char_type s1[2] = {};
+      char_type s2[2] = {};
+      s2[0] = char_type{1};
+      if (!check (s1, s2))
+	return false;
+    }
+
+    return true;
   }
 
 template<typename CT>
@@ -36,14 +66,48 @@ template<typename CT>
   test_compare()
   {
     using char_type = typename CT::char_type;
-    const char_type s1[3] = {1, 2, 3};
-    const char_type s2[3] = {1, 2, 3};
-    if (CT::compare(s1, s2, 3) != 0)
-      return false;
-    if (CT::compare(s2, s1, 3) != 0)
-      return false;
-    if (CT::compare(s1+1, s2, 2) <= 0)
-      return false;
+
+    auto check = [](const char_type* s1, const char_type* s2)
+      {
+	if (CT::compare(s1, s2, 3) != 0)
+	  return false;
+	if (CT::compare(s2, s1, 3) != 0)
+	  return false;
+	if (CT::compare(s1+1, s2, 2) <= 0)
+	  return false;
+	return true;
+      };
+
+    // const arrays.
+
+    {
+      const char_type s1[3] = {1, 2, 3};
+      const char_type s2[3] = {1, 2, 3};
+      if (!check (s1, s2))
+	return false;
+    }
+
+    // non-const arrays.
+
+    {
+      char_type s1[3] = {1, 2, 3};
+      char_type s2[3] = {1, 2, 3};
+      if (!check (s1, s2))
+	return false;
+    }
+
+    {
+      char_type s1[3] = {};
+      char_type s2[3] = {};
+      for (size_t i = 0; i < 3; i++)
+	{
+	  s1[i] = char_type(i+1);
+	  s2[i] = char_type(i+1);
+	}
+      if (!check (s1, s2))
+	return false;
+    }
+
     return true;
   }
 
@@ -52,11 +116,40 @@ template<typename CT>
   test_length()
   {
     using char_type = typename CT::char_type;
-    const char_type s1[4] = {1, 2, 3, 0};
-    if (CT::length(s1) != 3)
-      return false;
-    if (CT::length(s1+1) != 2)
-      return false;
+
+    auto check = [](const char_type* s)
+      {
+	if (CT::length(s) != 3)
+	  return false;
+	if (CT::length(s+1) != 2)
+	  return false;
+	return true;
+      };
+
+    // const strings.
+
+    {
+      const char_type s[4] = {1, 2, 3, 0};
+      if (!check (s))
+	return false;
+    }
+
+    // non-const strings.
+
+    {
+      char_type s[4] = {1, 2, 3, 0};
+      if (!check (s))
+	return false;
+    }
+
+    {
+      char_type s[4] = {};
+      for (size_t i = 0; i < 3; i++)
+	s[i] = char_type(i+1);
+      if (!check (s))
+	return false;
+    }
+
     return true;
   }
 
@@ -65,11 +158,40 @@ template<typename CT>
   test_find()
   {
     using char_type = typename CT::char_type;
-    const char_type s1[3] = {1, 2, 3};
-    if (CT::find(s1, 3, char_type{2}) != s1+1)
-      return false;
-    if (CT::find(s1, 3, char_type{4}) != nullptr)
-      return false;
+
+    auto check = [](const char_type* s)
+      {
+	if (CT::find(s, 3, char_type{2}) != s+1)
+	  return false;
+	if (CT::find(s, 3, char_type{4}) != nullptr)
+	  return false;
+	return true;
+      };
+
+    // const arrays.
+
+    {
+      const char_type s[3] = {1, 2, 3};
+      if (!check(s))
+	return false;
+    }
+
+    // non-const arrays.
+
+    {
+      char_type s[3] = {1, 2, 3};
+      if (!check(s))
+	return false;
+    }
+
+    {
+      char_type s[3] = {};
+      for (size_t i = 0; i < 3; i++)
+	s[i] = char_type(i+1);
+      if (!check(s))
+	return false;
+    }
+
     return true;
   }
 
@@ -98,7 +220,12 @@ static_assert( test_compare<std::char_traits<char32_t>>() );
 static_assert( test_length<std::char_traits<char32_t>>() );
 static_assert( test_find<std::char_traits<char32_t>>() );
 
-struct C { unsigned char c; };
+struct C
+{
+  C() = default;
+  constexpr C(auto c_) : c(c_) {}
+  unsigned char c;
+};
 constexpr bool operator==(const C& c1, const C& c2) { return c1.c == c2.c; }
 constexpr bool operator<(const C& c1, const C& c2) { return c1.c < c2.c; }
 static_assert( test_assign<std::char_traits<C>>() );
-- 
2.5.5


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

* Re: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17
  2017-06-12 22:28     ` Pedro Alves
@ 2017-06-13 15:57       ` Jonathan Wakely
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2017-06-13 15:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: libstdc++, gcc-patches

On 12/06/17 23:28 +0100, Pedro Alves wrote:
>On 06/05/2017 03:27 PM, Jonathan Wakely wrote:
>
>> Pedro, this is OK for trunk now we're in stage 1. Please go ahead and
>> commit it - thanks.
>
>Thanks Jonathan.  I've pushed it in now.
>
>>
>> It's probably safe for gcc-7-branch too, but let's leave it on trunk
>> for a while first.
>
>OK.
>
>BTW, for extra thoroughness, to confirm we're handling both
>const & non-const arrays correctly, I had written this testsuite
>tweak too.  Would you like to have this in?

Yes please, this looks useful.


>@@ -98,7 +220,12 @@ static_assert( test_compare<std::char_traits<char32_t>>() );
> static_assert( test_length<std::char_traits<char32_t>>() );
> static_assert( test_find<std::char_traits<char32_t>>() );
>
>-struct C { unsigned char c; };
>+struct C
>+{
>+  C() = default;
>+  constexpr C(auto c_) : c(c_) {}

Placeholder types as function parameters are non-standard, so this
would fail with -pedantic-errors.

How about:

struct C {
  constexpr C(unsigned char c_ = 0) : c(c_) { }
  unsigned char c;
};


>+  unsigned char c;
>+};
> constexpr bool operator==(const C& c1, const C& c2) { return c1.c == c2.c; }
> constexpr bool operator<(const C& c1, const C& c2) { return c1.c < c2.c; }
> static_assert( test_assign<std::char_traits<C>>() );
>-- 
>2.5.5
>
>

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

end of thread, other threads:[~2017-06-13 15:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-23 18:54 [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17 Pedro Alves
2017-04-23 21:38 ` Pedro Alves
2017-06-05 14:27   ` Jonathan Wakely
2017-06-12 22:28     ` Pedro Alves
2017-06-13 15:57       ` Jonathan Wakely

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).