* Hide move_iterator ill-form operators
@ 2019-05-06 17:36 François Dumont
2019-05-06 20:50 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: François Dumont @ 2019-05-06 17:36 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
Hi
   This is another attempt to make adapter iterator types operators
undefined when underlying iterator type doesn't support it. For the
move_iterator it is rather easy and even already done for the operator-
so I just generalize it to comparison operators. It doesn't cover all
operators of course but it is still better than current situation.
   * include/bits/stl_iterator.h (move_iterator<>::operator++(int)):
   Simplify implementation using underlying iterator type same
   post-increment operator.
   (move_iterator<>::operator--(int)):
   Simplify implementation using underlying iterator type same
   post-decrement operator.
   (move_iterator<>::operator<(const move_iterator<>&,
   const move_iterator<>&): Define return type as return type of the same
   expression on underlying iterator type.
   (move_iterator<>::operator<=(const move_iterator<>&,
   const move_iterator<>&): Likewise.
   (move_iterator<>::operator>(const move_iterator<>&,
   const move_iterator<>&): Likewise.
   (move_iterator<>::operator>=(const move_iterator<>&,
   const move_iterator<>&): Likewise.
   * testsuite/24_iterators/move_iterator/operator_neg.cc: New.
   Ok to commit or should the Standard be amended first ?
François
[-- Attachment #2: move_iterator.patch --]
[-- Type: text/x-patch, Size: 6673 bytes --]
diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 47be1a9dbcd..c1bbc75ca43 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1121,11 +1121,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX17_CONSTEXPR move_iterator
operator++(int)
- {
- move_iterator __tmp = *this;
- ++_M_current;
- return __tmp;
- }
+ { return move_iterator(_M_current++); }
_GLIBCXX17_CONSTEXPR move_iterator&
operator--()
@@ -1136,11 +1132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX17_CONSTEXPR move_iterator
operator--(int)
- {
- move_iterator __tmp = *this;
- --_M_current;
- return __tmp;
- }
+ { return move_iterator(_M_current--); }
_GLIBCXX17_CONSTEXPR move_iterator
operator+(difference_type __n) const
@@ -1197,51 +1189,59 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return !(__x == __y); }
template<typename _IteratorL, typename _IteratorR>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator<(const move_iterator<_IteratorL>& __x,
const move_iterator<_IteratorR>& __y)
+ -> decltype( __x.base() < __y.base() )
{ return __x.base() < __y.base(); }
template<typename _Iterator>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator<(const move_iterator<_Iterator>& __x,
const move_iterator<_Iterator>& __y)
+ -> decltype( __x.base() < __y.base() )
{ return __x.base() < __y.base(); }
template<typename _IteratorL, typename _IteratorR>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator<=(const move_iterator<_IteratorL>& __x,
const move_iterator<_IteratorR>& __y)
+ -> decltype( __x.base() <= __y.base() )
{ return !(__y < __x); }
template<typename _Iterator>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator<=(const move_iterator<_Iterator>& __x,
const move_iterator<_Iterator>& __y)
+ -> decltype( __x.base() <= __y.base() )
{ return !(__y < __x); }
template<typename _IteratorL, typename _IteratorR>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator>(const move_iterator<_IteratorL>& __x,
const move_iterator<_IteratorR>& __y)
+ -> decltype( __x.base() > __y.base() )
{ return __y < __x; }
template<typename _Iterator>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator>(const move_iterator<_Iterator>& __x,
const move_iterator<_Iterator>& __y)
+ -> decltype( __x.base() > __y.base() )
{ return __y < __x; }
template<typename _IteratorL, typename _IteratorR>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator>=(const move_iterator<_IteratorL>& __x,
const move_iterator<_IteratorR>& __y)
+ -> decltype( __x.base() >= __y.base() )
{ return !(__x < __y); }
template<typename _Iterator>
- inline _GLIBCXX17_CONSTEXPR bool
+ inline _GLIBCXX17_CONSTEXPR auto
operator>=(const move_iterator<_Iterator>& __x,
const move_iterator<_Iterator>& __y)
+ -> decltype( __x.base() >= __y.base() )
{ return !(__x < __y); }
// DR 685.
diff --git a/libstdc++-v3/testsuite/24_iterators/move_iterator/operator_neg.cc b/libstdc++-v3/testsuite/24_iterators/move_iterator/operator_neg.cc
new file mode 100644
index 00000000000..7b09425358e
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/move_iterator/operator_neg.cc
@@ -0,0 +1,73 @@
+// 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-do compile { target c++11 xfail *-*-* } }
+
+#include <forward_list>
+
+using ite_t =
+ std::move_iterator<typename std::forward_list<int>::iterator>;
+using const_ite_t =
+ std::move_iterator<typename std::forward_list<int>::const_iterator>;
+
+using lower_type =
+ decltype( std::declval<ite_t>() < std::declval<ite_t>() ); // { dg-error "no match for 'operator<'" }
+
+// { dg-error "no match for 'operator<'" "" { target *-*-* } 1195 }
+
+using heterogeneous_lower_type =
+ decltype( std::declval<const_ite_t>() < std::declval<ite_t>() ); // { dg-error "no match for 'operator<'" }
+
+// { dg-error "no match for 'operator<'" "" { target *-*-* } 1202 }
+
+using heterogeneous_lower_equal_type =
+ decltype( std::declval<const_ite_t>() <= std::declval<ite_t>() ); // { dg-error "no match for 'operator<='" }
+
+// { dg-error "no match for 'operator<='" "" { target *-*-* } 1209 }
+
+using lower_equal_type =
+ decltype( std::declval<ite_t>() <= std::declval<ite_t>() ); // { dg-error "no match for 'operator<='" }
+
+// { dg-error "no match for 'operator<='" "" { target *-*-* } 1216 }
+
+using heterogeneous_greater_type =
+ decltype( std::declval<const_ite_t>() > std::declval<ite_t>() ); // { dg-error "no match for 'operator>'" }
+
+// { dg-error "no match for 'operator>'" "" { target *-*-* } 1223 }
+
+using greater_type =
+ decltype( std::declval<ite_t>() > std::declval<ite_t>() ); // { dg-error "no match for 'operator>'" }
+
+// { dg-error "no match for 'operator>'" "" { target *-*-* } 1230 }
+
+using heterogeneous_greater_equal_type =
+ decltype( std::declval<const_ite_t>() >= std::declval<ite_t>() ); // { dg-error "no match for 'operator>='" }
+
+// { dg-error "no match for 'operator>='" "" { target *-*-* } 1237 }
+
+using greater_equal_type =
+ decltype( std::declval<ite_t>() >= std::declval<ite_t>() ); // { dg-error "no match for 'operator>='" }
+
+// { dg-error "no match for 'operator>='" "" { target *-*-* } 1244 }
+
+using diff_type =
+ decltype( std::declval<ite_t>() - std::declval<ite_t>() ); // { dg-error "no match for 'operator-'" }
+
+// { dg-error "no match for 'operator-'" "" { target *-*-* } 1252 }
+
+using integral_plus_ite_type =
+ decltype( 1 + std::declval<ite_t>() );
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Hide move_iterator ill-form operators
2019-05-06 17:36 Hide move_iterator ill-form operators François Dumont
@ 2019-05-06 20:50 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2019-05-06 20:50 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 06/05/19 19:36 +0200, François Dumont wrote:
>Hi
>
>Â Â Â This is another attempt to make adapter iterator types operators
>undefined when underlying iterator type doesn't support it. For the
>move_iterator it is rather easy and even already done for the
>operator- so I just generalize it to comparison operators. It doesn't
>cover all operators of course but it is still better than current
>situation.
>
>Â Â Â * include/bits/stl_iterator.h (move_iterator<>::operator++(int)):
>Â Â Â Simplify implementation using underlying iterator type same
>Â Â Â post-increment operator.
>Â Â Â (move_iterator<>::operator--(int)):
>Â Â Â Simplify implementation using underlying iterator type same
>Â Â Â post-decrement operator.
>Â Â Â (move_iterator<>::operator<(const move_iterator<>&,
>Â Â Â const move_iterator<>&): Define return type as return type of the same
>Â Â Â expression on underlying iterator type.
>Â Â Â (move_iterator<>::operator<=(const move_iterator<>&,
>Â Â Â const move_iterator<>&): Likewise.
>Â Â Â (move_iterator<>::operator>(const move_iterator<>&,
>Â Â Â const move_iterator<>&): Likewise.
>Â Â Â (move_iterator<>::operator>=(const move_iterator<>&,
>Â Â Â const move_iterator<>&): Likewise.
>Â Â Â * testsuite/24_iterators/move_iterator/operator_neg.cc: New.
>
>Â Â Â Ok to commit or should the Standard be amended first ?
Not OK.
The C++2a draft already solves the same problem, but differently.
Please follow the draft standard, instead of inventing something
different.
>François
>diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
>index 47be1a9dbcd..c1bbc75ca43 100644
>--- a/libstdc++-v3/include/bits/stl_iterator.h
>+++ b/libstdc++-v3/include/bits/stl_iterator.h
>@@ -1121,11 +1121,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> _GLIBCXX17_CONSTEXPR move_iterator
> operator++(int)
>- {
>- move_iterator __tmp = *this;
>- ++_M_current;
>- return __tmp;
>- }
>+ { return move_iterator(_M_current++); }
This is not what C++2a says.
>
> _GLIBCXX17_CONSTEXPR move_iterator&
> operator--()
>@@ -1136,11 +1132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> _GLIBCXX17_CONSTEXPR move_iterator
> operator--(int)
>- {
>- move_iterator __tmp = *this;
>- --_M_current;
>- return __tmp;
>- }
>+ { return move_iterator(_M_current--); }
This is not what C++2a says.
> _GLIBCXX17_CONSTEXPR move_iterator
> operator+(difference_type __n) const
>@@ -1197,51 +1189,59 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> { return !(__x == __y); }
>
> template<typename _IteratorL, typename _IteratorR>
>- inline _GLIBCXX17_CONSTEXPR bool
>+ inline _GLIBCXX17_CONSTEXPR auto
> operator<(const move_iterator<_IteratorL>& __x,
> const move_iterator<_IteratorR>& __y)
>+ -> decltype( __x.base() < __y.base() )
This is wrong, it needs to return bool, e.g.
-> decltype(bool(__x.base() < __y.base()))
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-05-06 20:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-06 17:36 Hide move_iterator ill-form operators François Dumont
2019-05-06 20:50 ` 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).