Fixed postfix increment for incrementable and single-pass iterator facade

[SVN r23456]

include/boost/iterator/iterator_categories.hpp

@@ -31,19 +31,39 @@ namespace boost {
 struct no_traversal_tag {};
 
 struct incrementable_traversal_tag 
-  : no_traversal_tag {};
+  : no_traversal_tag
+{
+//     incrementable_traversal_tag() {}
+//     incrementable_traversal_tag(std::output_iterator_tag const&) {};
+};
   
 struct single_pass_traversal_tag
-  : incrementable_traversal_tag {};
+  : incrementable_traversal_tag
+{
+//     single_pass_traversal_tag() {}
+//     single_pass_traversal_tag(std::input_iterator_tag const&) {};
+};
   
 struct forward_traversal_tag
-  : single_pass_traversal_tag {};
+  : single_pass_traversal_tag
+{
+//     forward_traversal_tag() {}
+//     forward_traversal_tag(std::forward_iterator_tag const&) {};
+};
   
 struct bidirectional_traversal_tag
-  : forward_traversal_tag {};
+  : forward_traversal_tag
+{
+//     bidirectional_traversal_tag() {};
+//     bidirectional_traversal_tag(std::bidirectional_iterator_tag const&) {};
+};
   
 struct random_access_traversal_tag
-  : bidirectional_traversal_tag {};
+  : bidirectional_traversal_tag
+{
+//     random_access_traversal_tag() {};
+//     random_access_traversal_tag(std::random_access_iterator_tag const&) {};
+};
 
 namespace detail
 {  

include/boost/iterator/iterator_facade.hpp

@@ -28,6 +28,7 @@
 #include <boost/mpl/apply_if.hpp>
 #include <boost/mpl/or.hpp>
 #include <boost/mpl/and.hpp>
+#include <boost/mpl/not.hpp>
 #include <boost/mpl/always.hpp>
 #include <boost/mpl/apply.hpp>
 
@@ -118,12 +119,87 @@ namespace boost
 # endif
     };
 
+    // iterators whose dereference operators reference the same value
+    // for all iterators into the same sequence (like many input
+    // iterators) need help with their postfix ++: the referenced
+    // value must be read and stored away before the increment occurs
+    // so that *a++ yields the originally referenced element and not
+    // the next one.
+    //
+    // In general, we can't determine that such an iterator isn't
+    // writable -- we also need to store a copy of the old iterator so
+    // that it can be written into.
+    template <class Iterator>
+    class postfix_increment_proxy
+    {
+        typedef typename iterator_value<Iterator>::type value_type;
+     public:
+        explicit postfix_increment_proxy(Iterator const& x)
+          : stored_value(*x)
+          , stored_iterator(x)
+        {}
+
+        // Dereferencing must return a proxy so that both *r++ = o and
+        // value_type(*r++) can work.  In this case, *r is the same as
+        // *r++, and the conversion operator below is used to ensure
+        // readability.
+        postfix_increment_proxy const&
+        operator*() const
+        {
+            return *this;
+        }
+
+        // Provides readability of *r++
+        operator value_type const&() const
+        {
+            return stored_value;
+        }
+
+        // Provides writability of *r++
+        template <class T>
+        T const& operator=(T const& x) const
+        {
+            *this->stored_iterator = x;
+            return x;
+        }
+
+        // This overload just in case only non-const objects are writable
+        template <class T>
+        T& operator=(T& x) const
+        {
+            *this->stored_iterator = x;
+            return x;
+        }
+     private:
+        value_type stored_value;
+        Iterator stored_iterator;
+    };
+
+    template <class Iterator, class Value, class Reference, class CategoryOrTraversal>
+    struct postfix_increment_result
+      : mpl::if_<
+            mpl::and_<
+                // This is only needed for readable iterators
+                is_convertible<Reference,Value>
+                
+                // No multipass iterator can have values that disappear
+                // before positions can be re-visited
+              , mpl::not_<
+                    is_convertible<
+                        typename iterator_category_to_traversal<CategoryOrTraversal>::type
+                      , forward_traversal_tag
+                    >
+                >
+            >
+          , postfix_increment_proxy<Iterator>
+          , Iterator
+        >
+    {};
 
     // operator->() needs special support for input iterators to strictly meet the
     // standard's requirements. If *i is not a reference type, we must still
     // produce a (constant) lvalue to which a pointer can be formed. We do that by
     // returning an instantiation of this special proxy class template.
-
     template <class T>
     struct operator_arrow_proxy
     {
@@ -138,7 +214,7 @@ namespace boost
     // A metafunction that gets the result type for operator->.  Also
     // has a static function make() which builds the result from a
     // Reference
-    template <class Value, class Reference, class Pointer>
+    template <class ValueType, class Reference, class Pointer>
     struct operator_arrow_result
     {
         // CWPro8.3 won't accept "operator_arrow_result::type", and we
@@ -147,7 +223,7 @@ namespace boost
         typedef typename mpl::if_<
             is_reference<Reference>
           , Pointer
-          , operator_arrow_proxy<Value>
+          , operator_arrow_proxy<ValueType>
         >::type type;
 
         static type make(Reference x)
@@ -165,13 +241,14 @@ namespace boost
     };
 # endif
 
-    //
-    // Iterator is actually an iterator_facade, so we do not have to
-    // go through iterator_traits to access the traits.
-    //
+    // A proxy return type for operator[], needed to deal with
+    // iterators that may invalidate referents upon destruction.
+    // Consider the temporary iterator in *(a + n)
     template <class Iterator>
     class operator_brackets_proxy
     {
+        // Iterator is actually an iterator_facade, so we do not have to
+        // go through iterator_traits to access the traits.
         typedef typename Iterator::reference  reference;
         typedef typename Iterator::value_type value_type;
 
@@ -195,13 +272,17 @@ namespace boost
         Iterator m_iter;
     };
 
-    template <class Value, class Reference>
+    // A metafunction that determines whether operator[] must return a
+    // proxy, or whether it can simply return a copy of the value_type.
+    template <class ValueType, class Reference>
     struct use_operator_brackets_proxy
-      : mpl::and_<
-            // Really we want an is_copy_constructible trait here,
-            // but is_POD will have to suffice in the meantime.
-            boost::is_POD<Value>
-          , iterator_writability_disabled<Value,Reference>
+      : mpl::not_<
+            mpl::and_<
+                // Really we want an is_copy_constructible trait here,
+                // but is_POD will have to suffice in the meantime.
+                boost::is_POD<ValueType>
+              , iterator_writability_disabled<ValueType,Reference>
+            >
         >
     {};
         
@@ -210,19 +291,19 @@ namespace boost
     {
         typedef typename mpl::if_<
             use_operator_brackets_proxy<Value,Reference>
-          , Value 
           , operator_brackets_proxy<Iterator>
+          , Value
         >::type type;
     };
 
     template <class Iterator>
-    operator_brackets_proxy<Iterator> make_operator_brackets_result(Iterator const& iter, mpl::false_)
+    operator_brackets_proxy<Iterator> make_operator_brackets_result(Iterator const& iter, mpl::true_)
     {
         return operator_brackets_proxy<Iterator>(iter);
     }
 
     template <class Iterator>
-    typename Iterator::value_type make_operator_brackets_result(Iterator const& iter, mpl::true_)
+    typename Iterator::value_type make_operator_brackets_result(Iterator const& iter, mpl::false_)
     {
       return *iter;
     }
@@ -385,6 +466,21 @@ namespace boost
           return f2.distance_to(f1);
       }
 
+      //
+      // Curiously Recurring Template interface.
+      //
+      template <class I, class V, class TC, class R, class D>
+      static I& derived(iterator_facade<I,V,TC,R,D>& facade)
+      {
+          return *static_cast<I*>(&facade);
+      }
+
+      template <class I, class V, class TC, class R, class D>
+      static I const& derived(iterator_facade<I,V,TC,R,D> const& facade)
+      {
+          return *static_cast<I const*>(&facade);
+      }
+
    private:
       // objects of this class are useless
       iterator_core_access(); //undefined
@@ -428,7 +524,7 @@ namespace boost
       typedef detail::iterator_facade_types<
          Value, CategoryOrTraversal, Reference, Difference
       > associated_types;
-      
+
    public:
 
       typedef typename associated_types::value_type value_type;
@@ -456,7 +552,7 @@ namespace boost
           >::make(*this->derived());
       }
         
