public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Help compiler detect invalid code
@ 2019-09-19 20:28 François Dumont
  2019-09-20  5:08 ` François Dumont
  2019-09-27 12:11 ` Jonathan Wakely
  0 siblings, 2 replies; 15+ messages in thread
From: François Dumont @ 2019-09-19 20:28 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

Hi

     I start working on making recently added constexpr tests to work in 
Debug mode.

     It appears that the compiler is able to detect code mistakes pretty 
well as long we don't try to hide the code intention with a defensive 
approach. This is why I'd like to propose to replace '__n > 0' 
conditions with '__n != 0'.

     The result is demonstrated by the constexpr_neg.cc tests. What do 
you think ?

     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
     (__memmove): Loop as long as __n is not 0.
     (__copy_move<>::__copy_m): Likewise.
     (__copy_move_backward<>::__copy_move_b): Likewise.
     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.

     I'll submit the patch to fix Debug mode depending on the decision 
for this one.

François


[-- Attachment #2: constexpr_algo.patch --]
[-- Type: text/x-patch, Size: 8214 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 4eba053ac75..33593197b4f 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -83,13 +83,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    */
   template<bool _IsMove, typename _Tp>
     _GLIBCXX14_CONSTEXPR
-    inline void*
-    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    inline _Tp*
+    __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
     {
 #ifdef __cpp_lib_is_constant_evaluated
       if (std::is_constant_evaluated())
 	{
-	  for(; __num > 0; --__num)
+	  for (; __num != 0; --__num)
 	    {
 	      if constexpr (_IsMove)
 		*__dst = std::move(*__src);
@@ -100,10 +100,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    }
 	  return __dst;
 	}
-      else
 #endif
-	return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
-      return __dst;
+      return static_cast<_Tp*>(
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num));
     }
 
   /*
@@ -398,7 +397,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__copy_m(_II __first, _II __last, _OI __result)
 	{
 	  typedef typename iterator_traits<_II>::difference_type _Distance;
-	  for(_Distance __n = __last - __first; __n > 0; --__n)
+	  for (_Distance __n = __last - __first; __n != 0; --__n)
 	    {
 	      *__result = *__first;
 	      ++__first;
@@ -418,7 +417,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__copy_m(_II __first, _II __last, _OI __result)
 	{
 	  typedef typename iterator_traits<_II>::difference_type _Distance;
-	  for(_Distance __n = __last - __first; __n > 0; --__n)
+	  for (_Distance __n = __last - __first; __n != 0; --__n)
 	    {
 	      *__result = std::move(*__first);
 	      ++__first;
@@ -446,8 +445,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 	  const ptrdiff_t _Num = __last - __first;
 	  if (_Num)
-	    std::__memmove<_IsMove>(__result, __first, _Num);
-	  return __result + _Num;
+	    return std::__memmove<_IsMove>(__result, __first, _Num);
+	  return __result;
 	}
     };
 
@@ -671,7 +670,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 	{
 	  typename iterator_traits<_BI1>::difference_type
 	    __n = __last - __first;
-	  for (; __n > 0; --__n)
+	  for (; __n != 0; --__n)
 	    *--__result = *--__last;
 	  return __result;
 	}
@@ -688,7 +687,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 	{
 	  typename iterator_traits<_BI1>::difference_type
 	    __n = __last - __first;
-	  for (; __n > 0; --__n)
+	  for (; __n != 0; --__n)
 	    *--__result = std::move(*--__last);
 	  return __result;
 	}
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
index 67910b8773e..a0460603496 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
@@ -24,12 +24,12 @@
 constexpr bool
 test()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
   const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
 
-  return out6 == ma0.begin() + 10;
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
 }
 
 static_assert(test());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
new file mode 100644
index 00000000000..34e20be97eb
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
@@ -0,0 +1,50 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test1()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test1()); // { dg-error "outside the bounds" }
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out6 = std::copy(ma0.begin(), ma0.begin() + 8, ma0.begin() + 2);
+
+  // Check what should be the result if the range didn't overlap.
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2()); // { dg-error "static assertion failed" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
index ed7487905a8..c0e1a747832 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
@@ -22,15 +22,27 @@
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
 
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
   const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8,
 				       ma0.begin() + 10);
 
-  return true;
+  return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8,
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
new file mode 100644
index 00000000000..3dc595d239f
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(),
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-19 20:28 [PATCH] Help compiler detect invalid code François Dumont
@ 2019-09-20  5:08 ` François Dumont
  2019-09-25 20:40   ` François Dumont
  2019-09-27 11:24   ` Jonathan Wakely
  2019-09-27 12:11 ` Jonathan Wakely
  1 sibling, 2 replies; 15+ messages in thread
From: François Dumont @ 2019-09-20  5:08 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

I already realized that previous patch will be too controversial to be 
accepted.

In this new version I just implement a real memmove in __memmove so that 
in copy_backward there is no need for a shortcut to a more defensive code.

I'll see if in Debug mode I can do something.

François


On 9/19/19 10:27 PM, François Dumont wrote:
> Hi
>
>     I start working on making recently added constexpr tests to work 
> in Debug mode.
>
>     It appears that the compiler is able to detect code mistakes 
> pretty well as long we don't try to hide the code intention with a 
> defensive approach. This is why I'd like to propose to replace '__n > 
> 0' conditions with '__n != 0'.
>
>     The result is demonstrated by the constexpr_neg.cc tests. What do 
> you think ?
>
>     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
>     (__memmove): Loop as long as __n is not 0.
>     (__copy_move<>::__copy_m): Likewise.
>     (__copy_move_backward<>::__copy_move_b): Likewise.
>     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
> values.
>     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
>     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
>     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
>
>     I'll submit the patch to fix Debug mode depending on the decision 
> for this one.
>
> François
>


[-- Attachment #2: constexpr_algo.patch --]
[-- Type: text/x-patch, Size: 7195 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 4eba053ac75..94a79b85d15 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -83,27 +83,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    */
   template<bool _IsMove, typename _Tp>
     _GLIBCXX14_CONSTEXPR
-    inline void*
-    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    inline void
+    __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
     {
 #ifdef __cpp_lib_is_constant_evaluated
       if (std::is_constant_evaluated())
 	{
-	  for(; __num > 0; --__num)
+	  __dst += __num;
+	  __src += __num;
+	  for (; __num != 0; --__num)
 	    {
 	      if constexpr (_IsMove)
-		*__dst = std::move(*__src);
+		*--__dst = std::move(*--__src);
 	      else
-		*__dst = *__src;
-	      ++__src;
-	      ++__dst;
+		*--__dst = *--__src;
 	    }
-	  return __dst;
 	}
       else
 #endif
-	return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
-      return __dst;
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
     }
 
   /*
@@ -730,12 +728,6 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 			     && __is_pointer<_BI2>::__value
 			     && __are_same<_ValueType1, _ValueType2>::__value);
 
-#ifdef __cpp_lib_is_constant_evaluated
-      if (std::is_constant_evaluated())
-	return std::__copy_move_backward<true, false,
-				 _Category>::__copy_move_b(__first, __last,
-							   __result);
-#endif
       return std::__copy_move_backward<_IsMove, __simple,
 				       _Category>::__copy_move_b(__first,
 								 __last,
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
index 67910b8773e..fc70db6dae7 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
@@ -22,14 +22,25 @@
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
   const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
 
-  return out6 == ma0.begin() + 10;
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out6 = std::copy(ma0.begin(), ma0.begin() + 8, ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
new file mode 100644
index 00000000000..49052467409
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
index ed7487905a8..c0e1a747832 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
@@ -22,15 +22,27 @@
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
 
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
   const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8,
 				       ma0.begin() + 10);
 
-  return true;
+  return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8,
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
new file mode 100644
index 00000000000..3dc595d239f
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(),
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-20  5:08 ` François Dumont
@ 2019-09-25 20:40   ` François Dumont
  2019-09-27 11:24   ` Jonathan Wakely
  1 sibling, 0 replies; 15+ messages in thread
From: François Dumont @ 2019-09-25 20:40 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

Some more tests have revealed  a small problem in is_sorted test. It was 
revealed by the Debug mode but not in a clean ways so for the moment I 
prefer to fix it and I'll add a _neg test when Debug is able to report 
it correctly.

I've also added a _neg test for equal which doesn't need Debug mode.

OK to commit ?

François



On 9/20/19 7:08 AM, François Dumont wrote:
> I already realized that previous patch will be too controversial to be 
> accepted.
>
> In this new version I just implement a real memmove in __memmove so 
> that in copy_backward there is no need for a shortcut to a more 
> defensive code.
>
> I'll see if in Debug mode I can do something.
>
> François
>
>
> On 9/19/19 10:27 PM, François Dumont wrote:
>> Hi
>>
>>     I start working on making recently added constexpr tests to work 
>> in Debug mode.
>>
>>     It appears that the compiler is able to detect code mistakes 
>> pretty well as long we don't try to hide the code intention with a 
>> defensive approach. This is why I'd like to propose to replace '__n > 
>> 0' conditions with '__n != 0'.
>>
>>     The result is demonstrated by the constexpr_neg.cc tests. What do 
>> you think ?
>>
>>     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
>>     (__memmove): Loop as long as __n is not 0.
>>     (__copy_move<>::__copy_m): Likewise.
>>     (__copy_move_backward<>::__copy_move_b): Likewise.
>>     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
>> values.
>>     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
>>     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
>>     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
>>
>>     I'll submit the patch to fix Debug mode depending on the decision 
>> for this one.
>>
>> François
>>
>


[-- Attachment #2: constexpr_algo.patch --]
[-- Type: text/x-patch, Size: 9443 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 4eba053ac75..94a79b85d15 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -83,27 +83,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    */
   template<bool _IsMove, typename _Tp>
     _GLIBCXX14_CONSTEXPR
-    inline void*
-    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    inline void
+    __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
     {
 #ifdef __cpp_lib_is_constant_evaluated
       if (std::is_constant_evaluated())
 	{
-	  for(; __num > 0; --__num)
+	  __dst += __num;
+	  __src += __num;
+	  for (; __num != 0; --__num)
 	    {
 	      if constexpr (_IsMove)
-		*__dst = std::move(*__src);
+		*--__dst = std::move(*--__src);
 	      else
-		*__dst = *__src;
-	      ++__src;
-	      ++__dst;
+		*--__dst = *--__src;
 	    }
-	  return __dst;
 	}
       else
 #endif
-	return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
-      return __dst;
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
     }
 
   /*
@@ -730,12 +728,6 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 			     && __is_pointer<_BI2>::__value
 			     && __are_same<_ValueType1, _ValueType2>::__value);
 
-#ifdef __cpp_lib_is_constant_evaluated
-      if (std::is_constant_evaluated())
-	return std::__copy_move_backward<true, false,
-				 _Category>::__copy_move_b(__first, __last,
-							   __result);
-#endif
       return std::__copy_move_backward<_IsMove, __simple,
 				       _Category>::__copy_move_b(__first,
 								 __last,
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
index 67910b8773e..a0460603496 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
@@ -24,12 +24,12 @@
 constexpr bool
 test()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
   const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
 
-  return out6 == ma0.begin() + 10;
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
 }
 
 static_assert(test());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
new file mode 100644
index 00000000000..49052467409
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
index ed7487905a8..c0e1a747832 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
@@ -22,15 +22,27 @@
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
 
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
   const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8,
 				       ma0.begin() + 10);
 
-  return true;
+  return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8,
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
new file mode 100644
index 00000000000..3dc595d239f
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(),
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/equal/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/equal/constexpr_neg.cc
new file mode 100644
index 00000000000..065ca1e89ff
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/equal/constexpr_neg.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test01()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+
+  const auto outa = std::equal(ca0.end(), ca0.begin(), ca1.begin());
+  return outa;
+}
+
+static_assert(test01()); // { dg-error "outside the bounds" }
+
+constexpr bool
+test02()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 11> ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}};
+
+  const auto outa = std::equal(ca0.begin(), ca0.end(), ca1.begin());
+  return outa;
+}
+
+static_assert(test02()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
index 0be2f5fed62..f549b3d9307 100644
--- a/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
@@ -26,7 +26,7 @@ constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
 constexpr auto outv = std::is_sorted(ca0.begin(), ca0.end());
 
 constexpr auto outw = std::is_sorted(ca0.begin(), ca0.end(),
-				     std::equal_to<int>());
+				     std::less<int>());
 
 constexpr bool
 test()

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-20  5:08 ` François Dumont
  2019-09-25 20:40   ` François Dumont
@ 2019-09-27 11:24   ` Jonathan Wakely
  2019-10-01 20:05     ` François Dumont
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-09-27 11:24 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 20/09/19 07:08 +0200, François Dumont wrote:
>I already realized that previous patch will be too controversial to be 
>accepted.
>
>In this new version I just implement a real memmove in __memmove so 

A real memmove doesn't just work backwards, it needs to detect any
overlaps and work forwards *or* backwards, as needed.

I think your change will break this case:

#include <algorithm>

constexpr int f(int i, int j, int k)
{
  int arr[5] = { 0, 0, i, j, k };
  std::move(arr+2, arr+5, arr);
  return arr[0] + arr[1] + arr[2];
}

static_assert( f(1, 2, 3) == 6 );

This is valid because std::move only requires that the result iterator
is not in the input range, but it's OK for the two ranges to overlap.

I haven't tested it, but I think with your change the array will end
up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-19 20:28 [PATCH] Help compiler detect invalid code François Dumont
  2019-09-20  5:08 ` François Dumont
@ 2019-09-27 12:11 ` Jonathan Wakely
  2019-09-27 16:24   ` François Dumont
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-09-27 12:11 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

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

On 19/09/19 22:27 +0200, François Dumont wrote:
>Hi
>
>    I start working on making recently added constexpr tests to work 
>in Debug mode.

The attached patch seems to be necessary for that, right?



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 21319 bytes --]

commit c47326afadb0c330c19b872d5969d2f656499d0a
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Sep 26 15:44:04 2019 +0100

    Make debug mode checks work in C++20 constexpr algorithms
    
            * include/debug/formatter.h (__check_singular): Add
            _GLIBCXX20_CONSTEXPR.
            * include/debug/functions.h (__check_singular_aux, __check_singular)
            (__check_valid_range, __foreign_iterator_aux4, __foreign_iterator_aux3)
            (__foreign_iterator_aux2, __foreign_iterator_aux, __foreign_iterator)
            (__check_sorted_aux, __check_sorted, __check_sorted_set_aux)
            __check_sorted_set, __check_partitioned_lower)
            (__check_partitioned_upper, _Irreflexive_checker::_S_is_valid)
            (_Irreflexive_checker::_S_is_valid_pred, __is_reflexive)
            (__is_reflexive_pred): Likewise.
            * include/debug/helper_functions.h (__get_distance, __valid_range_aux)
            (__valid_range, __can_advance, __base, __unsafe): Likewise.
            * include/debug/safe_iterator.h (__valid_range, __can_advance)
            (__base, __unsafe): Likewise.
            * include/debug/safe_local_iterator.h (__valid_range, __can_advance)
            (__unsafe): Likewise.

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index 220379994c0..6f17ab4627c 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -72,7 +72,7 @@ namespace __gnu_debug
   using std::type_info;
 
   template<typename _Iterator>
-    bool __check_singular(const _Iterator&);
+    _GLIBCXX20_CONSTEXPR bool __check_singular(const _Iterator&);
 
   class _Safe_sequence_base;
 
diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h
index f87838692c6..bbfbe15fefa 100644
--- a/libstdc++-v3/include/debug/functions.h
+++ b/libstdc++-v3/include/debug/functions.h
@@ -50,19 +50,19 @@ namespace __gnu_debug
     struct _Is_contiguous_sequence : std::__false_type { };
 
   // An arbitrary iterator pointer is not singular.
-  inline bool
+  _GLIBCXX20_CONSTEXPR inline bool
   __check_singular_aux(const void*) { return false; }
 
   // We may have an iterator that derives from _Safe_iterator_base but isn't
   // a _Safe_iterator.
   template<typename _Iterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_singular(const _Iterator& __x)
     { return __check_singular_aux(std::__addressof(__x)); }
 
   /** Non-NULL pointers are nonsingular. */
   template<typename _Tp>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_singular(const _Tp* __ptr)
     { return __ptr == 0; }
 
@@ -71,7 +71,7 @@ namespace __gnu_debug
    * assertion statement because, e.g., we are in a constructor.
   */
   template<typename _InputIterator>
-    inline _InputIterator
+    _GLIBCXX20_CONSTEXPR inline _InputIterator
     __check_valid_range(const _InputIterator& __first,
 			const _InputIterator& __last,
 			const char* __file,
@@ -85,7 +85,7 @@ namespace __gnu_debug
 
   /* Handle the case where __other is a pointer to _Sequence::value_type. */
   template<typename _Iterator, typename _Sequence, typename _Category>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux4(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 	const typename _Sequence::value_type* __other)
@@ -107,7 +107,7 @@ namespace __gnu_debug
 
   /* Fallback overload for when we can't tell, assume it is valid. */
   template<typename _Iterator, typename _Sequence, typename _Category>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux4(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>&, ...)
     { return true; }
@@ -115,7 +115,7 @@ namespace __gnu_debug
   /* Handle sequences with contiguous storage */
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux3(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 	const _InputIterator& __other, const _InputIterator& __other_end,
@@ -131,7 +131,7 @@ namespace __gnu_debug
   /* Handle non-contiguous containers, assume it is valid. */
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux3(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 	const _InputIterator&, const _InputIterator&,
@@ -141,7 +141,7 @@ namespace __gnu_debug
   /** Handle debug iterators from the same type of container. */
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _OtherIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux2(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 	const _Safe_iterator<_OtherIterator, _Sequence, _Category>& __other,
@@ -152,7 +152,7 @@ namespace __gnu_debug
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _OtherIterator, typename _OtherSequence,
 	   typename _OtherCategory>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux2(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 	const _Safe_iterator<_OtherIterator, _OtherSequence,
@@ -164,7 +164,7 @@ namespace __gnu_debug
   /* Handle non-debug iterators. */
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux2(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 	const _InputIterator& __other,
@@ -185,7 +185,7 @@ namespace __gnu_debug
   /* Handle the case where we aren't really inserting a range after all */
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _Integral>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 	_Integral, _Integral, std::__true_type)
@@ -194,7 +194,7 @@ namespace __gnu_debug
   /* Handle all iterators. */
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator_aux(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 	_InputIterator __other, _InputIterator __other_end,
@@ -207,7 +207,7 @@ namespace __gnu_debug
 
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __foreign_iterator(
 	const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 	_InputIterator __other, _InputIterator __other_end)
@@ -219,7 +219,7 @@ namespace __gnu_debug
   // Can't check if an input iterator sequence is sorted, because we
   // can't step through the sequence.
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_aux(const _InputIterator&, const _InputIterator&,
                        std::input_iterator_tag)
     { return true; }
@@ -227,7 +227,7 @@ namespace __gnu_debug
   // Can verify if a forward iterator sequence is in fact sorted using
   // std::__is_sorted
   template<typename _ForwardIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_aux(_ForwardIterator __first, _ForwardIterator __last,
                        std::forward_iterator_tag)
     {
@@ -245,7 +245,7 @@ namespace __gnu_debug
   // Can't check if an input iterator sequence is sorted, because we can't step
   // through the sequence.
   template<typename _InputIterator, typename _Predicate>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_aux(const _InputIterator&, const _InputIterator&,
                        _Predicate, std::input_iterator_tag)
     { return true; }
@@ -253,7 +253,7 @@ namespace __gnu_debug
   // Can verify if a forward iterator sequence is in fact sorted using
   // std::__is_sorted
   template<typename _ForwardIterator, typename _Predicate>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_aux(_ForwardIterator __first, _ForwardIterator __last,
                        _Predicate __pred, std::forward_iterator_tag)
     {
@@ -270,7 +270,7 @@ namespace __gnu_debug
 
   // Determine if a sequence is sorted.
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted(const _InputIterator& __first, const _InputIterator& __last)
     {
       // Verify that the < operator for elements in the sequence is a
@@ -282,7 +282,7 @@ namespace __gnu_debug
     }
 
   template<typename _InputIterator, typename _Predicate>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted(const _InputIterator& __first, const _InputIterator& __last,
                    _Predicate __pred)
     {
@@ -295,28 +295,28 @@ namespace __gnu_debug
     }
 
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_set_aux(const _InputIterator& __first,
 			   const _InputIterator& __last,
 			   std::__true_type)
     { return __check_sorted(__first, __last); }
 
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_set_aux(const _InputIterator&,
 			   const _InputIterator&,
 			   std::__false_type)
     { return true; }
 
   template<typename _InputIterator, typename _Predicate>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_set_aux(const _InputIterator& __first,
 			   const _InputIterator& __last,
 			   _Predicate __pred, std::__true_type)
     { return __check_sorted(__first, __last, __pred); }
 
   template<typename _InputIterator, typename _Predicate>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_set_aux(const _InputIterator&,
 			   const _InputIterator&, _Predicate,
 			   std::__false_type)
@@ -324,7 +324,7 @@ namespace __gnu_debug
 
   // ... special variant used in std::merge, std::includes, std::set_*.
   template<typename _InputIterator1, typename _InputIterator2>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_set(const _InputIterator1& __first,
 		       const _InputIterator1& __last,
 		       const _InputIterator2&)
@@ -341,7 +341,7 @@ namespace __gnu_debug
 
   template<typename _InputIterator1, typename _InputIterator2,
 	   typename _Predicate>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_sorted_set(const _InputIterator1& __first,
 		       const _InputIterator1& __last,
 		       const _InputIterator2&, _Predicate __pred)
@@ -360,7 +360,7 @@ namespace __gnu_debug
   // 270. Binary search requirements overly strict
   // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_partitioned_lower(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
     {
@@ -376,7 +376,7 @@ namespace __gnu_debug
     }
 
   template<typename _ForwardIterator, typename _Tp>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
     {
@@ -393,7 +393,7 @@ namespace __gnu_debug
 
   // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_partitioned_lower(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
 			      _Pred __pred)
@@ -410,7 +410,7 @@ namespace __gnu_debug
     }
 
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
 			      _Pred __pred)
@@ -435,36 +435,36 @@ namespace __gnu_debug
 
     template<typename _It,
 	     typename = decltype(__deref<_It>() < __deref<_It>())>
-      static bool
+      _GLIBCXX20_CONSTEXPR static bool
       _S_is_valid(_It __it)
       { return !(*__it < *__it); }
 
     // Fallback method if operator doesn't exist.
     template<typename... _Args>
-      static bool
+      _GLIBCXX20_CONSTEXPR static bool
       _S_is_valid(_Args...)
       { return true; }
 
     template<typename _It, typename _Pred, typename
 	= decltype(std::declval<_Pred>()(__deref<_It>(), __deref<_It>()))>
-      static bool
+      _GLIBCXX20_CONSTEXPR static bool
       _S_is_valid_pred(_It __it, _Pred __pred)
       { return !__pred(*__it, *__it); }
 
     // Fallback method if predicate can't be invoked.
     template<typename... _Args>
-      static bool
+      _GLIBCXX20_CONSTEXPR static bool
       _S_is_valid_pred(_Args...)
       { return true; }
   };
 
   template<typename _Iterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __is_irreflexive(_Iterator __it)
     { return _Irreflexive_checker::_S_is_valid(__it); }
 
   template<typename _Iterator, typename _Pred>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __is_irreflexive_pred(_Iterator __it, _Pred __pred)
     { return _Irreflexive_checker::_S_is_valid_pred(__it, __pred); }
 #endif
diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index b7aeafef12a..a8686701454 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -87,13 +87,13 @@ namespace __gnu_debug
    *	precision.
   */
   template<typename _Iterator>
-    inline typename _Distance_traits<_Iterator>::__type
+    _GLIBCXX20_CONSTEXPR inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs,
 		   std::random_access_iterator_tag)
     { return std::make_pair(__rhs - __lhs, __dp_exact); }
 
   template<typename _Iterator>
-    inline typename _Distance_traits<_Iterator>::__type
+    _GLIBCXX20_CONSTEXPR inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs,
 		   std::input_iterator_tag)
     {
@@ -104,7 +104,7 @@ namespace __gnu_debug
     }
 
   template<typename _Iterator>
-    inline typename _Distance_traits<_Iterator>::__type
+    _GLIBCXX20_CONSTEXPR inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs)
     { return __get_distance(__lhs, __rhs, std::__iterator_category(__lhs)); }
 
@@ -113,7 +113,7 @@ namespace __gnu_debug
    *  iterators.
   */
   template<typename _Integral>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range_aux(_Integral, _Integral,
 		      typename _Distance_traits<_Integral>::__type& __dist,
 		      std::__true_type)
@@ -126,7 +126,7 @@ namespace __gnu_debug
    *  to see if we can check the range ahead of time.
   */
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range_aux(_InputIterator __first, _InputIterator __last,
 		      typename _Distance_traits<_InputIterator>::__type& __dist,
 		      std::__false_type)
@@ -155,7 +155,7 @@ namespace __gnu_debug
    *  otherwise.
   */
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range(_InputIterator __first, _InputIterator __last,
 		  typename _Distance_traits<_InputIterator>::__type& __dist)
     {
@@ -164,21 +164,21 @@ namespace __gnu_debug
     }
 
   template<typename _Iterator, typename _Sequence, typename _Category>
-    bool
+    _GLIBCXX20_CONSTEXPR bool
     __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 		  const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 		  typename _Distance_traits<_Iterator>::__type&);
 
 #if __cplusplus >= 201103L
-  template<typename _Iterator,typename _Sequence>
-    bool
+  template<typename _Iterator, typename _Sequence>
+    _GLIBCXX20_CONSTEXPR bool
     __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>&,
 		  const _Safe_local_iterator<_Iterator, _Sequence>&,
 		  typename _Distance_traits<_Iterator>::__type&);
 #endif
 
   template<typename _InputIterator>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range(_InputIterator __first, _InputIterator __last)
     {
       typename _Distance_traits<_InputIterator>::__type __dist;
@@ -186,26 +186,26 @@ namespace __gnu_debug
     }
 
   template<typename _Iterator, typename _Sequence, typename _Category>
-    bool
+    _GLIBCXX20_CONSTEXPR bool
     __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 		  const _Safe_iterator<_Iterator, _Sequence, _Category>&);
 
 #if __cplusplus >= 201103L
   template<typename _Iterator, typename _Sequence>
