diff mbox series

libstdc++: Add a test that takes the split_view of a non-forward range

Message ID 20200311154654.837650-1-ppalka@redhat.com
State New
Headers show
Series libstdc++: Add a test that takes the split_view of a non-forward range | expand

Commit Message

Li, Pan2 via Gcc-patches March 11, 2020, 3:46 p.m. UTC 
This adds a tests that verifies taking the split_view of a non-forward range
works correctly.  Doing so revealed a typo in one of _OuterIter's constructors.

It also revealed that the default constructor of
__gnu_test::test_range::iterator misbehaves, because by delegating to
Iter<T>(nullptr, nullptr) we perform a null-pointer deref at runtime in
input_iterator_wrapper's constructor due to the ITERATOR_VERIFY check therein.
Instead of delegating to this constructor it seems we can just inherit the
protected default constructor, which does not contain this ITERATOR_VERIFY
check.

libstdc++-v3/ChangeLog:

	* include/std/ranges (split_view::_OuterIter::_OuterIter): Typo fix,
	'address' -> 'std::__addressof'.
	* testsuite/std/ranges/adaptors/split.cc: Test taking the split_view of
	a non-forward input_range.
	* testsuite/util/testsuite_iterators.h (output_iterator_wrapper): Make
	default constructor protected instead of deleted, like with
	input_iterator_wrapper.
	(test_range::iterator): Add comment explaining that this type is used
	only when the underlying wrapper is input_iterator_wrapper or
	output_iterator_wrapper.  Remove delegating defaulted constructor so
	that the inherited default constructor is used instead.
---
 libstdc++-v3/include/std/ranges               |  2 +-
 .../testsuite/std/ranges/adaptors/split.cc    | 20 +++++++++++++++++++
 .../testsuite/util/testsuite_iterators.h      | 14 ++++++++-----
 3 files changed, 30 insertions(+), 6 deletions(-)

Comments

Li, Pan2 via Gcc-patches March 11, 2020, 3:57 p.m. UTC | #1 
On 11/03/20 11:46 -0400, Patrick Palka wrote:
>This adds a tests that verifies taking the split_view of a non-forward range
>works correctly.  Doing so revealed a typo in one of _OuterIter's constructors.

Oops!

>It also revealed that the default constructor of
>__gnu_test::test_range::iterator misbehaves, because by delegating to
>Iter<T>(nullptr, nullptr) we perform a null-pointer deref at runtime in
>input_iterator_wrapper's constructor due to the ITERATOR_VERIFY check therein.

Oops for me this time.

>Instead of delegating to this constructor it seems we can just inherit the
>protected default constructor, which does not contain this ITERATOR_VERIFY
>check.

Yep, that makes much more sense.

OK for master, thanks.
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 23396947ac3..f4faec463e8 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -2786,7 +2786,7 @@  namespace views
 
 	  constexpr explicit
 	  _OuterIter(_Parent& __parent) requires (!forward_range<_Base>)
-	    : _M_parent(address(__parent))
+	    : _M_parent(std::__addressof(__parent))
 	  { }
 
 	  constexpr
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
index abdbfb41d8b..fe895827fc5 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
@@ -27,6 +27,7 @@ 
 
 using __gnu_test::test_range;
 using __gnu_test::forward_iterator_wrapper;
+using __gnu_test::input_iterator_wrapper;
 
 namespace ranges = std::ranges;
 namespace views = std::ranges::views;
@@ -133,6 +134,24 @@  test07()
   static_assert( noexcept(iter_swap(b, b2)) );
 }
 
+void
+test08()
+{
+  char x[] = "the quick brown fox";
+  test_range<char, input_iterator_wrapper> rx(x, x+sizeof(x)-1);
+  auto v = rx | views::split(' ');
+  auto i = v.begin();
+  VERIFY( ranges::equal(*i, "the"sv) );
+  ++i;
+  VERIFY( ranges::equal(*i, "quick"sv) );
+  ++i;
+  VERIFY( ranges::equal(*i, "brown"sv) );
+  ++i;
+  VERIFY( ranges::equal(*i, "fox"sv) );
+  ++i;
+  VERIFY( i == v.end() );
+}
+
 int
 main()
 {
@@ -143,4 +162,5 @@  main()
   test05();
   test06();
   test07();
+  test08();
 }
diff --git a/libstdc++-v3/testsuite/util/testsuite_iterators.h b/libstdc++-v3/testsuite/util/testsuite_iterators.h
index a915c02248b..5be47f47915 100644
--- a/libstdc++-v3/testsuite/util/testsuite_iterators.h
+++ b/libstdc++-v3/testsuite/util/testsuite_iterators.h
@@ -124,6 +124,11 @@  namespace __gnu_test
   struct output_iterator_wrapper
   : public std::iterator<std::output_iterator_tag, T, std::ptrdiff_t, T*, T&>
   {
+  protected:
+    output_iterator_wrapper() : ptr(0), SharedInfo(0)
+    { }
+
+  public:
     typedef OutputContainer<T> ContainerType;
     T* ptr;
     ContainerType* SharedInfo;
@@ -135,8 +140,6 @@  namespace __gnu_test
     }
 
 #if __cplusplus >= 201103L
-    output_iterator_wrapper() = delete;
-
     output_iterator_wrapper(const output_iterator_wrapper&) = default;
 
     output_iterator_wrapper&
@@ -706,13 +709,14 @@  namespace __gnu_test
   template<typename T, template<typename> class Iter>
     class test_range
     {
-      // Adds default constructor to Iter<T> if needed
+      // Exposes the protected default constructor of Iter<T> if needed.  This
+      // is needed only when Iter is input_iterator_wrapper or
+      // output_iterator_wrapper, because legacy forward iterators and beyond
+      // are already default constructible.
       struct iterator : Iter<T>
       {
 	using Iter<T>::Iter;
 
-	iterator() : Iter<T>(nullptr, nullptr) { }
-
 	using Iter<T>::operator++;
 
 	iterator& operator++() { Iter<T>::operator++(); return *this; }