Debug Mode ENH 4/4: Add special iterator support

Message ID 39e0d039-7d9a-d0c6-9553-c9c183a35c2e@gmail.com
State New
Headers show
Series
  • Debug Mode ENH 4/4: Add special iterator support
Related show

Commit Message

François Dumont May 8, 2018, 1:15 p.m.
Here is a patch to teach _Parameter type about special iterator types so 
that it improves final output.

It also get rid of the debug layer when possible so that failure output 
is cleaner. Debug mode is already transparent to users there is no need 
to show the Debug types in the output.

Here is the output for the newly added tests, for the move_iterator:

/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/vector:188:
In function:
     std::__debug::vector<_Tp, _Allocator>::vector(_InputIterator,
     _InputIterator, const _Allocator&) [with _InputIterator =
std::move_iterator<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, 

     std::vector<int> >, std::__debug::vector<int> >; 
<template-parameter-2-2> = void; _Tp
     = int; _Allocator = std::allocator<int>]

Backtrace:
     ./debug_neg.exe() [0x402956]
     ./debug_neg.exe() [0x402db5]
     ./debug_neg.exe() [0x4011b9]
     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) 
[0x7f3962ef8830]
     ./debug_neg.exe() [0x401219]

Error: function requires a valid iterator range [__first, __last).

Objects involved in the operation:
     iterator "__first" @ 0x0x7ffc704c05f0 {
       type = std::move_iterator<__gnu_cxx::__normal_iterator<int*, 
std::vector<int, std::allocator<int> > > > (mutable iterator);
       state = dereferenceable;
       references sequence with type 'std::__debug::vector<int, 
std::allocator<int> >' @ 0x0x7ffc704c07a0
     }
     iterator "__last" @ 0x0x7ffc704c05f0 {
       type = std::move_iterator<__gnu_cxx::__normal_iterator<int*, 
std::vector<int, std::allocator<int> > > > (mutable iterator);
       state = dereferenceable (start-of-sequence);
       references sequence with type 'std::__debug::vector<int, 
std::allocator<int> >' @ 0x0x7ffc704c07a0
     }
XFAIL: 24_iterators/move_iterator/debug_neg.cc execution test

For the reverse_iterator:

/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/vector:188:
In function:
     std::__debug::vector<_Tp, _Allocator>::vector(_InputIterator,
     _InputIterator, const _Allocator&) [with _InputIterator =
std::reverse_iterator<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, 

     std::vector<int> >, std::__debug::vector<int> >; 
<template-parameter-2-2> = void; _Tp
     = int; _Allocator = std::allocator<int>]

Backtrace:
     ./debug_neg.exe() [0x4020c1]
     ./debug_neg.exe() [0x400e59]
     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) 
[0x7f13fc56e830]
     ./debug_neg.exe() [0x400eb9]

Error: function requires a valid iterator range [__first, __last).

Objects involved in the operation:
     iterator "__first" @ 0x0x7ffc4e1f77d0 {
       type = std::reverse_iterator<__gnu_cxx::__normal_iterator<int*, 
std::vector<int, std::allocator<int> > > > (mutable iterator);
       state = past-the-reverse-end;
       references sequence with type 'std::__debug::vector<int, 
std::allocator<int> >' @ 0x0x7ffc4e1f7800
     }
     iterator "__last" @ 0x0x7ffc4e1f77d0 {
       type = std::reverse_iterator<__gnu_cxx::__normal_iterator<int*, 
std::vector<int, std::allocator<int> > > > (mutable iterator);
       state = dereferenceable (start-of-reverse-sequence);
       references sequence with type 'std::__debug::vector<int, 
std::allocator<int> >' @ 0x0x7ffc4e1f7800
     }
XFAIL: 24_iterators/reverse_iterator/debug_neg.cc execution test

Tested under Linux x8-_64.

I'll commit that tomorrow if not told otherwise.

     * include/debug/safe_iterator.h (_Safe_iterator<>::_M_constant()):
     Rename in...
     (_Safe_iterator<>::_S_constant()): ...that.
     * include/debug/safe_local_iterator.h
     (_Safe_local_iterator<>::_M_constant()): Rename in...
     (_Safe_local_iterator<>::_S_constant()): ...that.
     * include/debug/formatter.h: Remove bits/cpp_type_traits.h include.
     (_Iterator_state::__rbegin): New.
     (_Iterator_state::__rmiddle): New.
     (_Iterator_state::__rend): New.
     (_Parameter::_Parameter(const _Safe_iterator<>&, const char*,
     _Is_iterator)): Use _Safe_iterator<>::_S_constant. Grab normal 