-    bool
+    _GLIBCXX20_CONSTEXPR bool
     __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>&,
 		  const _Safe_local_iterator<_Iterator, _Sequence>&);
 #endif
 
   // Fallback method, always ok.
   template<typename _InputIterator, typename _Size>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __can_advance(_InputIterator, _Size)
     { return true; }
 
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _Size>
-    bool
+    _GLIBCXX20_CONSTEXPR bool
     __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>&,
 		  _Size);
 
@@ -216,7 +216,7 @@ namespace __gnu_debug
    *  thanks to the < operator.
    */
   template<typename _Iterator>
-    inline _Iterator
+    _GLIBCXX20_CONSTEXPR inline _Iterator
     __base(_Iterator __it)
     { return __it; }
 
@@ -228,7 +228,7 @@ namespace __gnu_debug
 
   /* Remove debug mode safe iterator layer, if any. */
   template<typename _Iterator>
-    inline _Iterator
+    _GLIBCXX20_CONSTEXPR inline _Iterator
     __unsafe(_Iterator __it)
     { return __it; }
 }
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 6700eafca0b..0387367a08f 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -905,7 +905,7 @@ namespace __gnu_debug
 
   /** Safe iterators know how to check if they form a valid range. */
   template<typename _Iterator, typename _Sequence, typename _Category>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range(const _Safe_iterator<_Iterator, _Sequence,
 				       _Category>& __first,
 		  const _Safe_iterator<_Iterator, _Sequence,
@@ -914,7 +914,7 @@ namespace __gnu_debug
     { return __first._M_valid_range(__last, __dist); }
 
   template<typename _Iterator, typename _Sequence, typename _Category>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range(const _Safe_iterator<_Iterator, _Sequence,
 				       _Category>& __first,
 		  const _Safe_iterator<_Iterator, _Sequence,
@@ -926,13 +926,13 @@ namespace __gnu_debug
 
   template<typename _Iterator, typename _Sequence, typename _Category,
 	   typename _Size>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __can_advance(const _Safe_iterator<_Iterator, _Sequence, _Category>& __it,
 		  _Size __n)
     { return __it._M_can_advance(__n); }
 
   template<typename _Iterator, typename _Sequence>
-    _Iterator
+    _GLIBCXX20_CONSTEXPR _Iterator
     __base(const _Safe_iterator<_Iterator, _Sequence,
 				std::random_access_iterator_tag>& __it)
     { return __it.base(); }
@@ -944,7 +944,7 @@ namespace __gnu_debug
 #endif
 
   template<typename _Iterator, typename _Sequence>
-    inline _Iterator
+    _GLIBCXX20_CONSTEXPR inline _Iterator
     __unsafe(const _Safe_iterator<_Iterator, _Sequence>& __it)
     { return __it.base(); }
 
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index 40919126c9f..299487241bc 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -403,14 +403,14 @@ namespace __gnu_debug
 
   /** Safe local iterators know how to check if they form a valid range. */
   template<typename _Iterator, typename _Sequence>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>& __first,
 		  const _Safe_local_iterator<_Iterator, _Sequence>& __last,
 		  typename _Distance_traits<_Iterator>::__type& __dist_info)
     { return __first._M_valid_range(__last, __dist_info); }
 
   template<typename _Iterator, typename _Sequence>
-    inline bool
+    _GLIBCXX20_CONSTEXPR inline bool
     __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>& __first,
 		  const _Safe_local_iterator<_Iterator, _Sequence>& __last)
     {
@@ -425,7 +425,7 @@ namespace __gnu_debug
 #endif
 
   template<typename _Iterator, typename _Sequence>
-    inline _Iterator
+    _GLIBCXX20_CONSTEXPR inline _Iterator
     __unsafe(const _Safe_local_iterator<_Iterator, _Sequence>& __it)
     { return __it.base(); }
 

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-27 12:11 ` Jonathan Wakely
@ 2019-09-27 16:24   ` François Dumont
  2019-09-27 16:45     ` Jonathan Wakely
  0 siblings, 1 reply; 15+ messages in thread
From: François Dumont @ 2019-09-27 16:24 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

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

On 9/27/19 2:11 PM, Jonathan Wakely wrote:
> On 19/09/19 22:27 +0200, François Dumont wrote:
>> Hi
>>
>>     I start working on making recently added constexpr tests to work 
>> in Debug mode.
>
> The attached patch seems to be necessary for that, right?
>
>
On my side I had done this, almost the same.

For the moment there is a FIXME in macros.h to find out how to generate 
a nice compilation error when the condition is not meant.

static_assert can't be called in this context, too bad.

I also try to define a function with a 
__attribute__((__error__("because"))) attribute. But when I make it 
constexpr gcc complains about missing definition. When I provide a 
definition gcc complains that this attribute must be on a declaration. 
And when I split declaration and definition gcc does not produce the 
expected compilation error.

Unless you have the solution I consider that we need help from the 
front-end.

For the moment if Debug mode finds a problem it will be reported as 
_M_error function not being constexpr !

François


[-- Attachment #2: debug_constexpr.patch --]
[-- Type: text/x-patch, Size: 19688 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index a672f8b2b39..f25b8b76df6 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
    *  @param  __last1   Another iterator.
    *  @param  __last2   Another iterator.
    *  @param  __result  An iterator pointing to the end of the merged range.
-   *  @return         An iterator pointing to the first element <em>not less
-   *                  than</em> @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
+   *            + (__last2 - __first2).
    *
    *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
    *  the sorted range @p [__result, __result + (__last1-__first1) +
@@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
    *  @param  __last2   Another iterator.
    *  @param  __result  An iterator pointing to the end of the merged range.
    *  @param  __comp    A functor to use for comparisons.
-   *  @return         An iterator pointing to the first element "not less
-   *                  than" @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
+   *            + (__last2 - __first2).
    *
    *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
    *  the sorted range @p [__result, __result + (__last1-__first1) +
diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h
index f87838692c6..8c385b87244 100644
--- a/libstdc++-v3/include/debug/functions.h
+++ b/libstdc++-v3/include/debug/functions.h
@@ -219,6 +219,7 @@ namespace __gnu_debug
   // Can't check if an input iterator sequence is sorted, because we
   // can't step through the sequence.
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(const _InputIterator&, const _InputIterator&,
                        std::input_iterator_tag)
@@ -227,6 +228,7 @@ namespace __gnu_debug
   // Can verify if a forward iterator sequence is in fact sorted using
   // std::__is_sorted
   template<typename _ForwardIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(_ForwardIterator __first, _ForwardIterator __last,
                        std::forward_iterator_tag)
@@ -245,6 +247,7 @@ namespace __gnu_debug
   // Can't check if an input iterator sequence is sorted, because we can't step
   // through the sequence.
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(const _InputIterator&, const _InputIterator&,
                        _Predicate, std::input_iterator_tag)
@@ -253,6 +256,7 @@ namespace __gnu_debug
   // Can verify if a forward iterator sequence is in fact sorted using
   // std::__is_sorted
   template<typename _ForwardIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(_ForwardIterator __first, _ForwardIterator __last,
                        _Predicate __pred, std::forward_iterator_tag)
@@ -270,31 +274,26 @@ namespace __gnu_debug
 
   // Determine if a sequence is sorted.
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted(const _InputIterator& __first, const _InputIterator& __last)
     {
-      // Verify that the < operator for elements in the sequence is a
-      // StrictWeakOrdering by checking that it is irreflexive.
-      __glibcxx_assert(__first == __last || !(*__first < *__first));
-
       return __check_sorted_aux(__first, __last,
 				std::__iterator_category(__first));
     }
 
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted(const _InputIterator& __first, const _InputIterator& __last,
                    _Predicate __pred)
     {
-      // Verify that the predicate is StrictWeakOrdering by checking that it
-      // is irreflexive.
-      __glibcxx_assert(__first == __last || !__pred(*__first, *__first));
-
       return __check_sorted_aux(__first, __last, __pred,
 				std::__iterator_category(__first));
     }
 
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator& __first,
 			   const _InputIterator& __last,
@@ -302,6 +301,7 @@ namespace __gnu_debug
     { return __check_sorted(__first, __last); }
 
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator&,
 			   const _InputIterator&,
@@ -309,6 +309,7 @@ namespace __gnu_debug
     { return true; }
 
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator& __first,
 			   const _InputIterator& __last,
@@ -316,6 +317,7 @@ namespace __gnu_debug
     { return __check_sorted(__first, __last, __pred); }
 
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator&,
 			   const _InputIterator&, _Predicate,
@@ -324,6 +326,7 @@ namespace __gnu_debug
 
   // ... special variant used in std::merge, std::includes, std::set_*.
   template<typename _InputIterator1, typename _InputIterator2>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set(const _InputIterator1& __first,
 		       const _InputIterator1& __last,
@@ -341,6 +344,7 @@ namespace __gnu_debug
 
   template<typename _InputIterator1, typename _InputIterator2,
 	   typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set(const _InputIterator1& __first,
 		       const _InputIterator1& __last,
@@ -360,6 +364,7 @@ namespace __gnu_debug
   // 270. Binary search requirements overly strict
   // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_lower(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
@@ -376,6 +381,7 @@ namespace __gnu_debug
     }
 
   template<typename _ForwardIterator, typename _Tp>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
@@ -393,6 +399,7 @@ namespace __gnu_debug
 
   // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_lower(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
@@ -410,6 +417,7 @@ namespace __gnu_debug
     }
 
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
@@ -435,35 +443,41 @@ namespace __gnu_debug
 
     template<typename _It,
 	     typename = decltype(__deref<_It>() < __deref<_It>())>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid(_It __it)
       { return !(*__it < *__it); }
 
     // Fallback method if operator doesn't exist.
     template<typename... _Args>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid(_Args...)
       { return true; }
 
     template<typename _It, typename _Pred, typename
 	= decltype(std::declval<_Pred>()(__deref<_It>(), __deref<_It>()))>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid_pred(_It __it, _Pred __pred)
       { return !__pred(*__it, *__it); }
 
     // Fallback method if predicate can't be invoked.
     template<typename... _Args>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid_pred(_Args...)
       { return true; }
   };
 
   template<typename _Iterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __is_irreflexive(_Iterator __it)
     { return _Irreflexive_checker::_S_is_valid(__it); }
 
   template<typename _Iterator, typename _Pred>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __is_irreflexive_pred(_Iterator __it, _Pred __pred)
     { return _Irreflexive_checker::_S_is_valid_pred(__it, __pred); }
diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index 9429bb90a55..16d871e68f4 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -88,12 +88,14 @@ namespace __gnu_debug
    *	precision.
   */
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs,
 		   std::random_access_iterator_tag)
     { return std::make_pair(__rhs - __lhs, __dp_exact); }
 
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs,
 		   std::input_iterator_tag)
@@ -105,6 +107,7 @@ namespace __gnu_debug
     }
 
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs)
     { return __get_distance(__lhs, __rhs, std::__iterator_category(__lhs)); }
@@ -114,6 +117,13 @@ namespace __gnu_debug
    *  iterators.
   */
   template<typename _Integral>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_Integral, _Integral, std::__true_type)
+    { return true; }
+
+  template<typename _Integral>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __valid_range_aux(_Integral, _Integral,
 		      typename _Distance_traits<_Integral>::__type& __dist,
@@ -123,10 +133,35 @@ namespace __gnu_debug
       return true;
     }
 
+  template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_InputIterator __first, _InputIterator __last,
+		      std::input_iterator_tag)
+    { return true; }
+
+  template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_InputIterator __first, _InputIterator __last,
+		      std::random_access_iterator_tag)
+    { return __first <= __last; }
+
   /** We have iterators, so figure out what kind of iterators they are
    *  to see if we can check the range ahead of time.
   */
   template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_InputIterator __first, _InputIterator __last,
+		      std::__false_type)
+    {
+      return __valid_range_aux(__first, __last,
+			       std::__iterator_category(__first));
+    }
+
+  template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __valid_range_aux(_InputIterator __first, _InputIterator __last,
 		      typename _Distance_traits<_InputIterator>::__type& __dist,
@@ -157,10 +192,16 @@ namespace __gnu_debug
    *  otherwise.
   */
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __valid_range(_InputIterator __first, _InputIterator __last,
 		  typename _Distance_traits<_InputIterator>::__type& __dist)
     {
+#ifdef __cpp_lib_is_constant_evaluated
+      if (std::is_constant_evaluated())
+	// Detected by the compiler directly.
+	return true;
+#endif
       typedef typename std::__is_integer<_InputIterator>::__type _Integral;
       return __valid_range_aux(__first, __last, __dist, _Integral());
     }
@@ -180,11 +221,17 @@ namespace __gnu_debug
 #endif
 
   template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
     inline bool
     __valid_range(_InputIterator __first, _InputIterator __last)
     {
-      typename _Distance_traits<_InputIterator>::__type __dist;
-      return __valid_range(__first, __last, __dist);
+#ifdef __cpp_lib_is_constant_evaluated
+      if (std::is_constant_evaluated())
+	// Detected by the compiler directly.
+	return true;
+#endif
+      typedef typename std::__is_integer<_InputIterator>::__type _Integral;
+      return __valid_range_aux(__first, __last, _Integral());
     }
 
   template<typename _Iterator, typename _Sequence, typename _Category>
@@ -201,6 +248,7 @@ namespace __gnu_debug
 
   // Fallback method, always ok.
   template<typename _InputIterator, typename _Size>
+    _GLIBCXX_CONSTEXPR
     inline bool
     __can_advance(_InputIterator, _Size)
     { return true; }
@@ -218,6 +266,7 @@ namespace __gnu_debug
    *  thanks to the < operator.
    */
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline _Iterator
     __base(_Iterator __it)
     { return __it; }
diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h
index b598192cb3e..a6384b21fa4 100644
--- a/libstdc++-v3/include/debug/macros.h
+++ b/libstdc++-v3/include/debug/macros.h
@@ -38,10 +38,20 @@
  * the user error and where the error is reported.
  *
  */
-#define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func)	\
+#if 0 /* defined __cpp_lib_is_constant_evaluated */
+# define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func)	\
+  if (std::is_constant_evaluated())					\
+    /* FIXME: Compilation error here when !_Cond. */			\
+    break;								\
   if (! (_Cond))							\
     __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func)		\
       ._ErrMsg._M_error()
+#else
+# define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func)	\
+  if (! (_Cond))							\
+    __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func)		\
+      ._ErrMsg._M_error()
+#endif
 
 #define _GLIBCXX_DEBUG_VERIFY_AT_F(_Cond,_ErrMsg,_File,_Line,_Func)	\
   do									\
@@ -291,9 +301,43 @@ _GLIBCXX_DEBUG_VERIFY(! this->empty(),					\
 		      _M_message(__gnu_debug::__msg_empty)	        \
                       ._M_sequence(*this, "this"))
 
+// Verify that a predicate is irreflexive
+#define __glibcxx_check_irreflexive(_First,_Last)			\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last || !(*_First < *_First),	\
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_iterator_value_type(_First, "< operator type"))
+
+#if __cplusplus >= 201103L
+# define __glibcxx_check_irreflexive2(_First,_Last)			\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
+			|| __gnu_debug::__is_irreflexive(_First),	\
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_iterator_value_type(_First, "< operator type"))
+#else
+# define __glibcxx_check_irreflexive2(_First,_Last)
+#endif
+
+#define __glibcxx_check_irreflexive_pred(_First,_Last,_Pred)		\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last || !_Pred(*_First, *_First),	\
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_instance(_Pred, "functor")			\
+			._M_iterator_value_type(_First, "ordered type"))
+
+#if __cplusplus >= 201103L
+# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)		\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
+			||__gnu_debug::__is_irreflexive_pred(_First, _Pred), \
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_instance(_Pred, "functor")			\
+			._M_iterator_value_type(_First, "ordered type"))
+#else
+# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)
+#endif
+
 // Verify that the iterator range [_First, _Last) is sorted
 #define __glibcxx_check_sorted(_First,_Last)				\
 __glibcxx_check_valid_range(_First,_Last);				\
+__glibcxx_check_irreflexive(_First,_Last);				\
  _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(			\
 			__gnu_debug::__base(_First),			\
 			__gnu_debug::__base(_Last)),			\
@@ -305,6 +349,7 @@ __glibcxx_check_valid_range(_First,_Last);				\
     predicate _Pred. */
 #define __glibcxx_check_sorted_pred(_First,_Last,_Pred)			\
 __glibcxx_check_valid_range(_First,_Last);				\
+__glibcxx_check_irreflexive_pred(_First,_Last,_Pred);			\
 _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(			\
 			__gnu_debug::__base(_First),			\
 			__gnu_debug::__base(_Last), _Pred),		\
@@ -423,37 +468,4 @@ _GLIBCXX_DEBUG_VERIFY(_This.get_allocator() == _Other.get_allocator(),	\
 #define __glibcxx_check_string_len(_String,_Len) \
   _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0)
 
-// Verify that a predicate is irreflexive
-#define __glibcxx_check_irreflexive(_First,_Last)			\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last || !(*_First < *_First),	\
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_iterator_value_type(_First, "< operator type"))
-
-#if __cplusplus >= 201103L
-# define __glibcxx_check_irreflexive2(_First,_Last)			\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
-			|| __gnu_debug::__is_irreflexive(_First),	\
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_iterator_value_type(_First, "< operator type"))
-#else
-# define __glibcxx_check_irreflexive2(_First,_Last)
-#endif
-
-#define __glibcxx_check_irreflexive_pred(_First,_Last,_Pred)		\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last	|| !_Pred(*_First, *_First),		\
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_instance(_Pred, "functor")			\
-			._M_iterator_value_type(_First, "ordered type"))
-
-#if __cplusplus >= 201103L
-# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)		\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
-			||__gnu_debug::__is_irreflexive_pred(_First, _Pred), \
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_instance(_Pred, "functor")			\
-			._M_iterator_value_type(_First, "ordered type"))
-#else
-# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)
-#endif
-
 #endif
diff --git a/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc
index 8406b3d147f..205a96a223b 100644
--- a/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc
@@ -29,7 +29,7 @@ test()
   const auto out4 = std::binary_search(ca0.begin(), ca0.end(), 5);
 
   const auto out5 = std::binary_search(ca0.begin(), ca0.end(), 5,
-				       std::equal_to<int>());
+				       std::less<int>());
 
   return true;
 }
diff --git a/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
index 0be2f5fed62..f549b3d9307 100644
--- a/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
@@ -26,7 +26,7 @@ constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
 constexpr auto outv = std::is_sorted(ca0.begin(), ca0.end());
 
 constexpr auto outw = std::is_sorted(ca0.begin(), ca0.end(),
-				     std::equal_to<int>());
+				     std::less<int>());
 
 constexpr bool
 test()
diff --git a/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc
index cc8d3755da4..794453dd50c 100644
--- a/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc
@@ -26,7 +26,7 @@ test()
 {
   constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
   constexpr std::array<int, 12> cas{{3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}};
-  constexpr std::array<int, 3> camm{{-4, -5, -6}};
+  constexpr std::array<int, 3> camm{{-6, -5, -4}};
   std::array<int, 24> out0{{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
 
   const auto outdd = std::merge(ca0.begin(), ca0.end(),
@@ -34,7 +34,7 @@ test()
 
   const auto outee = std::merge(ca0.begin(), ca0.end(),
 				camm.begin(), camm.end(), out0.begin(),
-				[](int i, int j){ return i < -j; });
+				[](int i, int j){ return i < j; });
 
   return true;
 }

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-27 16:24   ` François Dumont
@ 2019-09-27 16:45     ` Jonathan Wakely
  2019-09-28 21:12       ` [PATCH] Fix algo constexpr tests in Debug mode François Dumont
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-09-27 16:45 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 27/09/19 18:24 +0200, François Dumont wrote:
>On 9/27/19 2:11 PM, Jonathan Wakely wrote:
>>On 19/09/19 22:27 +0200, François Dumont wrote:
>>>Hi
>>>
>>>    I start working on making recently added constexpr tests to 
>>>work in Debug mode.
>>
>>The attached patch seems to be necessary for that, right?
>>
>>
>On my side I had done this, almost the same.
>
>For the moment there is a FIXME in macros.h to find out how to 
>generate a nice compilation error when the condition is not meant.
>
>static_assert can't be called in this context, too bad.
>
>I also try to define a function with a 
>__attribute__((__error__("because"))) attribute. But when I make it 
>constexpr gcc complains about missing definition. When I provide a 
>definition gcc complains that this attribute must be on a declaration. 
>And when I split declaration and definition gcc does not produce the 
>expected compilation error.

Yes, I've tried similar things without success.

>Unless you have the solution I consider that we need help from the 
>front-end.
>
>For the moment if Debug mode finds a problem it will be reported as 
>_M_error function not being constexpr !

A reasonable workaround is to do:

#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
  if (__builtin_is_constant_evaluated())
    asm("Debug Mode assertion failed");
  else
#endif
  if (!(Cond))
    __gnu_debug::_Error_formatter::...

The builtin is available even for C++98, whereas
std::is_constant_evaluated() is only available for C++20.

This produces errors that include lines like:

asm.cc:12:17:   in ‘constexpr’ expansion of ‘f(-1)’
asm.cc:4:7: error: inline assembly is not a constant expression
    4 |       asm("debug mode assertion failed");
      |       ^~~
asm.cc:8:3: note: in expansion of macro ‘CHECK’
    8 |   _GLIBCXX_ASSERT(i > 0);
      |   ^~~~~
asm.cc:4:7: note: only unevaluated inline assembly is allowed in a ‘constexpr’ function in C++2a
    4 |       asm("debug mode assertion failed");
      |       ^~~
asm.cc:8:3: note: in expansion of macro ‘CHECK’
    8 |   CHECK(i > 0);
      |   ^~~~~

It's not ideal, but it does show the failed condition and the text
"debug mode assertion failed" (or whatever message you choose to use
there).


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

* [PATCH] Fix algo constexpr tests in Debug mode
  2019-09-27 16:45     ` Jonathan Wakely
@ 2019-09-28 21:12       ` François Dumont
  2019-09-30  9:03         ` Jonathan Wakely
  2019-10-23 15:26         ` Jonathan Wakely
  0 siblings, 2 replies; 15+ messages in thread
From: François Dumont @ 2019-09-28 21:12 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

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

Here is what I just commited.

I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
didn't notice any enhancement. So for now I kept my solution to just 
have a non-constexpr call compiler error.

I fix my patch to use __builtin_is_constant_evaluated rather than 
std::is_constant_evaluated in __valid_range.

     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
     (__memmove): Loop as long as __n is not 0.
     (__copy_move<>::__copy_m): Likewise.
     (__copy_move_backward<>::__copy_move_b): Likewise.
     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.

     * include/debug/forward_list
(_Sequence_traits<__debug::forward_list<>>::_S_size): Returns __dp_sign
     distance when not empty.
     * include/debug/list
     (_Sequence_traits<__debug::list<>>::_S_size): Likewise.
     * include/debug/helper_functions.h (__dp_sign_max_size): New
     _Distance_precision enum entry.
     * include/debug/safe_iterator.h
     (__copy_move_a(_II, _II, const _Safe_iterator<>&)): Check for output
     iterator _M_can_advance as soon as input range distance precision is
     strictly higher than __dp_size.
     (__copy_move_a(const _Safe_iterator<>&, const _Safe_iterator<>&,
     const _Safe_iterator<>&)): Likewise.
     (__copy_move_backward_a(_II, _II, const _Safe_iterator<>&)): Likewise.
     (__copy_move_backward_a(const _Safe_iterator<>&,
     const _Safe_iterator<>&, const _Safe_iterator<>&)): Likewise.
     (__equal_aux(_II, _II, const _Safe_iterator<>&)): Likewise.
     (__equal_aux(const _Safe_iterator<>&,
     const _Safe_iterator<>&, const _Safe_iterator<>&)): Likewise.

François

On 9/27/19 6:45 PM, Jonathan Wakely wrote:
> On 27/09/19 18:24 +0200, François Dumont wrote:
>> On 9/27/19 2:11 PM, Jonathan Wakely wrote:
>>> On 19/09/19 22:27 +0200, François Dumont wrote:
>>>> Hi
>>>>
>>>>     I start working on making recently added constexpr tests to 
>>>> work in Debug mode.
>>>
>>> The attached patch seems to be necessary for that, right?
>>>
>>>
>> On my side I had done this, almost the same.
>>
>> For the moment there is a FIXME in macros.h to find out how to 
>> generate a nice compilation error when the condition is not meant.
>>
>> static_assert can't be called in this context, too bad.
>>
>> I also try to define a function with a 
>> __attribute__((__error__("because"))) attribute. But when I make it 
>> constexpr gcc complains about missing definition. When I provide a 
>> definition gcc complains that this attribute must be on a 
>> declaration. And when I split declaration and definition gcc does not 
>> produce the expected compilation error.
>
> Yes, I've tried similar things without success.
>
>> Unless you have the solution I consider that we need help from the 
>> front-end.
>>
>> For the moment if Debug mode finds a problem it will be reported as 
>> _M_error function not being constexpr !
>
> A reasonable workaround is to do:
>
> #ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
>  if (__builtin_is_constant_evaluated())
>    asm("Debug Mode assertion failed");
>  else
> #endif
>  if (!(Cond))
>    __gnu_debug::_Error_formatter::...
>
> The builtin is available even for C++98, whereas
> std::is_constant_evaluated() is only available for C++20.
>
> This produces errors that include lines like:
>
> asm.cc:12:17:   in ‘constexpr’ expansion of ‘f(-1)’
> asm.cc:4:7: error: inline assembly is not a constant expression
>    4 |       asm("debug mode assertion failed");
>      |       ^~~
> asm.cc:8:3: note: in expansion of macro ‘CHECK’
>    8 |   _GLIBCXX_ASSERT(i > 0);
>      |   ^~~~~
> asm.cc:4:7: note: only unevaluated inline assembly is allowed in a 
> ‘constexpr’ function in C++2a
>    4 |       asm("debug mode assertion failed");
>      |       ^~~
> asm.cc:8:3: note: in expansion of macro ‘CHECK’
>    8 |   CHECK(i > 0);
>      |   ^~~~~
>
> It's not ideal, but it does show the failed condition and the text
> "debug mode assertion failed" (or whatever message you choose to use
> there).
>
>
>


[-- Attachment #2: debug_constexpr.patch --]
[-- Type: text/x-patch, Size: 19721 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index a672f8b2b39..f25b8b76df6 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
    *  @param  __last1   Another iterator.
    *  @param  __last2   Another iterator.
    *  @param  __result  An iterator pointing to the end of the merged range.
-   *  @return         An iterator pointing to the first element <em>not less
-   *                  than</em> @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
+   *            + (__last2 - __first2).
    *
    *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
    *  the sorted range @p [__result, __result + (__last1-__first1) +
@@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
    *  @param  __last2   Another iterator.
    *  @param  __result  An iterator pointing to the end of the merged range.
    *  @param  __comp    A functor to use for comparisons.
-   *  @return         An iterator pointing to the first element "not less
-   *                  than" @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
+   *            + (__last2 - __first2).
    *
    *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
    *  the sorted range @p [__result, __result + (__last1-__first1) +
diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h
index f87838692c6..8c385b87244 100644
--- a/libstdc++-v3/include/debug/functions.h
+++ b/libstdc++-v3/include/debug/functions.h
@@ -219,6 +219,7 @@ namespace __gnu_debug
   // Can't check if an input iterator sequence is sorted, because we
   // can't step through the sequence.
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(const _InputIterator&, const _InputIterator&,
                        std::input_iterator_tag)
@@ -227,6 +228,7 @@ namespace __gnu_debug
   // Can verify if a forward iterator sequence is in fact sorted using
   // std::__is_sorted
   template<typename _ForwardIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(_ForwardIterator __first, _ForwardIterator __last,
                        std::forward_iterator_tag)
@@ -245,6 +247,7 @@ namespace __gnu_debug
   // Can't check if an input iterator sequence is sorted, because we can't step
   // through the sequence.
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(const _InputIterator&, const _InputIterator&,
                        _Predicate, std::input_iterator_tag)
@@ -253,6 +256,7 @@ namespace __gnu_debug
   // Can verify if a forward iterator sequence is in fact sorted using
   // std::__is_sorted
   template<typename _ForwardIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_aux(_ForwardIterator __first, _ForwardIterator __last,
                        _Predicate __pred, std::forward_iterator_tag)
@@ -270,31 +274,26 @@ namespace __gnu_debug
 
   // Determine if a sequence is sorted.
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted(const _InputIterator& __first, const _InputIterator& __last)
     {
-      // Verify that the < operator for elements in the sequence is a
-      // StrictWeakOrdering by checking that it is irreflexive.
-      __glibcxx_assert(__first == __last || !(*__first < *__first));
-
       return __check_sorted_aux(__first, __last,
 				std::__iterator_category(__first));
     }
 
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted(const _InputIterator& __first, const _InputIterator& __last,
                    _Predicate __pred)
     {
-      // Verify that the predicate is StrictWeakOrdering by checking that it
-      // is irreflexive.
-      __glibcxx_assert(__first == __last || !__pred(*__first, *__first));
-
       return __check_sorted_aux(__first, __last, __pred,
 				std::__iterator_category(__first));
     }
 
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator& __first,
 			   const _InputIterator& __last,
@@ -302,6 +301,7 @@ namespace __gnu_debug
     { return __check_sorted(__first, __last); }
 
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator&,
 			   const _InputIterator&,
@@ -309,6 +309,7 @@ namespace __gnu_debug
     { return true; }
 
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator& __first,
 			   const _InputIterator& __last,
@@ -316,6 +317,7 @@ namespace __gnu_debug
     { return __check_sorted(__first, __last, __pred); }
 
   template<typename _InputIterator, typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set_aux(const _InputIterator&,
 			   const _InputIterator&, _Predicate,
@@ -324,6 +326,7 @@ namespace __gnu_debug
 
   // ... special variant used in std::merge, std::includes, std::set_*.
   template<typename _InputIterator1, typename _InputIterator2>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set(const _InputIterator1& __first,
 		       const _InputIterator1& __last,
@@ -341,6 +344,7 @@ namespace __gnu_debug
 
   template<typename _InputIterator1, typename _InputIterator2,
 	   typename _Predicate>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_sorted_set(const _InputIterator1& __first,
 		       const _InputIterator1& __last,
@@ -360,6 +364,7 @@ namespace __gnu_debug
   // 270. Binary search requirements overly strict
   // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_lower(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
@@ -376,6 +381,7 @@ namespace __gnu_debug
     }
 
   template<typename _ForwardIterator, typename _Tp>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value)
@@ -393,6 +399,7 @@ namespace __gnu_debug
 
   // Determine if a sequence is partitioned w.r.t. this element.
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_lower(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
@@ -410,6 +417,7 @@ namespace __gnu_debug
     }
 
   template<typename _ForwardIterator, typename _Tp, typename _Pred>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __check_partitioned_upper(_ForwardIterator __first,
 			      _ForwardIterator __last, const _Tp& __value,
@@ -435,35 +443,41 @@ namespace __gnu_debug
 
     template<typename _It,
 	     typename = decltype(__deref<_It>() < __deref<_It>())>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid(_It __it)
       { return !(*__it < *__it); }
 
     // Fallback method if operator doesn't exist.
     template<typename... _Args>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid(_Args...)
       { return true; }
 
     template<typename _It, typename _Pred, typename
 	= decltype(std::declval<_Pred>()(__deref<_It>(), __deref<_It>()))>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid_pred(_It __it, _Pred __pred)
       { return !__pred(*__it, *__it); }
 
     // Fallback method if predicate can't be invoked.
     template<typename... _Args>
+      _GLIBCXX20_CONSTEXPR
       static bool
       _S_is_valid_pred(_Args...)
       { return true; }
   };
 
   template<typename _Iterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __is_irreflexive(_Iterator __it)
     { return _Irreflexive_checker::_S_is_valid(__it); }
 
   template<typename _Iterator, typename _Pred>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __is_irreflexive_pred(_Iterator __it, _Pred __pred)
     { return _Irreflexive_checker::_S_is_valid_pred(__it, __pred); }
diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index 9429bb90a55..5a920bb9a6f 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -88,12 +88,14 @@ namespace __gnu_debug
    *	precision.
   */
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs,
 		   std::random_access_iterator_tag)
     { return std::make_pair(__rhs - __lhs, __dp_exact); }
 
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs,
 		   std::input_iterator_tag)
@@ -105,6 +107,7 @@ namespace __gnu_debug
     }
 
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline typename _Distance_traits<_Iterator>::__type
     __get_distance(_Iterator __lhs, _Iterator __rhs)
     { return __get_distance(__lhs, __rhs, std::__iterator_category(__lhs)); }
@@ -114,6 +117,13 @@ namespace __gnu_debug
    *  iterators.
   */
   template<typename _Integral>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_Integral, _Integral, std::__true_type)
+    { return true; }
+
+  template<typename _Integral>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __valid_range_aux(_Integral, _Integral,
 		      typename _Distance_traits<_Integral>::__type& __dist,
@@ -123,10 +133,35 @@ namespace __gnu_debug
       return true;
     }
 
+  template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_InputIterator __first, _InputIterator __last,
+		      std::input_iterator_tag)
+    { return true; }
+
+  template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_InputIterator __first, _InputIterator __last,
+		      std::random_access_iterator_tag)
+    { return __first <= __last; }
+
   /** We have iterators, so figure out what kind of iterators they are
    *  to see if we can check the range ahead of time.
   */
   template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