-      typename detail::operator_brackets_result<Derived,Value,Reference>::type
+      typename detail::operator_brackets_result<Derived,Value,reference>::type
       operator[](difference_type n) const
       {
           typedef detail::use_operator_brackets_proxy<Value,Reference> use_proxy;
@@ -473,13 +569,17 @@ namespace boost
           return this->derived();
       }
 
-      Derived operator++(int)
+# if BOOST_WORKAROUND(BOOST_MSVC, == 1200)
+      typename detail::postfix_increment_result<Derived,Value,Reference,CategoryOrTraversal>::type
+      operator++(int)
       {
-          Derived tmp(this->derived());
+          typename detail::postfix_increment_result<Derived,Value,Reference,CategoryOrTraversal>::type
+          tmp(this->derived());
           ++*this;
           return tmp;
       }
-
+# endif
+      
       Derived& operator--()
       {
           iterator_core_access::decrement(this->derived());
@@ -524,8 +624,26 @@ namespace boost
 # endif
   };
 
+# if !BOOST_WORKAROUND(BOOST_MSVC, == 1200)
+  template <class I, class V, class TC, class R, class D>
+  typename detail::postfix_increment_result<I,V,R,TC>::type
+  operator++(
+      iterator_facade<I,V,TC,R,D>& i
+    , int
+  )
+  {
+      typename detail::postfix_increment_result<I,V,R,TC>::type
+          tmp(*static_cast<I*>(&i));
+      
+      ++i;
+      
+      return tmp;
+  }
+# endif 
+
+  
   //
