* [committed] libstdc++: Constrain reverse_iterator and move_iterator conversions [LWG 3435]
@ 2020-07-22 14:10 Jonathan Wakely
2020-07-31 22:49 ` Martin Jambor
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2020-07-22 14:10 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
libstdc++-v3/ChangeLog:
* include/bits/stl_iterator.h (reverse_iterator): Constrain
converting constructor and converting assignment operator.
Access source iterator's data member directly instead of
calling base().
(move_iterator): Likewise.
* testsuite/24_iterators/move_iterator/dr3435.cc: New test.
* testsuite/24_iterators/reverse_iterator/dr3435.cc: New test.
Tested powerpc64le-linux, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 6926 bytes --]
commit a5a8a4e61565a2a66391e29eb80813c581b7dc52
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed Jul 22 13:25:11 2020
libstdc++: Constrain reverse_iterator and move_iterator conversions [LWG 3435]
libstdc++-v3/ChangeLog:
* include/bits/stl_iterator.h (reverse_iterator): Constrain
converting constructor and converting assignment operator.
Access source iterator's data member directly instead of
calling base().
(move_iterator): Likewise.
* testsuite/24_iterators/move_iterator/dr3435.cc: New test.
* testsuite/24_iterators/reverse_iterator/dr3435.cc: New test.
diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 6d2d19eb068..60bb40a659f 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -129,6 +129,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename iterator_traits<_Iterator>::pointer,
typename iterator_traits<_Iterator>::reference>
{
+#if __cplusplus >= 201103L
+ template<typename _Iter>
+ friend class reverse_iterator;
+#endif
+
+#if __cpp_lib_concepts
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 3435. three_way_comparable_with<reverse_iterator<int*>, [...]>
+ template<typename _Iter>
+ static constexpr bool __convertible = !is_same_v<_Iter, _Iterator>
+ && convertible_to<const _Iter&, _Iterator>;
+#endif
+
protected:
_Iterator current;
@@ -182,9 +195,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* underlying %iterator can be converted to the type of @c current.
*/
template<typename _Iter>
+#if __cpp_lib_concepts
+ requires __convertible<_Iter>
+#endif
_GLIBCXX17_CONSTEXPR
reverse_iterator(const reverse_iterator<_Iter>& __x)
- : current(__x.base()) { }
+ : current(__x.current) { }
+
+#if __cplusplus >= 201103L
+ template<typename _Iter>
+#if __cpp_lib_concepts
+ requires __convertible<_Iter>
+ && assignable_from<_Iterator&, const _Iter&>
+#endif
+ _GLIBCXX17_CONSTEXPR
+ reverse_iterator&
+ operator=(const reverse_iterator<_Iter>& __x)
+ {
+ current = __x.current;
+ return *this;
+ }
+#endif
/**
* @return @c current, the %iterator used for underlying work.
@@ -1270,6 +1301,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using __base_ref = typename __traits_type::reference;
#endif
+ template<typename _Iter2>
+ friend class move_iterator;
+
+#if __cpp_lib_concepts
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 3435. three_way_comparable_with<reverse_iterator<int*>, [...]>
+ template<typename _Iter2>
+ static constexpr bool __convertible = !is_same_v<_Iter2, _Iterator>
+ && convertible_to<const _Iter2&, _Iterator>;
+#endif
+
public:
using iterator_type = _Iterator;
@@ -1303,15 +1345,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _M_current(std::move(__i)) { }
template<typename _Iter>
+#if __cpp_lib_concepts
+ requires __convertible<_Iter>
+#endif
_GLIBCXX17_CONSTEXPR
move_iterator(const move_iterator<_Iter>& __i)
- : _M_current(__i.base()) { }
+ : _M_current(__i._M_current) { }
template<typename _Iter>
+#if __cpp_lib_concepts
+ requires __convertible<_Iter>
+ && assignable_from<_Iterator&, const _Iter&>
+#endif
_GLIBCXX17_CONSTEXPR
move_iterator& operator=(const move_iterator<_Iter>& __i)
{
- _M_current = __i.base();
+ _M_current = __i._M_current;
return *this;
}
diff --git a/libstdc++-v3/testsuite/24_iterators/move_iterator/dr3435.cc b/libstdc++-v3/testsuite/24_iterators/move_iterator/dr3435.cc
new file mode 100644
index 00000000000..170d4977df1
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/move_iterator/dr3435.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2020 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 <iterator>
+#include <compare>
+
+// LWG 3435. three_way_comparable_with<reverse_iterator<int*>, reverse_iterator<const int*>>
+// and
+// LWG 3265. move_iterator's conversions are more broken after P1207
+
+using RI = std::move_iterator<int*>;
+using CRI = std::move_iterator<const int*>;
+
+static_assert( std::three_way_comparable_with<RI, CRI> );
+
+static_assert( std::is_convertible_v<const RI&, CRI> );
+static_assert( ! std::is_convertible_v<const CRI&, RI> );
+
+static_assert( std::is_assignable_v<CRI, const RI&> );
+static_assert( ! std::is_assignable_v<RI, const CRI&> );
diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/dr3435.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/dr3435.cc
new file mode 100644
index 00000000000..77b0f39c535
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/dr3435.cc
@@ -0,0 +1,36 @@
+// Copyright (C) 2020 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 <iterator>
+#include <compare>
+
+// LWG 3435.
+// three_way_comparable_with<reverse_iterator<int*>, reverse_iterator<const int*>>
+
+using RI = std::reverse_iterator<int*>;
+using CRI = std::reverse_iterator<const int*>;
+
+static_assert( std::three_way_comparable_with<RI, CRI> );
+
+static_assert( std::is_convertible_v<const RI&, CRI> );
+static_assert( ! std::is_convertible_v<const CRI&, RI> );
+
+static_assert( std::is_assignable_v<CRI, const RI&> );
+static_assert( ! std::is_assignable_v<RI, const CRI&> );
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [committed] libstdc++: Constrain reverse_iterator and move_iterator conversions [LWG 3435]
2020-07-22 14:10 [committed] libstdc++: Constrain reverse_iterator and move_iterator conversions [LWG 3435] Jonathan Wakely
@ 2020-07-31 22:49 ` Martin Jambor
2020-08-03 13:10 ` Jonathan Wakely
0 siblings, 1 reply; 3+ messages in thread
From: Martin Jambor @ 2020-07-31 22:49 UTC (permalink / raw)
To: Jonathan Wakely, libstdc++, gcc-patches
Hi Jonathan,
On Wed, Jul 22 2020, Jonathan Wakely via Gcc-patches wrote:
> libstdc++-v3/ChangeLog:
>
> * include/bits/stl_iterator.h (reverse_iterator): Constrain
> converting constructor and converting assignment operator.
> Access source iterator's data member directly instead of
> calling base().
> (move_iterator): Likewise.
> * testsuite/24_iterators/move_iterator/dr3435.cc: New test.
> * testsuite/24_iterators/reverse_iterator/dr3435.cc: New test.
>
> Tested powerpc64le-linux, committed to trunk.
>
with this patch, I fail to build 541.leela_r from SPEC INT 2017 even
when I specifically pass -std=c++98 to all compiler invocations. Is
that expected?
A simple reproducer is:
------------------------------ aaa.cpp ------------------------------
#include <vector>
typedef unsigned long long int uint64;
std::vector<uint64> ko_hash_history;
uint64 ko_hash;
bool bar(std::vector<uint64>::const_reverse_iterator fisrst,
std::vector<uint64>::const_reverse_iterator last,
std::vector<uint64>::const_reverse_iterator res);
bool foo(void) {
std::vector<uint64>::const_reverse_iterator first = ko_hash_history.rbegin();
std::vector<uint64>::const_reverse_iterator last = ko_hash_history.rend();
std::vector<uint64>::const_reverse_iterator res;
return bar(first, last, res);
}
----------------------------------------------------------------------
$ /home/mjambor/gcc/mine/inst/bin/g++ -std=c++98 -m64 -c -O2 -g aaa.cpp
In file included from /home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_algobase.h:67,
from /home/mjambor/gcc/mine/inst/include/c++/11.0.0/vector:60,
from aaa.cpp:1:
/home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_iterator.h: In instantiation of ‘std::reverse_iterator<_Iterator>::reverse_iterator(const std::reverse_iterator<_Iter>&) [with _Iter = __gnu_cxx::__normal_iterator<long long unsigned int*, std::vector<long long unsigned int> >; _Iterator = __gnu_cxx::__normal_iterator<const long long unsigned int*, std::vector<long long unsigned int> >]’:
aaa.cpp:12:80: required from here
/home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_iterator.h:203:23: error: ‘__gnu_cxx::__normal_iterator<long long unsigned int*, std::vector<long long unsigned int> > std::reverse_iterator<__gnu_cxx::__normal_iterator<long long unsigned int*, std::vector<long long unsigned int> > >::current’ is protected within this context
203 | : current(__x.current) { }
| ~~~~^~~~~~~
/home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_iterator.h:146:17: note: declared protected here
146 | _Iterator current;
| ^~~~~~~
...which is exactly the error I get from leela. The error goes away
when I use -std=c++11 instead (I promise I'll switch to building my
benchmarks with that or c++14 over the remaining of this year :-).
Is there a problem in the test (and the benchmark)? Should I file a PR?
Thanks,
Martin
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [committed] libstdc++: Constrain reverse_iterator and move_iterator conversions [LWG 3435]
2020-07-31 22:49 ` Martin Jambor
@ 2020-08-03 13:10 ` Jonathan Wakely
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2020-08-03 13:10 UTC (permalink / raw)
To: Martin Jambor; +Cc: libstdc++, gcc-patches
On 01/08/20 00:49 +0200, Martin Jambor wrote:
>Hi Jonathan,
>
>On Wed, Jul 22 2020, Jonathan Wakely via Gcc-patches wrote:
>> libstdc++-v3/ChangeLog:
>>
>> * include/bits/stl_iterator.h (reverse_iterator): Constrain
>> converting constructor and converting assignment operator.
>> Access source iterator's data member directly instead of
>> calling base().
>> (move_iterator): Likewise.
>> * testsuite/24_iterators/move_iterator/dr3435.cc: New test.
>> * testsuite/24_iterators/reverse_iterator/dr3435.cc: New test.
>>
>> Tested powerpc64le-linux, committed to trunk.
>>
>
>with this patch, I fail to build 541.leela_r from SPEC INT 2017 even
>when I specifically pass -std=c++98 to all compiler invocations. Is
>that expected?
>
>A simple reproducer is:
>
>------------------------------ aaa.cpp ------------------------------
>#include <vector>
>
>typedef unsigned long long int uint64;
>std::vector<uint64> ko_hash_history;
>uint64 ko_hash;
>
>bool bar(std::vector<uint64>::const_reverse_iterator fisrst,
> std::vector<uint64>::const_reverse_iterator last,
> std::vector<uint64>::const_reverse_iterator res);
>
>bool foo(void) {
> std::vector<uint64>::const_reverse_iterator first = ko_hash_history.rbegin();
> std::vector<uint64>::const_reverse_iterator last = ko_hash_history.rend();
> std::vector<uint64>::const_reverse_iterator res;
>
> return bar(first, last, res);
>}
>----------------------------------------------------------------------
>
>$ /home/mjambor/gcc/mine/inst/bin/g++ -std=c++98 -m64 -c -O2 -g aaa.cpp
>
>In file included from /home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_algobase.h:67,
> from /home/mjambor/gcc/mine/inst/include/c++/11.0.0/vector:60,
> from aaa.cpp:1:
>/home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_iterator.h: In instantiation of ‘std::reverse_iterator<_Iterator>::reverse_iterator(const std::reverse_iterator<_Iter>&) [with _Iter = __gnu_cxx::__normal_iterator<long long unsigned int*, std::vector<long long unsigned int> >; _Iterator = __gnu_cxx::__normal_iterator<const long long unsigned int*, std::vector<long long unsigned int> >]’:
>aaa.cpp:12:80: required from here
>/home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_iterator.h:203:23: error: ‘__gnu_cxx::__normal_iterator<long long unsigned int*, std::vector<long long unsigned int> > std::reverse_iterator<__gnu_cxx::__normal_iterator<long long unsigned int*, std::vector<long long unsigned int> > >::current’ is protected within this context
> 203 | : current(__x.current) { }
> | ~~~~^~~~~~~
>/home/mjambor/gcc/mine/inst/include/c++/11.0.0/bits/stl_iterator.h:146:17: note: declared protected here
> 146 | _Iterator current;
> | ^~~~~~~
>
>...which is exactly the error I get from leela. The error goes away
>when I use -std=c++11 instead (I promise I'll switch to building my
>benchmarks with that or c++14 over the remaining of this year :-).
>
>Is there a problem in the test (and the benchmark)? Should I file a PR?
FTR I replied to Martin off-list, pointing out this was already filed
as https://gcc.gnu.org/PR96382 and is fixed already.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-03 13:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22 14:10 [committed] libstdc++: Constrain reverse_iterator and move_iterator conversions [LWG 3435] Jonathan Wakely
2020-07-31 22:49 ` Martin Jambor
2020-08-03 13:10 ` 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).