+    inline bool
+    __valid_range_aux(_InputIterator __first, _InputIterator __last,
+		      std::__false_type)
+    {
+      return __valid_range_aux(__first, __last,
+			       std::__iterator_category(__first));
+    }
+
+  template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __valid_range_aux(_InputIterator __first, _InputIterator __last,
 		      typename _Distance_traits<_InputIterator>::__type& __dist,
@@ -157,10 +192,16 @@ namespace __gnu_debug
    *  otherwise.
   */
   template<typename _InputIterator>
+    _GLIBCXX20_CONSTEXPR
     inline bool
     __valid_range(_InputIterator __first, _InputIterator __last,
 		  typename _Distance_traits<_InputIterator>::__type& __dist)
     {
+#ifdef __cpp_lib_is_constant_evaluated
+      if (std::is_constant_evaluated())
+	// Detected by the compiler directly.
+	return true;
+#endif
       typedef typename std::__is_integer<_InputIterator>::__type _Integral;
       return __valid_range_aux(__first, __last, __dist, _Integral());
     }
@@ -180,11 +221,17 @@ namespace __gnu_debug
 #endif
 
   template<typename _InputIterator>
+    _GLIBCXX_CONSTEXPR
     inline bool
     __valid_range(_InputIterator __first, _InputIterator __last)
     {
-      typename _Distance_traits<_InputIterator>::__type __dist;
-      return __valid_range(__first, __last, __dist);
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+      if (__builtin_is_constant_evaluated())
+	// Detected by the compiler directly.
+	return true;
+#endif
+      typedef typename std::__is_integer<_InputIterator>::__type _Integral;
+      return __valid_range_aux(__first, __last, _Integral());
     }
 
   template<typename _Iterator, typename _Sequence, typename _Category>
@@ -201,6 +248,7 @@ namespace __gnu_debug
 
   // Fallback method, always ok.
   template<typename _InputIterator, typename _Size>
+    _GLIBCXX_CONSTEXPR
     inline bool
     __can_advance(_InputIterator, _Size)
     { return true; }
@@ -218,6 +266,7 @@ namespace __gnu_debug
    *  thanks to the < operator.
    */
   template<typename _Iterator>
+    _GLIBCXX_CONSTEXPR
     inline _Iterator
     __base(_Iterator __it)
     { return __it; }
diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h
index b598192cb3e..97e54fd8390 100644
--- a/libstdc++-v3/include/debug/macros.h
+++ b/libstdc++-v3/include/debug/macros.h
@@ -38,10 +38,20 @@
  * the user error and where the error is reported.
  *
  */
-#define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func)	\
+#if 0 /* defined _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED */
+# define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func)	\
+  if (__builtin_is_constant_evaluated())				\
+    /* FIXME: Compilation error here when !_Cond. */			\
+    break;								\
   if (! (_Cond))							\
     __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func)		\
       ._ErrMsg._M_error()