underlying
     iterator type.
     (_Parameter::_Parameter(const _Safe_local_iterator<>&, const char*,
     _Is_iterator)): Likewise.
     (_Parameter::_S_reverse_state(_Iterator_state)): New.
         (_Parameter(__gnu_cxx::__normal_iterator<> const&, const char*,
     _Is_iterator)): New.
     (_Parameter(std::reverse_iterator<> const&, const char*,
     _Is_iterator)): New.
(_Parameter(std::reverse_iterator<_Safe_iterator<>> const&,
     const char*, _Is_iterator)): New.
     (_Parameter(std::move_iterator<> const&, const char*, _Is_iterator):
     New.
     (_Parameter(std::move_iterator<_Safe_iterator<>> const&, const char*,
     _Is_iterator)): New.
     * testsuite/24_iterators/move_iterator/debug_neg.cc: New.
     * testsuite/24_iterators/normal_iterator/debug_neg.cc: New.
     * testsuite/24_iterators/reverse_iterator/debug_neg.cc: New.

François

Patch

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index 2939203..7b3c30b 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -30,7 +30,6 @@ 
 #define _GLIBCXX_DEBUG_FORMATTER_H 1
 
 #include <bits/c++config.h>
-#include <bits/cpp_type_traits.h>
 
 #if __cpp_rtti
 # include <typeinfo>
@@ -43,6 +42,31 @@  namespace std
 # define _GLIBCXX_TYPEID(_Type) 0
 #endif
 
+#if __cplusplus >= 201103L
+namespace __gnu_cxx
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template<typename _Iterator, typename _Container>
+  class __normal_iterator;
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template<typename _Iterator>
+  class reverse_iterator;
+
+template<typename _Iterator>
+  class move_iterator;
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif
+
 namespace __gnu_debug
 {
   using std::type_info;
@@ -158,6 +182,9 @@  namespace __gnu_debug
       __middle,		// dereferenceable, not at the beginning
       __end,		// past-the-end, may be at beginning if sequence empty
       __before_begin,	// before begin
+      __rbegin,		// dereferenceable, and at the reverse-beginning
+      __rmiddle,	// reverse-dereferenceable, not at the reverse-beginning
+      __rend,		// reverse-past-the-end
       __last_state
     };
 
@@ -244,11 +271,9 @@  namespace __gnu_debug
 	{
 	  _M_variant._M_iterator._M_name = __name;
 	  _M_variant._M_iterator._M_address = std::__addressof(__it);
-	  _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it);
+	  _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(_Iterator);
 	  _M_variant._M_iterator._M_constness =
-	    std::__are_same<_Safe_iterator<_Iterator, _Sequence>,
-			    typename _Sequence::iterator>::
-	      __value ? __mutable_iterator : __const_iterator;
+	    __it._S_constant() ? __const_iterator : __mutable_iterator;
 	  _M_variant._M_iterator._M_sequence = __it._M_get_sequence();
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
@@ -274,11 +299,10 @@  namespace __gnu_debug
 	{
 	  _M_variant._M_iterator._M_name = __name;
 	  _M_variant._M_iterator._M_address = std::__addressof(__it);
-	  _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it);
+	  _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(_Iterator);
+	  _M_variant._M_iterator._M_constness =
 	  _M_variant._M_iterator._M_constness =
-	    std::__are_same<_Safe_local_iterator<_Iterator, _Sequence>,
-			    typename _Sequence::local_iterator>::
-	      __value ? __mutable_iterator : __const_iterator;
+	    __it._S_constant() ? __const_iterator : __mutable_iterator;
 	  _M_variant._M_iterator._M_sequence = __it._M_get_sequence();
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
@@ -335,6 +359,74 @@  namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = 0;
 	}
 
