Repair postfix increment proxies for input iterators

[SVN r23508]

include/boost/iterator/iterator_facade.hpp

@@ -26,15 +26,13 @@
 #include <boost/type_traits/is_pod.hpp>
 
 #include <boost/mpl/apply_if.hpp>
+#include <boost/mpl/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>
-
+#include <boost/mpl/identity.hpp>
-#if BOOST_WORKAROUND(BOOST_MSVC, == 1200)
-# include <boost/mpl/if.hpp>
-#endif
 
 #include <boost/iterator/detail/config_def.hpp> // this goes last
 
@@ -125,10 +123,6 @@ namespace boost
     // 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
     {
@@ -136,6 +130,32 @@ namespace boost
      public:
         explicit postfix_increment_proxy(Iterator const& x)
           : stored_value(*x)
+        {}
+
+        // Returning a mutable reference allows nonsense like
+        // (*r++).mutate(), but it imposes fewer assumptions about the
+        // behavior of the value_type.  In particular, recall taht
+        // (*r).mutate() is legal if operator* returns by value.
+        value_type&
+        operator*() const
+        {
+            return this->stored_value;
+        }
+     private:
+        mutable value_type stored_value;
+    };
+    
+    //
+    // 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 writable_postfix_increment_proxy
+    {
+        typedef typename iterator_value<Iterator>::type value_type;
+     public:
+        explicit writable_postfix_increment_proxy(Iterator const& x)
+          : stored_value(*x)
           , stored_iterator(x)
         {}
 
@@ -143,14 +163,14 @@ namespace boost
         // 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&
+        writable_postfix_increment_proxy const&
         operator*() const
         {
             return *this;
         }
 
         // Provides readability of *r++
-        operator value_type const&() const
+        operator value_type&() const
         {
             return stored_value;
         }
@@ -171,16 +191,31 @@ namespace boost
             return x;
         }
      private:
-        value_type stored_value;
+        mutable value_type stored_value;
         Iterator stored_iterator;
     };
 
+    // A metafunction to choose the result type of postfix ++
+    //
+    // Because the C++98 input iterator requirements say that *r++ has
+    // type T (value_type), implementations of some standard
+    // algorithms like lexicographical_compare may use constructions
+    // like:
+    //
+    //          *r++ < *s++
+    //
+    // If *r++ returns a proxy (as required if r is writable but not
+    // multipass), this sort of expression will fail unless the proxy
+    // supports the operator<.  Since there are any number of such
+    // operations, we're not going to try to support them.  Therefore,
+    // even if r++ returns a proxy, *r++ will only return a proxy if
+    // CategoryOrTraversal is convertible to std::output_iterator_tag.
     template <class Iterator, class Value, class Reference, class CategoryOrTraversal>
     struct postfix_increment_result
-      : mpl::if_<
+      : mpl::apply_if<
             mpl::and_<
-                // This is only needed for readable iterators
+            // A proxy is only needed for readable iterators
-                is_convertible<Reference,Value>
+            is_convertible<Reference,Value>
                 
                 // No multipass iterator can have values that disappear
                 // before positions can be re-visited
@@ -191,8 +226,12 @@ namespace boost
                     >
                 >
             >
-          , postfix_increment_proxy<Iterator>
+          , mpl::if_<
-          , Iterator
+                is_convertible<CategoryOrTraversal,std::output_iterator_tag>
+              , writable_postfix_increment_proxy<Iterator>
+              , postfix_increment_proxy<Iterator>
+            >
+          , mpl::identity<Iterator>
         >
     {};
 

include/boost/iterator/new_iterator_tests.hpp

@@ -32,16 +32,32 @@
 
 namespace boost {
 
+
+// Do separate tests for *i++ so we can treat, e.g., smart pointers,
+// as readable and/or writable iterators.
 template <class Iterator, class T>
-void readable_iterator_test_aux(Iterator i1, T v, mpl::true_)
+void readable_iterator_traversal_test(Iterator i1, T v, mpl::true_)
 {
-    assert(v == *i1++);
+    T v2 = *i1++;
+    assert(v == v2);
 }
 
 template <class Iterator, class T>
-void readable_iterator_test_aux(const Iterator i1, T v, mpl::false_)
+void readable_iterator_traversal_test(const Iterator i1, T v, mpl::false_)
 {}
 
+template <class Iterator, class T>
+void writable_iterator_traversal_test(Iterator i1, T v, mpl::true_)
+{
+    ++i1;  // we just wrote into that position
+    *i1++ = v;
+}
+
+template <class Iterator, class T>
+void writable_iterator_traversal_test(const Iterator i1, T v, mpl::false_)
+{}
+
+
 // Preconditions: *i == v
 template <class Iterator, class T>
 void readable_iterator_test(const Iterator i1, T v)
@@ -56,7 +72,7 @@ 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>());
+  readable_iterator_traversal_test(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.
@@ -65,10 +81,18 @@ void readable_iterator_test(const Iterator i1, T v)
 }
 
 template <class Iterator, class T>
-void writable_iterator_test(Iterator i, T v)
+void writable_iterator_test(Iterator i, T v, T v2)
 {
   Iterator i2(i); // Copy Constructible
   *i2 = v;
+
+# if !BOOST_WORKAROUND(__MWERKS__, <= 0x2407)
+  writable_iterator_traversal_test(
+      i, v2, mpl::and_<
+          detail::is_incrementable<Iterator>
+        , detail::is_postfix_incrementable<Iterator>
+      >());
+# endif 
 }
 
 template <class Iterator>

test/iterator_facade.cpp

@@ -6,16 +6,17 @@
 
 #include <boost/iterator/iterator_facade.hpp>
 #include <boost/iterator/new_iterator_tests.hpp>
+#include <boost/assert.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>
+template <class Ref, class CategoryOrTraversal = boost::single_pass_traversal_tag>
 class counter_iterator
   : public boost::iterator_facade<
-        counter_iterator<Ref>
+        counter_iterator<Ref, CategoryOrTraversal>
       , int const
-      , boost::single_pass_traversal_tag
+      , CategoryOrTraversal
       , Ref
     >
 {
@@ -62,6 +63,9 @@ int main()
     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);
+    state = 5;
+    boost::readable_iterator_test(counter_iterator<proxy,std::output_iterator_tag>(&state), 5);
+    boost::writable_iterator_test(counter_iterator<proxy,std::output_iterator_tag>(&state), 9, 7);
+    BOOST_ASSERT(state == 7);
     return 0;
 }