+#else
+# define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func)	\
+  if (! (_Cond))							\
+    __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func)		\
+      ._ErrMsg._M_error()
+#endif
 
 #define _GLIBCXX_DEBUG_VERIFY_AT_F(_Cond,_ErrMsg,_File,_Line,_Func)	\
   do									\
@@ -291,9 +301,43 @@ _GLIBCXX_DEBUG_VERIFY(! this->empty(),					\
 		      _M_message(__gnu_debug::__msg_empty)	        \
                       ._M_sequence(*this, "this"))
 
+// Verify that a predicate is irreflexive
+#define __glibcxx_check_irreflexive(_First,_Last)			\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last || !(*_First < *_First),	\
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_iterator_value_type(_First, "< operator type"))
+
+#if __cplusplus >= 201103L
+# define __glibcxx_check_irreflexive2(_First,_Last)			\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
+			|| __gnu_debug::__is_irreflexive(_First),	\
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_iterator_value_type(_First, "< operator type"))
+#else
+# define __glibcxx_check_irreflexive2(_First,_Last)
+#endif
+
+#define __glibcxx_check_irreflexive_pred(_First,_Last,_Pred)		\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last || !_Pred(*_First, *_First),	\
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_instance(_Pred, "functor")			\
+			._M_iterator_value_type(_First, "ordered type"))
+
+#if __cplusplus >= 201103L
+# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)		\
+  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
+			||__gnu_debug::__is_irreflexive_pred(_First, _Pred), \
+			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
+			._M_instance(_Pred, "functor")			\
+			._M_iterator_value_type(_First, "ordered type"))
+#else
+# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)
+#endif
+
 // Verify that the iterator range [_First, _Last) is sorted
 #define __glibcxx_check_sorted(_First,_Last)				\
 __glibcxx_check_valid_range(_First,_Last);				\
