From 979e89a9a94f66241fa8355e2b2e8f4a680c83e1 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 27 May 2020 21:58:56 +0100 Subject: [PATCH] libstdc++: Fix std::reverse_iterator comparisons (PR 94354) The std::reverse_iterator comparisons have always been implemented only in terms of equality and less than. In C++98 that made no difference for reasonable code, because when the underlying operators are the same type they are required to support all comparisons anyway. But since LWG 280 it's possible to compare reverse_iterator and reverse_iterator, and comparisons between X and Y might not support the full set of equality and relational operators. This means that it matters whether we implement operator!= as x.base() != y.base() or !(x.base() == y.base()), and the current implementation is non-conforming. This was already fixed in GCC 10.1 for C++20, this change also fixes it for all other -std modes. PR libstdc++/94354 * include/bits/stl_iterator.h (reverse_iterator): Fix comparison operators to use the correct operations on the underlying iterators. * testsuite/24_iterators/reverse_iterator/rel_ops.cc: New test. --- libstdc++-v3/include/bits/stl_iterator.h | 11 +-- .../24_iterators/reverse_iterator/rel_ops.cc | 99 ++++++++++++++++++++++ 2 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index 19b1d53..b0f4549 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -393,6 +393,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 280. Comparison of reverse_iterator to const reverse_iterator. + template inline _GLIBCXX17_CONSTEXPR bool operator==(const reverse_iterator<_IteratorL>& __x, @@ -403,31 +404,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION inline _GLIBCXX17_CONSTEXPR bool operator<(const reverse_iterator<_IteratorL>& __x, const reverse_iterator<_IteratorR>& __y) - { return __y.base() < __x.base(); } + { return __x.base() > __y.base(); } template inline _GLIBCXX17_CONSTEXPR bool operator!=(const reverse_iterator<_IteratorL>& __x, const reverse_iterator<_IteratorR>& __y) - { return !(__x == __y); } + { return __x.base() != __y.base(); } template inline _GLIBCXX17_CONSTEXPR bool operator>(const reverse_iterator<_IteratorL>& __x, const reverse_iterator<_IteratorR>& __y) - { return __y < __x; } + { return __x.base() < __y.base(); } template inline _GLIBCXX17_CONSTEXPR bool operator<=(const reverse_iterator<_IteratorL>& __x, const reverse_iterator<_IteratorR>& __y) - { return !(__y < __x); } + { return __x.base() >= __y.base(); } template inline _GLIBCXX17_CONSTEXPR bool operator>=(const reverse_iterator<_IteratorL>& __x, const reverse_iterator<_IteratorR>& __y) - { return !(__x < __y); } + { return __x.base() <= __y.base(); } #else // C++20 template constexpr bool diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc new file mode 100644 index 0000000..4f2675f --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/rel_ops.cc @@ -0,0 +1,99 @@ +// 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 +// . + +// { dg-do compile } + +#include + +template +struct Iter +{ + typedef std::random_access_iterator_tag iterator_category; + typedef int value_type; + typedef int* pointer; + typedef int& reference; + typedef std::ptrdiff_t difference_type; + + Iter(); + + Iter& operator++(); + Iter operator++(int); + Iter& operator--(); + Iter operator--(int); + int& operator*() const; + int* operator->() const; + + int& operator[](difference_type) const; + + Iter& operator+=(difference_type); + Iter& operator-=(difference_type); + + template friend Iter operator+(Iter, difference_type); + template friend Iter operator+(difference_type, Iter); + template friend Iter operator-(Iter, difference_type); + template friend difference_type operator-(Iter, Iter); + + // Define the full set of operators for same-type comparisons + template friend bool operator==(Iter, Iter); // synthesizes != + template friend bool operator<(Iter, Iter); + template friend bool operator>(Iter, Iter); + template friend bool operator<=(Iter, Iter); + template friend bool operator>=(Iter, Iter); +}; + +// Define a single kind of mixed-type comparison for each specialization. +int operator==(Iter<0>, long*); +void* operator!=(Iter<1>, long*); +bool& operator< (Iter<2>, long*); +int operator> (Iter<3>, long*); +void* operator<=(Iter<4>, long*); +bool& operator>=(Iter<5>, long*); + +using std::reverse_iterator; + +reverse_iterator< Iter<0> > l0; +reverse_iterator< Iter<1> > l1; +reverse_iterator< Iter<2> > l2; +reverse_iterator< Iter<3> > l3; +reverse_iterator< Iter<4> > l4; +reverse_iterator< Iter<5> > l5; +reverse_iterator r; + +// PR libstdc++/94354 +// Check that these operators use the correct operator on the +// underlying iterator types. +bool b0 = l0 == r; +bool b1 = l1 != r; +bool b2 = l2 > r; +bool b3 = l3 < r; +bool b4 = l4 >= r; +bool b5 = l5 <= r; + +#if __cplusplus >= 201703L +int arr[3] = { 1, 2, 3 }; +constexpr std::reverse_iterator rbeg = std::rbegin(arr); +constexpr std::reverse_iterator crbeg = std::crbegin(arr); +static_assert( rbeg == crbeg ); +static_assert( rbeg <= crbeg ); +static_assert( rbeg >= crbeg ); +constexpr std::reverse_iterator crend = std::crend(arr); +static_assert( rbeg != crend ); +static_assert( rbeg < crend ); +static_assert( crend > rbeg ); +static_assert( rbeg <= crend ); +static_assert( crend >= rbeg ); +#endif // C++17 -- 2.7.4