+#if __cplusplus >= 201103L
+      // The following constructors are only defined in C++11 to take
+      // advantage of the constructor delegation feature.
+      template<typename _Iterator, typename _Container>
+        _Parameter(
+	  __gnu_cxx::__normal_iterator<_Iterator, _Container> const& __it,
+	const char* __name, _Is_iterator)
+	: _Parameter(__it.base(), __name, _Is_iterator{})
+	{ _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); }
+
+      template<typename _Iterator>
+	_Parameter(std::reverse_iterator<_Iterator> const& __it,
+		   const char* __name, _Is_iterator)
+	: _Parameter(__it.base(), __name, _Is_iterator{})
+	{
+	  _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it);
+	  _M_variant._M_iterator._M_state
+	    = _S_reverse_state(_M_variant._M_iterator._M_state);
+	}
+
+      template<typename _Iterator, typename _Sequence>
+	_Parameter(std::reverse_iterator<_Safe_iterator<_Iterator,
+							_Sequence>> const& __it,
+		   const char* __name, _Is_iterator)
+	: _Parameter(__it.base(), __name, _Is_iterator{})
+	{
+	  _M_variant._M_iterator._M_type
+	    = _GLIBCXX_TYPEID(std::reverse_iterator<_Iterator>);
+	  _M_variant._M_iterator._M_state
+	    = _S_reverse_state(_M_variant._M_iterator._M_state);
+	}
+
+      template<typename _Iterator>
+	_Parameter(std::move_iterator<_Iterator> const& __it,
+		   const char* __name, _Is_iterator)
+	: _Parameter(__it.base(), __name, _Is_iterator{})
+	{ _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); }
+
+      template<typename _Iterator, typename _Sequence>
+	_Parameter(std::move_iterator<_Safe_iterator<_Iterator,
+						     _Sequence>> const& __it,
+		   const char* __name, _Is_iterator)
+	: _Parameter(__it.base(), __name, _Is_iterator{})
+      {
+	_M_variant._M_iterator._M_type
+	  = _GLIBCXX_TYPEID(std::move_iterator<_Iterator>);
+      }
+
+    private:
+      _Iterator_state
+      _S_reverse_state(_Iterator_state __state)
+      {
+	  switch (__state)
+	    {
+	    case __begin:
+	      return __rend;
+	    case __middle:
+	      return __rmiddle;
+	    case __end:
+	      return __rbegin;
+	    default:
+	      return __state;
+	    }
+      }
+
+    public:
+#endif
+
       template<typename _Sequence>
 	_Parameter(const _Safe_sequence<_Sequence>& __seq,
 		   const char* __name, _Is_sequence)
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index caf7d11..c61d480 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -91,11 +91,6 @@  namespace __gnu_debug
       typedef _Safe_iterator_base _Safe_base;
       typedef typename _Sequence::const_iterator _Const_iterator;
 
-      /// Determine if this is a constant iterator.
-      bool
-      _M_constant() const
-      { return std::__are_same<_Const_iterator, _Safe_iterator>::__value; }
-
       typedef std::iterator_traits<_Iterator> _Traits;
 
       struct _Attach_single
@@ -127,7 +122,7 @@  namespace __gnu_debug
        */
       _Safe_iterator(const _Iterator& __i, const _Safe_sequence_base* __seq)
       _GLIBCXX_NOEXCEPT
-      : _Iter_base(__i), _Safe_base(__seq, _M_constant())
+      : _Iter_base(__i), _Safe_base(__seq, _S_constant())
       {
 	_GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
 			      _M_message(__msg_init_singular)
@@ -396,6 +391,12 @@  namespace __gnu_debug
       }
 
       // ------ Utilities ------
+
+      /// Determine if this is a constant iterator.
+      static bool
+      _S_constant()
+      { return std::__are_same<_Const_iterator, _Safe_iterator>::__value; }
+
       /**
        * @brief Return the underlying iterator
        */
@@ -414,12 +415,12 @@  namespace __gnu_debug
       /** Attach iterator to the given sequence. */
       void
       _M_attach(_Safe_sequence_base* __seq)
-      { _Safe_base::_M_attach(__seq, _M_constant()); }
+      { _Safe_base::_M_attach(__seq, _S_constant()); }
 
       /** Likewise, but not thread-safe. */
       void
       _M_attach_single(_Safe_sequence_base* __seq)
-      { _Safe_base::_M_attach_single(__seq, _M_constant()); }
+      { _Safe_base::_M_attach_single(__seq, _S_constant()); }
 
       /// Is the iterator dereferenceable?
       bool
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index bb89aed..f9597a6 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -54,14 +54,6 @@  namespace __gnu_debug
       typedef typename _Sequence::const_local_iterator _Const_local_iterator;
       typedef typename _Sequence::size_type size_type;
 