-  // Operator implementation. The library supplied operators
+  // Comparison operator implementation. The library supplied operators
   // enables the user to provide fully interoperable constant/mutable
   // iterator types. I.e. the library provides all operators
   // for all mutable/constant iterator combinations.

include/boost/iterator/new_iterator_tests.hpp

@@ -28,9 +28,20 @@
 # include <boost/iterator/is_lvalue_iterator.hpp>
 
 # include <boost/iterator/detail/config_def.hpp>
+# include <boost/detail/is_incrementable.hpp>
 
 namespace boost {
 
+template <class Iterator, class T>
+void readable_iterator_test_aux(Iterator i1, T v, mpl::true_)
+{
+    assert(v == *i1++);
+}
+
+template <class Iterator, class T>
+void readable_iterator_test_aux(const Iterator i1, T v, mpl::false_)
+{}
+
 // Preconditions: *i == v
 template <class Iterator, class T>
 void readable_iterator_test(const Iterator i1, T v)
@@ -45,6 +56,8 @@ void readable_iterator_test(const Iterator i1, T v)
   assert(v2 == v);
 
 # if !BOOST_WORKAROUND(__MWERKS__, <= 0x2407)
+  readable_iterator_test_aux(i1, v, detail::is_postfix_incrementable<Iterator>());
+      
   // I think we don't really need this as it checks the same things as
   // the above code.
   BOOST_STATIC_ASSERT(is_readable_iterator<Iterator>::value);

test/Jamfile

@@ -42,6 +42,7 @@ test-suite iterator
     [ run indirect_iterator_test.cpp ]
     [ compile indirect_iterator_member_types.cpp ]
     [ run filter_iterator_test.cpp ]
+    [ run iterator_facade.cpp ]
     [ run reverse_iterator_test.cpp ]
     [ run counting_iterator_test.cpp ]
     [ run interoperable.cpp ]

test/iterator_facade.cpp

@@ -0,0 +1,67 @@
+// Copyright David Abrahams 2004. Distributed under the Boost
+// Software License, Version 1.0. (See accompanying
+// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+
+// This is really an incomplete test; should be fleshed out.
+
+#include <boost/iterator/iterator_facade.hpp>
+#include <boost/iterator/new_iterator_tests.hpp>
+
+// This is a really, really limited test so far.  All we're doing
+// right now is checking that the postfix++ proxy for single-pass
+// iterators works properly.
+template <class Ref>
+class counter_iterator
+  : public boost::iterator_facade<
+        counter_iterator<Ref>
+      , int const
+      , boost::single_pass_traversal_tag
+      , Ref
+    >
+{
+ public:
+    counter_iterator() {}
+    counter_iterator(int* state) : state(state) {}
+
+    void increment()
+    {
+        ++*state;
+    }
+
+    Ref
+    dereference() const
+    {
+        return *state;
+    }
+
+    bool equal(counter_iterator const& y) const
+    {
+        return *this->state == *y.state;
+    }
+
+    int* state;
+};
+
+struct proxy
+{
+    proxy(int& x) : state(x) {}
+        
+    operator int const&() const
+    {
+        return state;
+    }
+
+    int& operator=(int x) { state = x; return state; }
+        
+    int& state;
+};
+    
+int main()
+{
+    int state = 0;
+    boost::readable_iterator_test(counter_iterator<int const&>(&state), 0);
+    state = 3;
+    boost::readable_iterator_test(counter_iterator<proxy>(&state), 3);
+    boost::writable_iterator_test(counter_iterator<proxy>(&state), 9);
+    return 0;
+}