From: "François Dumont" <frs.dumont@gmail.com>
To: "libstdc++@gcc.gnu.org" <libstdc++@gcc.gnu.org>,
gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Help compiler detect invalid code
Date: Wed, 25 Sep 2019 20:40:00 -0000 [thread overview]
Message-ID: <9d9df95f-a430-7920-a6d8-c9dec78aaa67@gmail.com> (raw)
In-Reply-To: <96b51704-67fe-3ac4-405c-6997151d926c@gmail.com>
[-- 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()
next prev parent reply other threads:[~2019-09-25 20:40 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-19 20:28 François Dumont
2019-09-20 5:08 ` François Dumont
2019-09-25 20:40 ` François Dumont [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9d9df95f-a430-7920-a6d8-c9dec78aaa67@gmail.com \
--to=frs.dumont@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).