diff mbox series

libstdc++: Fix common_iterator::operator-> [PR95322]

Message ID 20200526191817.3306612-1-ppalka@redhat.com
State New
Headers show
Series libstdc++: Fix common_iterator::operator-> [PR95322] | expand

Commit Message

Patrick Palka May 26, 2020, 7:18 p.m. UTC
This patch fixes the definition of common_iterator::operator-> when the
underlying iterator type's operator* returns a non-reference.

The first problem is that the class __detail::_Common_iter_proxy is used
unqualified.  Fixing that revealed another problem: the class's template
friend declaration of common_iterator doesn't match up with the
definition of common_iterator, because the friend declaration isn't
constrained.

If we try to make the friend declaration match up by adding constraints,
we run into frontend bug PR93467.  So we currently can't correctly
express this friend relation between __detail::_Common_iter_proxy and
common_iterator.

As a workaround to this frontend bug, this patch moves the definition of
_Common_iter_proxy into the class template of common_iterator so that we
could instead express the friend relation via the injected-class-name.

(This bug was found when attempting to use views::common to work around
the compile failure of the testcase in PR95322.)

Tested on x86_64-pc-linux-gnu, does this look OK to commit?

libstdc++-v3/ChangeLog:

	PR libstdc++/95322
	* include/bits/stl_iterator.h (__detail::_Common_iter_proxy):
	Remove and redefine it ...
	(common_iterator::_Proxy): ... here.
	(common_iterator::operator->): Use it.
	* testsuite/24_iterators/common_iterator/95322.cc: New test.
	* testsuite/std/ranges/adaptors/95322.cc: New test.
---
 libstdc++-v3/include/bits/stl_iterator.h      | 34 +++++-----
 .../24_iterators/common_iterator/2.cc         | 63 +++++++++++++++++++
 .../testsuite/std/ranges/adaptors/95322.cc    | 50 +++++++++++++++
 3 files changed, 129 insertions(+), 18 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/24_iterators/common_iterator/2.cc
 create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc

Comments

Jonathan Wakely May 26, 2020, 7:57 p.m. UTC | #1
On 26/05/20 15:18 -0400, Patrick Palka via Libstdc++ wrote:
>This patch fixes the definition of common_iterator::operator-> when the
>underlying iterator type's operator* returns a non-reference.
>
>The first problem is that the class __detail::_Common_iter_proxy is used
>unqualified.  Fixing that revealed another problem: the class's template
>friend declaration of common_iterator doesn't match up with the
>definition of common_iterator, because the friend declaration isn't
>constrained.
>
>If we try to make the friend declaration match up by adding constraints,
>we run into frontend bug PR93467.  So we currently can't correctly
>express this friend relation between __detail::_Common_iter_proxy and
>common_iterator.
>
>As a workaround to this frontend bug, this patch moves the definition of
>_Common_iter_proxy into the class template of common_iterator so that we
>could instead express the friend relation via the injected-class-name.

OK. I put it at namespace scope originally because it only depends on
the iterator not the sentinel, so we'd only instantiate it once for
common_iterator<X,Y> and common_iterator<X,Z>, but this is fine.

>(This bug was found when attempting to use views::common to work around
>the compile failure of the testcase in PR95322.)
>
>Tested on x86_64-pc-linux-gnu, does this look OK to commit?

Yes, thanks. OK for gcc-10 too.

Please be aware of the new ChangeLog policies announced on the lists
recently.
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index fdb1121f823..19b1d53f781 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1568,23 +1568,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   namespace __detail
   {
-    template<input_or_output_iterator _It>
-      class _Common_iter_proxy
-      {
-	iter_value_t<_It> _M_keep;
-
-	_Common_iter_proxy(iter_reference_t<_It>&& __x)
-	: _M_keep(std::move(__x)) { }
-
-	template<typename _Iter, typename _Sent>
-	  friend class common_iterator;
-
-      public:
-	const iter_value_t<_It>*
-	operator->() const
-	{ return std::__addressof(_M_keep); }
-      };
-
     template<typename _It>
       concept __common_iter_has_arrow = indirectly_readable<const _It>
 	&& (requires(const _It& __it) { __it.operator->(); }
@@ -1613,6 +1596,21 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_noexcept()
       { return _S_noexcept1<_It, _It2>() && _S_noexcept1<_Sent, _Sent2>(); }
 
+    class _Proxy
+    {
+      iter_value_t<_It> _M_keep;
+
+      _Proxy(iter_reference_t<_It>&& __x)
+      : _M_keep(std::move(__x)) { }
+
+      friend class common_iterator;
+
+    public:
+      const iter_value_t<_It>*
+      operator->() const
+      { return std::__addressof(_M_keep); }
+    };
+
   public:
     constexpr
     common_iterator()
@@ -1769,7 +1767,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return std::__addressof(__tmp);
 	}
       else
-	return _Common_iter_proxy(*_M_it);
+	return _Proxy{*_M_it};
     }
 
     common_iterator&
diff --git a/libstdc++-v3/testsuite/24_iterators/common_iterator/2.cc b/libstdc++-v3/testsuite/24_iterators/common_iterator/2.cc
new file mode 100644
index 00000000000..270e8eaa91c
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/common_iterator/2.cc
@@ -0,0 +1,63 @@ 
+// 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 run { target c++2a } }
+
+#include <iterator>
+#include <testsuite_hooks.h>
+
+struct value { int n; };
+
+struct sentinel { int limit; };
+
+struct iterator
+{
+  using iterator_category = std::input_iterator_tag;
+  using value_type = value;
+  using difference_type = std::ptrdiff_t;
+  using reference = value;
+
+  value operator*() const { return value{counter}; }
+
+  iterator& operator++() { ++counter; return *this; }
+
+  iterator operator++(int) { auto i = *this; ++counter; return i; }
+
+  bool operator==(sentinel s) const { return counter == s.limit; }
+
+  int counter = 0;
+};
+
+void
+test01()
+{
+  iterator i;
+  sentinel s{2};
+  std::common_iterator<iterator, sentinel> begin = i, end = s;
+  VERIFY( begin->n == 0 );
+  ++begin;
+  VERIFY( begin->n == 1 );
+  ++begin;
+  VERIFY( begin == end );
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc
new file mode 100644
index 00000000000..44619d6719a
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc
@@ -0,0 +1,50 @@ 
+// 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 run { target c++2a } }
+
+#include <ranges>
+#include <list>
+#include <testsuite_hooks.h>
+
+namespace ranges = std::ranges;
+namespace views = std::views;
+
+void
+test01()
+{
+  std::list container{1, 2, 3, 4, 5};
+  auto v = (container
+	    | views::take(3)
+	    | views::transform(std::negate{})
+	    | views::common);
+  auto i = ranges::cbegin(v);
+  VERIFY( *i == -1 );
+  ++i;
+  VERIFY( *i == -2 );
+  ++i;
+  VERIFY( *i == -3 );
+  ++i;
+  VERIFY( i == ranges::end(v) );
+}
+
+int
+main()
+{
+  test01();
+}