public inbox for gcc-patches@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
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:27       ` Jonathan Wakely
2019-10-16 20:35         ` 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 16:02         ` 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).