+__glibcxx_check_irreflexive(_First,_Last);				\
  _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(			\
 			__gnu_debug::__base(_First),			\
 			__gnu_debug::__base(_Last)),			\
@@ -305,6 +349,7 @@ __glibcxx_check_valid_range(_First,_Last);				\
     predicate _Pred. */
 #define __glibcxx_check_sorted_pred(_First,_Last,_Pred)			\
 __glibcxx_check_valid_range(_First,_Last);				\
+__glibcxx_check_irreflexive_pred(_First,_Last,_Pred);			\
 _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted(			\
 			__gnu_debug::__base(_First),			\
 			__gnu_debug::__base(_Last), _Pred),		\
@@ -423,37 +468,4 @@ _GLIBCXX_DEBUG_VERIFY(_This.get_allocator() == _Other.get_allocator(),	\
 #define __glibcxx_check_string_len(_String,_Len) \
   _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0)
 
-// Verify that a predicate is irreflexive
-#define __glibcxx_check_irreflexive(_First,_Last)			\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last || !(*_First < *_First),	\
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_iterator_value_type(_First, "< operator type"))
-
-#if __cplusplus >= 201103L
-# define __glibcxx_check_irreflexive2(_First,_Last)			\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
-			|| __gnu_debug::__is_irreflexive(_First),	\
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_iterator_value_type(_First, "< operator type"))
-#else
-# define __glibcxx_check_irreflexive2(_First,_Last)
-#endif
-
-#define __glibcxx_check_irreflexive_pred(_First,_Last,_Pred)		\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last	|| !_Pred(*_First, *_First),		\
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_instance(_Pred, "functor")			\
-			._M_iterator_value_type(_First, "ordered type"))
-
-#if __cplusplus >= 201103L
-# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)		\
-  _GLIBCXX_DEBUG_VERIFY(_First == _Last					\
-			||__gnu_debug::__is_irreflexive_pred(_First, _Pred), \
-			_M_message(__gnu_debug::__msg_irreflexive_ordering) \
-			._M_instance(_Pred, "functor")			\
-			._M_iterator_value_type(_First, "ordered type"))
-#else
-# define __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred)
-#endif
-
 #endif
