Re: [v3] Add missing forward_list<>::splice_after and merge overloads fix splice_after taking a range

[François Dumont <frs dot dumont at gmail dot com>]

One thing after the other :-)

Here is a patch to fix this issue Paolo. The safe iterator 
_M_valid_range method was considering that a range with different 
iterators and last being a begin one is invalid which is wrong when 
there is a before_begin.

2012-04-12  François Dumont <fdumont@gcc.gnu.org>

    * include/debug/safe_iterator.h (_BeforeBeginHelper<>::__value): Add.
    (_Safe_Iterator<>::_M_is_beginnest()): Add.
    * include/debug/safe_iterator.tcc
    (_Safe_Iterator<>::_M_valid_range): Use latter.
    * include/debug/forward_list (_BeforeBeginHelper<>::__value): Add.
    * testsuite/23_containers/forward_list/debug/splice_after.cc:
    Add check.

Tested in 4.7 branch linux x86_64.

I did this patch in the 4.7 branch because I only had this one ready and 
moreover I think it should be integrated in it.

Ok ?

François


On 04/11/2012 11:57 PM, Jonathan Wakely wrote:
> On 11 April 2012 11:32, Paolo Carlini wrote:
> > Francois, can you please review the debug-mode checks for cases like:
> >
> >     std::forward_list<int>  fl1(1), fl2(1);
> >     fl1.splice_after(fl1.before_begin(), fl2, fl2.before_begin(),
> > fl2.begin());
> >
> > ?
> >
> > I don't think we should error out. We don't for things like:
> >
> >     std::list<int>  fl1(1), fl2(1);
> >     fl1.splice(fl1.begin(), fl2, fl2.begin(), fl2.begin());
> >
> > ie, the source is in both cases just an empty range, not an invalid range.
> btw, in debug mode we should be checking fl1.get_allocator() ==
> fl2.get_allocator() for both forward_list and list.

Index: include/debug/safe_iterator.tcc
===================================================================
--- include/debug/safe_iterator.tcc	(revision 186244)
+++ include/debug/safe_iterator.tcc	(working copy)
@@ -92,9 +92,9 @@
 	   iterators is at an extreme. */
 	/* Optim for classic [begin, it) or [it, end) ranges, limit checks
 	 * when code is valid. */
-	if (_M_is_begin() || __rhs._M_is_end())
+	if (_M_is_beginnest() || __rhs._M_is_end())
 	  return true;
-	if (_M_is_end() || __rhs._M_is_begin())
+	if (_M_is_end() || __rhs._M_is_beginnest())
 	  return false;
 
 	// Assume that this is a valid range; we can't check anything else
Index: include/debug/forward_list
===================================================================
--- include/debug/forward_list	(revision 186244)
+++ include/debug/forward_list	(working copy)
@@ -732,6 +732,8 @@
       typedef typename _Sequence::const_iterator _It;
       typedef typename _It::iterator_type _BaseIt;
 
+      static const bool __value = true;
+
       static bool
       _M_Is(_BaseIt __it, const _Sequence* __seq)
       { return __it == __seq->_M_base().cbefore_begin(); }
Index: include/debug/safe_iterator.h
===================================================================
--- include/debug/safe_iterator.h	(revision 186244)
+++ include/debug/safe_iterator.h	(working copy)
@@ -49,6 +49,8 @@
       typedef typename _Sequence::const_iterator _It;
       typedef typename _It::iterator_type _BaseIt;
 
+      static const bool __value = false;
+
       static bool
       _M_Is(_BaseIt __it, const _Sequence* __seq)
       { return false; }
@@ -464,6 +466,15 @@
       {
 	return _BeforeBeginHelper<_Sequence>::_M_Is(base(), _M_get_sequence());
       }
+
+      /// Is this iterator equal to the sequence's before_begin() iterator if
+      /// any or to the sequence's begin() otherwise ?
+      bool _M_is_beginnest() const
+      {
+	return _BeforeBeginHelper<_Sequence>::__value
+	  ? _M_is_before_begin()
+	  : _M_is_begin();
+      }
     };
 
   template<typename _IteratorL, typename _IteratorR, typename _Sequence>
Index: testsuite/23_containers/forward_list/debug/splice_after.cc
===================================================================
--- testsuite/23_containers/forward_list/debug/splice_after.cc	(revision 186244)
+++ testsuite/23_containers/forward_list/debug/splice_after.cc	(working copy)
@@ -34,6 +34,10 @@
 
   VERIFY( before == fl1.before_begin() );
   VERIFY( end == fl1.end() );
+
+  // no-op just to check that debug mode do not see any problem with it.
+  fl1.splice_after(fl1.before_begin(), std::move(fl2),
+		   fl2.before_begin(), fl2.begin());
 }
 
 int