-      /// Determine if this is a constant iterator.
-      bool
-      _M_constant() const
-      {
-	return std::__are_same<_Const_local_iterator,
-			       _Safe_local_iterator>::__value;
-      }
-
       typedef std::iterator_traits<_Iterator> _Traits;
 
       struct _Attach_single
@@ -92,7 +84,7 @@  namespace __gnu_debug
        */
       _Safe_local_iterator(const _Iterator& __i,
 			   const _Safe_sequence_base* __cont)
-      : _Iter_base(__i), _Safe_base(__cont, _M_constant())
+      : _Iter_base(__i), _Safe_base(__cont, _S_constant())
       {
 	_GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
 			      _M_message(__msg_init_singular)
@@ -278,6 +270,15 @@  namespace __gnu_debug
       }
 
       // ------ Utilities ------
+
+      /// Determine if this is a constant iterator.
+      static bool
+      _S_constant()
+      {
+	return std::__are_same<_Const_local_iterator,
+			       _Safe_local_iterator>::__value;
+      }
+
       /**
        * @brief Return the underlying iterator
        */
@@ -302,12 +303,12 @@  namespace __gnu_debug
       /** Attach iterator to the given sequence. */
       void
       _M_attach(_Safe_sequence_base* __seq)
-      { _Safe_base::_M_attach(__seq, _M_constant()); }
+      { _Safe_base::_M_attach(__seq, _S_constant()); }
 
       /** Likewise, but not thread-safe. */
       void
       _M_attach_single(_Safe_sequence_base* __seq)
-      { _Safe_base::_M_attach_single(__seq, _M_constant()); }
+      { _Safe_base::_M_attach_single(__seq, _S_constant()); }
 
       /// Is the iterator dereferenceable?
       bool
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index b1a6e21..cd1b43d 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -722,7 +722,10 @@  namespace
 		"dereferenceable (start-of-sequence)",
 		"dereferenceable",
 		"past-the-end",
-		"before-begin"
+		"before-begin",
+		"dereferenceable (start-of-reverse-sequence)",
+		"dereferenceable (reverse)",
+		"past-the-reverse-end"
 	      };
 	    print_word(ctx, state_names[iterator._M_state]);
 	  }
diff --git a/libstdc++-v3/testsuite/24_iterators/move_iterator/debug_neg.cc b/libstdc++-v3/testsuite/24_iterators/move_iterator/debug_neg.cc
new file mode 100644
index 0000000..d64fe8f
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/move_iterator/debug_neg.cc
@@ -0,0 +1,34 @@ 
+// { dg-do run { target c++11 xfail *-*-* } }
+
+// Copyright (C) 2018 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/>.
+
+#include <debug/vector>
+#include <iterator>
+
+void test01()
+{
+  __gnu_debug::vector<int> vals { 0, 1, 2, 3 };
+  __gnu_debug::vector<int> mval(std::make_move_iterator(vals.begin() + 1),
+				std::make_move_iterator(vals.begin()));
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/debug_neg.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/debug_neg.cc
new file mode 100644
index 0000000..4c866c6
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/debug_neg.cc
@@ -0,0 +1,34 @@ 
+// Copyright (C) 2018 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/>.
+
+// Requires C++11 because we check for correct output of
+// __gnu_cxx::__normal_iterator which is improved in this mode.
+// { dg-do run { target c++11 xfail *-*-* } }
+
+#include <debug/vector>
+
+void test01()
+{
+  std::vector<int> vals { 0, 1, 2, 3 };
+  __gnu_debug::vector<int> vals2(vals.begin() + 1, vals.begin());
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/debug_neg.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/debug_neg.cc
new file mode 100644
index 0000000..6c795c7
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/debug_neg.cc
@@ -0,0 +1,34 @@ 
+// Copyright (C) 2018 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/>.
+
+// Requires C++11 because we check for correct output of
+// std::reverse_iterator which is improved only in this mode.
+// { dg-do run { target c++11 xfail *-*-* } }
+
+#include <debug/vector>
+
+void test01()
+{
+  __gnu_debug::vector<int> vals { 0, 1, 2, 3 };
+  __gnu_debug::vector<int> vals2(vals.rend(), vals.rbegin());
+}
+
+int main()
+{
+  test01();
+  return 0;
+}