diff --git a/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc
index 8406b3d147f..205a96a223b 100644
--- a/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/binary_search/constexpr.cc
@@ -29,7 +29,7 @@ test()
   const auto out4 = std::binary_search(ca0.begin(), ca0.end(), 5);
 
   const auto out5 = std::binary_search(ca0.begin(), ca0.end(), 5,
-				       std::equal_to<int>());
+				       std::less<int>());
 
   return true;
 }
diff --git a/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
index 0be2f5fed62..f549b3d9307 100644
--- a/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/is_sorted/constexpr.cc
@@ -26,7 +26,7 @@ constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
 constexpr auto outv = std::is_sorted(ca0.begin(), ca0.end());
 
 constexpr auto outw = std::is_sorted(ca0.begin(), ca0.end(),
-				     std::equal_to<int>());
+				     std::less<int>());
 
 constexpr bool
 test()
diff --git a/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc
index cc8d3755da4..794453dd50c 100644
--- a/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/merge/constexpr.cc
@@ -26,7 +26,7 @@ test()
 {
   constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
   constexpr std::array<int, 12> cas{{3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}};
-  constexpr std::array<int, 3> camm{{-4, -5, -6}};
+  constexpr std::array<int, 3> camm{{-6, -5, -4}};
   std::array<int, 24> out0{{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
 
   const auto outdd = std::merge(ca0.begin(), ca0.end(),
@@ -34,7 +34,7 @@ test()
 
   const auto outee = std::merge(ca0.begin(), ca0.end(),
 				camm.begin(), camm.end(), out0.begin(),
-				[](int i, int j){ return i < -j; });
+				[](int i, int j){ return i < j; });
 
   return true;
 }

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

* Re: [PATCH] Fix algo constexpr tests in Debug mode
  2019-09-28 21:12       ` [PATCH] Fix algo constexpr tests in Debug mode François Dumont
@ 2019-09-30  9:03         ` Jonathan Wakely
  2019-09-30 20:21           ` François Dumont
  2019-10-23 15:26         ` Jonathan Wakely
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-09-30  9:03 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 28/09/19 23:12 +0200, François Dumont wrote:
>Here is what I just commited.

Thanks. In my branch I had a lot more _GLIBCXX20_CONSTEXPR additions
in the debug mode headers. Is it not needed on __check_singular,
__foreign_iterator etc. ?

>I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
>didn't notice any enhancement. So for now I kept my solution to just 
>have a non-constexpr call compiler error.

You won't see any improvement in the testsuite, because the compiler
flags for the testsuite suppress diagnostic notes. But I'd expect it
to give better output for users with the default compiler flags.


>diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
>index a672f8b2b39..f25b8b76df6 100644
>--- a/libstdc++-v3/include/bits/stl_algo.h
>+++ b/libstdc++-v3/include/bits/stl_algo.h
>@@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
>    *  @param  __last1   Another iterator.
>    *  @param  __last2   Another iterator.
>    *  @param  __result  An iterator pointing to the end of the merged range.
>-   *  @return         An iterator pointing to the first element <em>not less
>-   *                  than</em> @e val.
>+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
>+   *            + (__last2 - __first2).
>    *
>    *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
>    *  the sorted range @p [__result, __result + (__last1-__first1) +
>@@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
>    *  @param  __last2   Another iterator.
>    *  @param  __result  An iterator pointing to the end of the merged range.
>    *  @param  __comp    A functor to use for comparisons.
>-   *  @return         An iterator pointing to the first element "not less
>-   *                  than" @e val.
>+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
>+   *            + (__last2 - __first2).
>    *
>    *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
>    *  the sorted range @p [__result, __result + (__last1-__first1) +

These changes are fine but should have been a separate, unrelated
commit.


>@@ -157,10 +192,16 @@ namespace __gnu_debug
>    *  otherwise.
>   */
>   template<typename _InputIterator>
>+    _GLIBCXX20_CONSTEXPR
>     inline bool
>     __valid_range(_InputIterator __first, _InputIterator __last,
> 		  typename _Distance_traits<_InputIterator>::__type& __dist)
>     {
>+#ifdef __cpp_lib_is_constant_evaluated
>+      if (std::is_constant_evaluated())
>+	// Detected by the compiler directly.
>+	return true;
>+#endif

Should this be using the built-in, not the C++20 function?

In practice it's probably equivalent, because the function is only
going to be constant-evaluated when called from C++20 code, and in
that case the std::is_constant_evaluated() function will be available.
It just seems inconsistent to use the built-in in one place and not in
the other.

>       typedef typename std::__is_integer<_InputIterator>::__type _Integral;
>       return __valid_range_aux(__first, __last, __dist, _Integral());
>     }
>@@ -180,11 +221,17 @@ namespace __gnu_debug
> #endif
> 
>   template<typename _InputIterator>
>+    _GLIBCXX_CONSTEXPR
>     inline bool
>     __valid_range(_InputIterator __first, _InputIterator __last)
>     {
>-      typename _Distance_traits<_InputIterator>::__type __dist;
>-      return __valid_range(__first, __last, __dist);
>+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
>+      if (__builtin_is_constant_evaluated())
>+	// Detected by the compiler directly.
>+	return true;
>+#endif
>+      typedef typename std::__is_integer<_InputIterator>::__type _Integral;
>+      return __valid_range_aux(__first, __last, _Integral());
>     }
> 
>   template<typename _Iterator, typename _Sequence, typename _Category>

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

* Re: [PATCH] Fix algo constexpr tests in Debug mode
  2019-09-30  9:03         ` Jonathan Wakely
@ 2019-09-30 20:21           ` François Dumont
  2019-10-01 11:21             ` Jonathan Wakely
  0 siblings, 1 reply; 15+ messages in thread
From: François Dumont @ 2019-09-30 20:21 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 9/30/19 11:03 AM, Jonathan Wakely wrote:
> On 28/09/19 23:12 +0200, François Dumont wrote:
>> Here is what I just commited.
>
> Thanks. In my branch I had a lot more _GLIBCXX20_CONSTEXPR additions
> in the debug mode headers. Is it not needed on __check_singular,
> __foreign_iterator etc. ?

Yes, I know, I just limited myself to fixing testsuite tests for the 
moment. I'll check if those need it too.

>
>> I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
>> didn't notice any enhancement. So for now I kept my solution to just 
>> have a non-constexpr call compiler error.
>
> You won't see any improvement in the testsuite, because the compiler
> flags for the testsuite suppress diagnostic notes. But I'd expect it
> to give better output for users with the default compiler flags.
Ok, I didn't know, I'll give it another try then outside testsuite.
>
>
>> diff --git a/libstdc++-v3/include/bits/stl_algo.h 
>> b/libstdc++-v3/include/bits/stl_algo.h
>> index a672f8b2b39..f25b8b76df6 100644
>> --- a/libstdc++-v3/include/bits/stl_algo.h
>> +++ b/libstdc++-v3/include/bits/stl_algo.h
>> @@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
>>    *  @param  __last1   Another iterator.
>>    *  @param  __last2   Another iterator.
>>    *  @param  __result  An iterator pointing to the end of the merged 
>> range.
>> -   *  @return         An iterator pointing to the first element 
>> <em>not less
>> -   *                  than</em> @e val.
>> +   *  @return   An output iterator equal to @p __result + (__last1 - 
>> __first1)
>> +   *            + (__last2 - __first2).
>>    *
>>    *  Merges the ranges @p [__first1,__last1) and @p 
>> [__first2,__last2) into
>>    *  the sorted range @p [__result, __result + (__last1-__first1) +
>> @@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
>>    *  @param  __last2   Another iterator.
>>    *  @param  __result  An iterator pointing to the end of the merged 
>> range.
>>    *  @param  __comp    A functor to use for comparisons.
>> -   *  @return         An iterator pointing to the first element "not 
>> less
>> -   *                  than" @e val.
>> +   *  @return   An output iterator equal to @p __result + (__last1 - 
>> __first1)
>> +   *            + (__last2 - __first2).
>>    *
>>    *  Merges the ranges @p [__first1,__last1) and @p 
>> [__first2,__last2) into
>>    *  the sorted range @p [__result, __result + (__last1-__first1) +
>
> These changes are fine but should have been a separate, unrelated
> commit.

Ok, sorry, I consider that just a comment change was fine.


>
>
>> @@ -157,10 +192,16 @@ namespace __gnu_debug
>>    *  otherwise.
>>   */
>>   template<typename _InputIterator>
>> +    _GLIBCXX20_CONSTEXPR
>>     inline bool
>>     __valid_range(_InputIterator __first, _InputIterator __last,
>>           typename _Distance_traits<_InputIterator>::__type& __dist)
>>     {
>> +#ifdef __cpp_lib_is_constant_evaluated
>> +      if (std::is_constant_evaluated())
>> +    // Detected by the compiler directly.
>> +    return true;
>> +#endif
>
> Should this be using the built-in, not the C++20 function?
>
>
> In practice it's probably equivalent, because the function is only
> going to be constant-evaluated when called from C++20 code, and in
> that case the std::is_constant_evaluated() function will be available.


Yes, this is why I did it this way. And moreover it is using std::pair 
move assignment operator which is also C++20 constexpr.

>
> It just seems inconsistent to use the built-in in one place and not in
> the other.

It is also the reason why the simple simple __valid_range is not using 
the other anymore.

Maybe once I'll have check all the algo calls I'll find out that this 
one need _GLIBCXX_CONSTEXPR.

I got the sensation that library is being 'constexpr' decorated only 
when needed and when properly Standardise so are the Debug helpers.

François


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

* Re: [PATCH] Fix algo constexpr tests in Debug mode
  2019-09-30 20:21           ` François Dumont
@ 2019-10-01 11:21             ` Jonathan Wakely
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-10-01 11:21 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 30/09/19 22:21 +0200, François Dumont wrote:
>On 9/30/19 11:03 AM, Jonathan Wakely wrote:
>>These changes are fine but should have been a separate, unrelated
>>commit.
>
>Ok, sorry, I consider that just a comment change was fine.

It's fine, but it is unrelated so should be a separate commit. That
makes it easier to backport the documentation fix independently of the
rest of the patch. Or if the patch had to be reverted for some reason,
we wouldn't also revert the doc fix if it was in a separate commit.

Unrelated changes should usually be separate commits.


>>>@@ -157,10 +192,16 @@ namespace __gnu_debug
>>>   *  otherwise.
>>>  */
>>>  template<typename _InputIterator>
>>>+    _GLIBCXX20_CONSTEXPR
>>>    inline bool
>>>    __valid_range(_InputIterator __first, _InputIterator __last,
>>>          typename _Distance_traits<_InputIterator>::__type& __dist)
>>>    {
>>>+#ifdef __cpp_lib_is_constant_evaluated
>>>+      if (std::is_constant_evaluated())
>>>+    // Detected by the compiler directly.
>>>+    return true;
>>>+#endif
>>
>>Should this be using the built-in, not the C++20 function?
>>
>>
>>In practice it's probably equivalent, because the function is only
>>going to be constant-evaluated when called from C++20 code, and in
>>that case the std::is_constant_evaluated() function will be available.
>
>
>Yes, this is why I did it this way. And moreover it is using std::pair 
>move assignment operator which is also C++20 constexpr.
>
>>
>>It just seems inconsistent to use the built-in in one place and not in
>>the other.
>
>It is also the reason why the simple simple __valid_range is not using 
>the other anymore.
>
>Maybe once I'll have check all the algo calls I'll find out that this 
>one need _GLIBCXX_CONSTEXPR.
>
>I got the sensation that library is being 'constexpr' decorated only 
>when needed and when properly Standardise so are the Debug helpers.

The standard says when something should be a constexpr function, we
don't get to decide. So if a function is not constexpr in C++17 and is
constexpr in C++20, we have to conform to that. For functions that are
not part of the standard and are our own implementation details, we
can choose when to make them constexpr. We could make all the debug
helpers use _GLIBCXX14_CONSTEXPR (or even _GLIBCXX_CONSTEXPR if they
meet the very restrictive C++11 constexpr requirements) but since they
are never going to be constant evaluated except in C++20, there
doesn't seem to be much point to doing that.

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

* Re: [PATCH] Help compiler detect invalid code
  2019-09-27 11:24   ` Jonathan Wakely
@ 2019-10-01 20:05     ` François Dumont
  2019-10-10 20:03       ` Jonathan Wakely
  0 siblings, 1 reply; 15+ messages in thread
From: François Dumont @ 2019-10-01 20:05 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

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

On 9/27/19 1:24 PM, Jonathan Wakely wrote:
> On 20/09/19 07:08 +0200, François Dumont wrote:
>> I already realized that previous patch will be too controversial to 
>> be accepted.
>>
>> In this new version I just implement a real memmove in __memmove so 
>
> A real memmove doesn't just work backwards, it needs to detect any
> overlaps and work forwards *or* backwards, as needed.
ok, good to know, I understand now why using __builtin_memcopy didn't 
show any performance enhancement when I tested it !
>
> I think your change will break this case:
>
> #include <algorithm>
>
> constexpr int f(int i, int j, int k)
> {
>  int arr[5] = { 0, 0, i, j, k };
>  std::move(arr+2, arr+5, arr);
>  return arr[0] + arr[1] + arr[2];
> }
>
> static_assert( f(1, 2, 3) == 6 );
>
> This is valid because std::move only requires that the result iterator
> is not in the input range, but it's OK for the two ranges to overlap.
>
> I haven't tested it, but I think with your change the array will end
> up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
>
Indeed, I've added a std::move constexpr test in this new proposal which 
demonstrate that.

C++ Standard clearly states that [copy|move]_backward is done backward. 
So in this new proposal I propose to add a __memcopy used in copy/move 
and keep __memmove for *_backward algos. Both are using 
__builtin_memmove as before.


     * include/bits/stl_algobase.h (__memmove): Return void, loop as long as
     __n != 0.
     (__memcopy): New.
     (__copy_move<_IsMove, true, 
std::random_access_iterator_tag>::__copy_m):
     Adapt to use latter.
     (__copy_move_backward_a): Remove std::is_constant_evaluated block.
     * testsuite/25_algorithms/copy/constexpr.cc (test): Add check on copied
     values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc (test): Likewise
     and rename in test1.
     (test2): New.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
     * testsuite/25_algorithms/equal/constexpr_neg.cc: New.
     * testsuite/25_algorithms/move/constexpr.cc: New.
     * testsuite/25_algorithms/move/constexpr_neg.cc: New.

Tested under Linux x86_64.

Ok to commit ?

François


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: constexpr_algo.patch --]
[-- Type: text/x-patch; name="constexpr_algo.patch", Size: 13138 bytes --]

Index: include/bits/stl_algobase.h
===================================================================
--- include/bits/stl_algobase.h	(révision 276259)
+++ include/bits/stl_algobase.h	(copie de travail)
@@ -78,18 +78,18 @@
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /*
-   * A constexpr wrapper for __builtin_memmove.
+   * A constexpr wrapper for memcopy.
    * @param __num The number of elements of type _Tp (not bytes).
    */
   template<bool _IsMove, typename _Tp>
     _GLIBCXX14_CONSTEXPR
-    inline void*
-    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    inline void
+    __memcopy(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
     {
 #ifdef __cpp_lib_is_constant_evaluated
       if (std::is_constant_evaluated())
 	{
-	  for(; __num > 0; --__num)
+	  for (; __num != 0; --__num)
 	    {
 	      if constexpr (_IsMove)
 		*__dst = std::move(*__src);
@@ -98,15 +98,40 @@
 	      ++__src;
 	      ++__dst;
 	    }
-	  return __dst;
 	}
       else
 #endif
-	return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
-      return __dst;
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
     }
 
   /*
+   * A constexpr wrapper for memmove.
+   * @param __num The number of elements of type _Tp (not bytes).
+   */
+  template<bool _IsMove, typename _Tp>
+    _GLIBCXX14_CONSTEXPR
+    inline void
+    __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
+    {
+#ifdef __cpp_lib_is_constant_evaluated
+      if (std::is_constant_evaluated())
+	{
+	  __dst += __num;
+	  __src += __num;
+	  for (; __num != 0; --__num)
+	    {
+	      if constexpr (_IsMove)
+		*--__dst = std::move(*--__src);
+	      else
+		*--__dst = *--__src;
+	    }
+	}
+      else
+#endif
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
+    }
+
+  /*
    * A constexpr wrapper for __builtin_memcmp.
    * @param __num The number of elements of type _Tp (not bytes).
    */
@@ -446,7 +471,7 @@
 #endif
 	  const ptrdiff_t _Num = __last - __first;
 	  if (_Num)
-	    std::__memmove<_IsMove>(__result, __first, _Num);
+	    std::__memcopy<_IsMove>(__result, __first, _Num);
 	  return __result + _Num;
 	}
     };
@@ -676,12 +701,6 @@
 			     && __is_pointer<_BI2>::__value
 			     && __are_same<_ValueType1, _ValueType2>::__value);
 
-#ifdef __cpp_lib_is_constant_evaluated
-      if (std::is_constant_evaluated())
-	return std::__copy_move_backward<true, false,
-			      _Category>::__copy_move_b(__first, __last,
-							__result);
-#endif
       return std::__copy_move_backward<_IsMove, __simple,
 				       _Category>::__copy_move_b(__first,
 								 __last,
Index: testsuite/25_algorithms/copy/constexpr.cc
===================================================================
--- testsuite/25_algorithms/copy/constexpr.cc	(révision 276259)
+++ testsuite/25_algorithms/copy/constexpr.cc	(copie de travail)
@@ -24,12 +24,12 @@
 constexpr bool
 test()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
   const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
 
-  return out6 == ma0.begin() + 10;
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
 }
 
 static_assert(test());
Index: testsuite/25_algorithms/copy/constexpr_neg.cc
===================================================================
--- testsuite/25_algorithms/copy/constexpr_neg.cc	(nonexistent)
+++ testsuite/25_algorithms/copy/constexpr_neg.cc	(copie de travail)
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
Index: testsuite/25_algorithms/copy_backward/constexpr.cc
===================================================================
--- testsuite/25_algorithms/copy_backward/constexpr.cc	(révision 276259)
+++ testsuite/25_algorithms/copy_backward/constexpr.cc	(copie de travail)
@@ -22,15 +22,27 @@
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
 
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
   const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8,
 				       ma0.begin() + 10);
 
-  return true;
+  return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8,
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
Index: testsuite/25_algorithms/copy_backward/constexpr_neg.cc
===================================================================
--- testsuite/25_algorithms/copy_backward/constexpr_neg.cc	(nonexistent)
+++ testsuite/25_algorithms/copy_backward/constexpr_neg.cc	(copie de travail)
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(),
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
Index: testsuite/25_algorithms/equal/constexpr_neg.cc
===================================================================
--- testsuite/25_algorithms/equal/constexpr_neg.cc	(nonexistent)
+++ testsuite/25_algorithms/equal/constexpr_neg.cc	(copie de travail)
@@ -0,0 +1,49 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test01()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+
+  const auto outa = std::equal(ca0.end(), ca0.begin(), ca1.begin());
+  return outa;
+}
+
+static_assert(test01()); // { dg-error "outside the bounds" }
+
+constexpr bool
+test02()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 11> ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}};
+
+  const auto outa = std::equal(ca0.begin(), ca0.end(), ca1.begin());
+  return outa;
+}
+
+static_assert(test02()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
Index: testsuite/25_algorithms/move/constexpr.cc
===================================================================
--- testsuite/25_algorithms/move/constexpr.cc	(nonexistent)
+++ testsuite/25_algorithms/move/constexpr.cc	(copie de travail)
@@ -0,0 +1,47 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test1()
+{
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::move(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
+}
+
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
+
+  const auto out6 = std::move(ca0.begin() + 2, ca0.begin() + 8, ca0.begin());
+
+  return out6 == ca0.begin() + 6 && *(ca0.begin()) == 3 && *out6 == 7;
+}
+
+static_assert(test2());
Index: testsuite/25_algorithms/move/constexpr_neg.cc
===================================================================
--- testsuite/25_algorithms/move/constexpr_neg.cc	(nonexistent)
+++ testsuite/25_algorithms/move/constexpr_neg.cc	(copie de travail)
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::move(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }


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

* Re: [PATCH] Help compiler detect invalid code
  2019-10-01 20:05     ` François Dumont
@ 2019-10-10 20:03       ` Jonathan Wakely
  2019-10-16 20:22         ` François Dumont
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-10-10 20:03 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 01/10/19 22:05 +0200, François Dumont wrote:
>On 9/27/19 1:24 PM, Jonathan Wakely wrote:
>>On 20/09/19 07:08 +0200, François Dumont wrote:
>>>I already realized that previous patch will be too controversial 
>>>to be accepted.
>>>
>>>In this new version I just implement a real memmove in __memmove 
>>>so
>>
>>A real memmove doesn't just work backwards, it needs to detect any
>>overlaps and work forwards *or* backwards, as needed.
>ok, good to know, I understand now why using __builtin_memcopy didn't 
>show any performance enhancement when I tested it !
>>
>>I think your change will break this case:
>>
>>#include <algorithm>
>>
>>constexpr int f(int i, int j, int k)
>>{
>> int arr[5] = { 0, 0, i, j, k };
>> std::move(arr+2, arr+5, arr);
>> return arr[0] + arr[1] + arr[2];
>>}
>>
>>static_assert( f(1, 2, 3) == 6 );
>>
>>This is valid because std::move only requires that the result iterator
>>is not in the input range, but it's OK for the two ranges to overlap.
>>
>>I haven't tested it, but I think with your change the array will end
>>up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
>>
>Indeed, I've added a std::move constexpr test in this new proposal 
>which demonstrate that.
>
>C++ Standard clearly states that [copy|move]_backward is done 
>backward. So in this new proposal I propose to add a __memcopy used in 
>copy/move and keep __memmove for *_backward algos. Both are using 
>__builtin_memmove as before.

Then they *really* need better names now (__memmove was already a bad
name, but now it's terrible). If the difference is that one goes
forwards and one goes backwards, the names should reflect that.

I'll review it properly tomorrow.

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

* Re: [PATCH] Help compiler detect invalid code
  2019-10-10 20:03       ` Jonathan Wakely
@ 2019-10-16 20:22         ` François Dumont
  0 siblings, 0 replies; 15+ messages in thread
From: François Dumont @ 2019-10-16 20:22 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

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

Here is a version with __detail::__copy and __detail::__copy_backward.

I prefered to introduce the __detail namespace cause __copy is quite a 
common name so putting it in __detail namespace will I hope clarify that 
it is for internal usage only.

I even hesitated to put more details into this namespace, maybe for 
another patch later.

     * include/bits/stl_algobase.h (__memmove): Replace by...
     (__detail::__copy) ...that. Return void, loop as long as __n != 0.
     (__copy_move<_IsMove, true, 
std::random_access_iterator_tag>::__copy_m):
     Adapt to use latter.
     (__detail::__copy_backward): New.
     (__copy_move_backward<_IsMove, true,
     std::random_access_iterator_tag>::__copy_m): Adapt to use latter.
     (__copy_move_backward_a): Remove std::is_constant_evaluated block.
     * testsuite/25_algorithms/copy/constexpr.cc (test): Add check on copied
     values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc (test): Likewise
     and rename in test1.
     (test2): New.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
     * testsuite/25_algorithms/equal/constexpr_neg.cc: New.
     * testsuite/25_algorithms/move/constexpr.cc: New.
     * testsuite/25_algorithms/move/constexpr_neg.cc: New.

François

On 10/10/19 10:03 PM, Jonathan Wakely wrote:
> On 01/10/19 22:05 +0200, François Dumont wrote:
>> On 9/27/19 1:24 PM, Jonathan Wakely wrote:
>>> On 20/09/19 07:08 +0200, François Dumont wrote:
>>>> I already realized that previous patch will be too controversial to 
>>>> be accepted.
>>>>
>>>> In this new version I just implement a real memmove in __memmove so
>>>
>>> A real memmove doesn't just work backwards, it needs to detect any
>>> overlaps and work forwards *or* backwards, as needed.
>> ok, good to know, I understand now why using __builtin_memcopy didn't 
>> show any performance enhancement when I tested it !
>>>
>>> I think your change will break this case:
>>>
>>> #include <algorithm>
>>>
>>> constexpr int f(int i, int j, int k)
>>> {
>>>  int arr[5] = { 0, 0, i, j, k };
>>>  std::move(arr+2, arr+5, arr);
>>>  return arr[0] + arr[1] + arr[2];
>>> }
>>>
>>> static_assert( f(1, 2, 3) == 6 );
>>>
>>> This is valid because std::move only requires that the result iterator
>>> is not in the input range, but it's OK for the two ranges to overlap.
>>>
>>> I haven't tested it, but I think with your change the array will end
>>> up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
>>>
>> Indeed, I've added a std::move constexpr test in this new proposal 
>> which demonstrate that.
>>
>> C++ Standard clearly states that [copy|move]_backward is done 
>> backward. So in this new proposal I propose to add a __memcopy used 
>> in copy/move and keep __memmove for *_backward algos. Both are using 
>> __builtin_memmove as before.
>
> Then they *really* need better names now (__memmove was already a bad
> name, but now it's terrible). If the difference is that one goes
> forwards and one goes backwards, the names should reflect that.
>
> I'll review it properly tomorrow.
>
>


[-- Attachment #2: constexpr_algo.patch --]
[-- Type: text/x-patch, Size: 13815 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 98d324827ed..dc6b3d3fc76 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -77,19 +77,22 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+namespace __detail
+{
   /*
    * A constexpr wrapper for __builtin_memmove.
+   * When constant-evaluated performs a forward copy.
    * @param __num The number of elements of type _Tp (not bytes).
    */
   template<bool _IsMove, typename _Tp>
     _GLIBCXX14_CONSTEXPR
-    inline void*
-    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    inline void
+    __copy(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
     {
 #ifdef __cpp_lib_is_constant_evaluated
       if (std::is_constant_evaluated())
 	{
-	  for(; __num > 0; --__num)
+	  for (; __num != 0; --__num)
 	    {
 	      if constexpr (_IsMove)
 		*__dst = std::move(*__src);
@@ -98,13 +101,40 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      ++__src;
 	      ++__dst;
 	    }
-	  return __dst;
 	}
       else
 #endif
-	return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
-      return __dst;
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
+    }
+
+  /*
+   * A constexpr wrapper for __builtin_memmove.
+   * When constant-evaluated performs a backward copy.
+   * @param __num The number of elements of type _Tp (not bytes).
+   */
+  template<bool _IsMove, typename _Tp>
+    _GLIBCXX14_CONSTEXPR
+    inline void
+    __copy_backward(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
+    {
+#ifdef __cpp_lib_is_constant_evaluated
+      if (std::is_constant_evaluated())
+	{
+	  __dst += __num;
+	  __src += __num;
+	  for (; __num != 0; --__num)
+	    {
+	      if constexpr (_IsMove)
+		*--__dst = std::move(*--__src);
+	      else
+		*--__dst = *--__src;
+	    }
+	}
+      else
+#endif
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
     }
+} // namespace __detail
 
   /*
    * A constexpr wrapper for __builtin_memcmp.
@@ -446,7 +476,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 	  const ptrdiff_t _Num = __last - __first;
 	  if (_Num)
-	    std::__memmove<_IsMove>(__result, __first, _Num);
+	    std::__detail::__copy<_IsMove>(__result, __first, _Num);
 	  return __result + _Num;
 	}
     };
@@ -658,7 +688,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 	  const ptrdiff_t _Num = __last - __first;
 	  if (_Num)
-	    std::__memmove<_IsMove>(__result - _Num, __first, _Num);
+	    std::__detail::__copy_backward<_IsMove>(__result - _Num,
+						    __first, _Num);
 	  return __result - _Num;
 	}
     };
@@ -676,12 +707,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			     && __is_pointer<_BI2>::__value
 			     && __are_same<_ValueType1, _ValueType2>::__value);
 
-#ifdef __cpp_lib_is_constant_evaluated
-      if (std::is_constant_evaluated())
-	return std::__copy_move_backward<true, false,
-			      _Category>::__copy_move_b(__first, __last,
-							__result);
-#endif
       return std::__copy_move_backward<_IsMove, __simple,
 				       _Category>::__copy_move_b(__first,
 								 __last,
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
index 67910b8773e..a0460603496 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
@@ -24,12 +24,12 @@
 constexpr bool
 test()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
   const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
 
-  return out6 == ma0.begin() + 10;
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
 }
 
 static_assert(test());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
new file mode 100644
index 00000000000..49052467409
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
index ed7487905a8..c0e1a747832 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
@@ -22,15 +22,27 @@
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
 
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
   const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8,
 				       ma0.begin() + 10);
 
-  return true;
+  return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8,
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
new file mode 100644
index 00000000000..3dc595d239f
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(),
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/equal/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/equal/constexpr_neg.cc
new file mode 100644
index 00000000000..065ca1e89ff
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/equal/constexpr_neg.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test01()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+
+  const auto outa = std::equal(ca0.end(), ca0.begin(), ca1.begin());
+  return outa;
+}
+
+static_assert(test01()); // { dg-error "outside the bounds" }
+
+constexpr bool
+test02()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 11> ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}};
+
+  const auto outa = std::equal(ca0.begin(), ca0.end(), ca1.begin());
+  return outa;
+}
+
+static_assert(test02()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/move/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/move/constexpr.cc
new file mode 100644
index 00000000000..140df5ceb2c
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/move/constexpr.cc
@@ -0,0 +1,47 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test1()
+{
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::move(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
+}
+
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
+
+  const auto out6 = std::move(ca0.begin() + 2, ca0.begin() + 8, ca0.begin());
+
+  return out6 == ca0.begin() + 6 && *(ca0.begin()) == 3 && *out6 == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/move/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/move/constexpr_neg.cc
new file mode 100644
index 00000000000..333dbfae742
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/move/constexpr_neg.cc
@@ -0,0 +1,38 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::move(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }

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

* Re: [PATCH] Fix algo constexpr tests in Debug mode
  2019-09-28 21:12       ` [PATCH] Fix algo constexpr tests in Debug mode François Dumont
  2019-09-30  9:03         ` Jonathan Wakely
@ 2019-10-23 15:26         ` Jonathan Wakely
  1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-10-23 15:26 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

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

On 28/09/19 23:12 +0200, François Dumont wrote:
>Here is what I just commited.
>
>I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
>didn't notice any enhancement. So for now I kept my solution to just 
>have a non-constexpr call compiler error.
>
>I fix my patch to use __builtin_is_constant_evaluated rather than 
>std::is_constant_evaluated in __valid_range.
>
>    * include/bits/stl_algobase.h (__memmove): Return _Tp*.
>    (__memmove): Loop as long as __n is not 0.
>    (__copy_move<>::__copy_m): Likewise.
>    (__copy_move_backward<>::__copy_move_b): Likewise.
>    * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
>values.
>    * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
>    * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
>    * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
>
>    * include/debug/forward_list
>(_Sequence_traits<__debug::forward_list<>>::_S_size): Returns __dp_sign
>    distance when not empty.
>    * include/debug/list
>    (_Sequence_traits<__debug::list<>>::_S_size): Likewise.
>    * include/debug/helper_functions.h (__dp_sign_max_size): New
>    _Distance_precision enum entry.
>    * include/debug/safe_iterator.h
>    (__copy_move_a(_II, _II, const _Safe_iterator<>&)): Check for output
>    iterator _M_can_advance as soon as input range distance precision is
>    strictly higher than __dp_size.
>    (__copy_move_a(const _Safe_iterator<>&, const _Safe_iterator<>&,
>    const _Safe_iterator<>&)): Likewise.
>    (__copy_move_backward_a(_II, _II, const _Safe_iterator<>&)): Likewise.
>    (__copy_move_backward_a(const _Safe_iterator<>&,
>    const _Safe_iterator<>&, const _Safe_iterator<>&)): Likewise.
>    (__equal_aux(_II, _II, const _Safe_iterator<>&)): Likewise.
>    (__equal_aux(const _Safe_iterator<>&,
>    const _Safe_iterator<>&, const _Safe_iterator<>&)): Likewise.

I'm going to commit this small fix.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 962 bytes --]

commit d78a141b86aca5a1265ec2df96428ef387492a1f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 23 16:19:28 2019 +0100

    Only qualify function as constexpr for C++14 and later
    
    This helper function is not a valid constexpr function in C++11, so
    should only be marked constexpr for C++14 and later.
    
            * include/debug/helper_functions.h (__valid_range): Change
            _GLIBCXX_CONSTEXPR to _GLIBCXX14_CONSTEXPR.

diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index 5a920bb9a6f..c3e7478f649 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -221,7 +221,7 @@ namespace __gnu_debug
 #endif
 
   template<typename _InputIterator>
-    _GLIBCXX_CONSTEXPR
+    _GLIBCXX14_CONSTEXPR
     inline bool
     __valid_range(_InputIterator __first, _InputIterator __last)
     {

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

end of thread, other threads:[~2019-10-23 15:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 20:28 [PATCH] Help compiler detect invalid code François Dumont
2019-09-20  5:08 ` François Dumont
2019-09-25 20:40   ` François Dumont
2019-09-27 11:24   ` Jonathan Wakely
2019-10-01 20:05     ` François Dumont
2019-10-10 20:03       ` Jonathan Wakely
2019-10-16 20:22         ` François Dumont
2019-09-27 12:11 ` Jonathan Wakely
2019-09-27 16:24   ` François Dumont
2019-09-27 16:45     ` Jonathan Wakely
2019-09-28 21:12       ` [PATCH] Fix algo constexpr tests in Debug mode François Dumont
2019-09-30  9:03         ` Jonathan Wakely
2019-09-30 20:21           ` François Dumont
2019-10-01 11:21             ` Jonathan Wakely
2019-10-23 15:26         ` 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).