Fixed accessing members of the dereferenced value after iterator post-increment.

The recent commit 5777e9944b broke code such as
(*it++).foo(), where the result of dereferencing would be convertible to
the value type but did not provide the members of the value type. To mitigate
this, return a reference to the value instead of a proxy object. This will only
work for non-writable iterators (and it didn't work for writable iterators
before either) because in that case a proxy is needed to be able to intercept
operator=.

Also fix a similar issue with (it++)->foo() by adding operator-> overloads
to the post-increment result proxies.

Added tests for the fixes.

.github/workflows/ci.yml

@@ -1,4 +1,4 @@
-# Copyright 2021 Andrey Semashev
+# Copyright 2021-2022 Andrey Semashev
 #
 # Distributed under the Boost Software License, Version 1.0.
 # (See accompanying file LICENSE_1_0.txt or copy at http://boost.org/LICENSE_1_0.txt)
@@ -35,7 +35,7 @@ jobs:
           # Linux, gcc
           - toolset: gcc-4.4
             cxxstd: "98,0x"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - g++-4.4
@@ -43,7 +43,7 @@ jobs:
               - "ppa:ubuntu-toolchain-r/test"
           - toolset: gcc-4.6
             cxxstd: "03,0x"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - g++-4.6
@@ -51,45 +51,49 @@ jobs:
               - "ppa:ubuntu-toolchain-r/test"
           - toolset: gcc-4.7
             cxxstd: "03,11"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - g++-4.7
           - toolset: gcc-4.8
             cxxstd: "03,11"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - g++-4.8
           - toolset: gcc-4.9
             cxxstd: "03,11"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - g++-4.9
           - toolset: gcc-5
             cxxstd: "03,11,14,1z"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - g++-5
           - toolset: gcc-6
             cxxstd: "03,11,14,1z"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - g++-6
           - toolset: gcc-7
             cxxstd: "03,11,14,17"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - g++-7
           - toolset: gcc-8
             cxxstd: "03,11,14,17,2a"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - g++-8
           - toolset: gcc-9
             cxxstd: "03,11,14,17,2a"
-            os: ubuntu-18.04
+            os: ubuntu-20.04
             install:
               - g++-9
           - toolset: gcc-10
@@ -98,87 +102,94 @@ jobs:
             install:
               - g++-10
           - toolset: gcc-11
-            cxxstd: "03,11,14,17,20"
-            os: ubuntu-20.04
+            cxxstd: "03,11,14,17,20,23"
+            os: ubuntu-22.04
             install:
               - g++-11
-            sources:
-              - "ppa:ubuntu-toolchain-r/test"
+          - toolset: gcc-12
+            cxxstd: "03,11,14,17,20,23"
+            os: ubuntu-22.04
+            install:
+              - g++-12
           - name: UBSAN
             toolset: gcc-11
-            cxxstd: "03,11,14,17,20"
+            cxxstd: "03,11,14,17,20,23"
             ubsan: 1
             build_variant: debug
-            os: ubuntu-20.04
+            os: ubuntu-22.04
             install:
               - g++-11
-            sources:
-              - "ppa:ubuntu-toolchain-r/test"
 
           # Linux, clang
           - toolset: clang
             compiler: clang++-3.5
             cxxstd: "03,11"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - clang-3.5
           - toolset: clang
             compiler: clang++-3.6
             cxxstd: "03,11,14"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - clang-3.6
           - toolset: clang
             compiler: clang++-3.7
             cxxstd: "03,11,14"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - clang-3.7
           - toolset: clang
             compiler: clang++-3.8
             cxxstd: "03,11,14"
-            os: ubuntu-20.04
+            os: ubuntu-latest
             container: ubuntu:16.04
             install:
               - clang-3.8
           - toolset: clang
             compiler: clang++-3.9
             cxxstd: "03,11,14"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - clang-3.9
           - toolset: clang
             compiler: clang++-4.0
             cxxstd: "03,11,14"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - clang-4.0
           - toolset: clang
             compiler: clang++-5.0
             cxxstd: "03,11,14,1z"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - clang-5.0
           - toolset: clang
             compiler: clang++-6.0
             cxxstd: "03,11,14,17"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - clang-6.0
           - toolset: clang
             compiler: clang++-7
             cxxstd: "03,11,14,17"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - clang-7
           # Note: clang-8 does not fully support C++20, so it is not compatible with libstdc++-8 in this mode
           - toolset: clang
             compiler: clang++-8
             cxxstd: "03,11,14,17,2a"
-            os: ubuntu-18.04
+            os: ubuntu-latest
+            container: ubuntu:18.04
             install:
               - clang-8
               - g++-7
@@ -198,42 +209,68 @@ jobs:
           - toolset: clang
             compiler: clang++-11
             cxxstd: "03,11,14,17,20"
-            os: ubuntu-20.04
+            os: ubuntu-22.04
             install:
               - clang-11
           - toolset: clang
             compiler: clang++-12
-            cxxstd: "03,11,14,17,20"
-            os: ubuntu-20.04
+            cxxstd: "03,11,14,17,20,2b"
+            os: ubuntu-22.04
             install:
               - clang-12
           - toolset: clang
-            compiler: clang++-12
-            cxxstd: "03,11,14,17,20"
-            os: ubuntu-20.04
+            compiler: clang++-13
+            cxxstd: "03,11,14,17,20,2b"
+            os: ubuntu-22.04
             install:
-              - clang-12
-              - libc++-12-dev
-              - libc++abi-12-dev
+              - clang-13
+          - toolset: clang
+            compiler: clang++-14
+            cxxstd: "03,11,14,17,20,2b"
+            os: ubuntu-22.04
+            install:
+              - clang-14
+          - toolset: clang
+            compiler: clang++-15
+            cxxstd: "03,11,14,17,20,2b"
+            os: ubuntu-22.04
+            install:
+              - clang-15
+            sources:
+              - "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main"
+            source_keys:
+              - "https://apt.llvm.org/llvm-snapshot.gpg.key"
+          - toolset: clang
+            compiler: clang++-15
+            cxxstd: "03,11,14,17,20,2b"
+            os: ubuntu-22.04
+            install:
+              - clang-15
+              - libc++-15-dev
+              - libc++abi-15-dev
+            sources:
+              - "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main"
+            source_keys:
+              - "https://apt.llvm.org/llvm-snapshot.gpg.key"
             cxxflags: -stdlib=libc++
             linkflags: -stdlib=libc++
           - name: UBSAN
             toolset: clang
-            compiler: clang++-12
-            cxxstd: "03,11,14,17,20"
+            compiler: clang++-14
+            cxxstd: "03,11,14,17,20,2b"
             cxxflags: -stdlib=libc++
             linkflags: -stdlib=libc++
             ubsan: 1
             build_variant: debug
-            os: ubuntu-20.04
+            os: ubuntu-22.04
             install:
-              - clang-12
-              - libc++-12-dev
-              - libc++abi-12-dev
+              - clang-14
+              - libc++-14-dev
+              - libc++abi-14-dev
 
           - toolset: clang
             cxxstd: "03,11,14,17,2a"
-            os: macos-10.15
+            os: macos-11
 
     timeout-minutes: 60
     runs-on: ${{matrix.os}}
@@ -253,12 +290,18 @@ jobs:
                 if [ -f "/etc/debian_version" ]
                 then
                     apt-get -o Acquire::Retries=$NET_RETRY_COUNT update
-                    apt-get -o Acquire::Retries=$NET_RETRY_COUNT install -y sudo software-properties-common tzdata wget curl apt-transport-https ca-certificates make build-essential g++ python python3 perl git cmake
+                    if [ "$(apt-cache search "^python-is-python3$" | wc -l)" -ne 0 ]
+                    then
+                        PYTHON_PACKAGE="python-is-python3"
+                    else
+                        PYTHON_PACKAGE="python"
+                    fi
+                    apt-get -o Acquire::Retries=$NET_RETRY_COUNT install -y sudo software-properties-common tzdata wget curl apt-transport-https ca-certificates make build-essential g++ $PYTHON_PACKAGE python3 perl git cmake
                 fi
             fi
             git config --global pack.threads 0
 
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Install packages
         if: matrix.install

include/boost/iterator/function_input_iterator.hpp

@@ -28,6 +28,9 @@ namespace boost {
 
 namespace iterators {
 
+    template <class Function, class Input>
+    class function_input_iterator;
+
     namespace impl {
 
         // Computes the return type of an lvalue-call with an empty argument,
@@ -46,21 +49,21 @@ namespace iterators {
         };
 
         template <class Function, class Input>
-        class function_input_iterator :
+        class function_object_input_iterator :
             public iterator_facade<
-                function_input_iterator<Function, Input>,
+                iterators::function_input_iterator<Function, Input>,
                 typename result_of_nullary_lvalue_call<Function>::type,
                 single_pass_traversal_tag,
                 typename result_of_nullary_lvalue_call<Function>::type const &
             >
         {
         public:
-            function_input_iterator() {}
-            function_input_iterator(Function & f_, Input state_ = Input())
+            function_object_input_iterator() {}
+            function_object_input_iterator(Function & f_, Input state_ = Input())
                 : f(boost::addressof(f_)), state(state_) {}
 
             void increment() {
-                if(value)
+                if (value)
                     value = none;
                 else
                     (*f)();
@@ -69,10 +72,12 @@ namespace iterators {
 
             typename result_of_nullary_lvalue_call<Function>::type const &
                 dereference() const {
-                    return (value ? value : value = (*f)()).get();
+                if (!value)
+                    value = (*f)();
+                return value.get();
             }
 
-            bool equal(function_input_iterator const & other) const {
+            bool equal(function_object_input_iterator const & other) const {
                 return f == other.f && state == other.state;
             }
 
@@ -85,7 +90,7 @@ namespace iterators {
         template <class Function, class Input>
         class function_pointer_input_iterator :
             public iterator_facade<
-                function_pointer_input_iterator<Function, Input>,
+                iterators::function_input_iterator<Function, Input>,
                 typename function_types::result_type<Function>::type,
                 single_pass_traversal_tag,
                 typename function_types::result_type<Function>::type const &
@@ -97,7 +102,7 @@ namespace iterators {
                 : f(f_), state(state_) {}
 
             void increment() {
-                if(value)
+                if (value)
                     value = none;
                 else
                     (*f)();
@@ -106,7 +111,9 @@ namespace iterators {
 
             typename function_types::result_type<Function>::type const &
                 dereference() const {
-                    return (value ? value : value = (*f)()).get();
+                if (!value)
+                    value = (*f)();
+                return value.get();
             }
 
             bool equal(function_pointer_input_iterator const & other) const {
@@ -126,13 +133,13 @@ namespace iterators {
         public boost::conditional<
             function_types::is_function_pointer<Function>::value,
             impl::function_pointer_input_iterator<Function,Input>,
-            impl::function_input_iterator<Function,Input>
+            impl::function_object_input_iterator<Function,Input>
         >::type
     {
         typedef typename boost::conditional<
             function_types::is_function_pointer<Function>::value,
             impl::function_pointer_input_iterator<Function,Input>,
-            impl::function_input_iterator<Function,Input>
+            impl::function_object_input_iterator<Function,Input>
         >::type base_type;
     public:
         function_input_iterator(Function & f, Input i)

include/boost/iterator/iterator_facade.hpp

@@ -154,79 +154,160 @@ namespace iterators {
     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)
+          , 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 that
         // (*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)
-        {}
-
-        // 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.
-        writable_postfix_increment_proxy const&
-        operator*() const
-        {
-            return *this;
-        }
-
         // Provides readability of *r++
-        operator value_type&() const
+        value_type& operator*() 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;
-        }
-
         // Provides X(r++)
         operator Iterator const&() const
         {
             return stored_iterator;
         }
 
+        // Provides (r++)->foo()
+        value_type* operator->() const
+        {
+            return boost::addressof(stored_value);
+        }
+
      private:
-        mutable value_type stored_value;
         Iterator stored_iterator;
+        mutable value_type stored_value;
+    };
+
+
+    template <class Iterator>
+    class writable_postfix_increment_dereference_proxy;
+
+    template <class T>
+    struct is_not_writable_postfix_increment_dereference_proxy :
+        public boost::true_type
+    {};
+
+    template <class Iterator>
+    struct is_not_writable_postfix_increment_dereference_proxy<
+        writable_postfix_increment_dereference_proxy<Iterator>
+    > :
+        public boost::false_type
+    {};
+
+    template <class Iterator>
+    class writable_postfix_increment_proxy;
+
+    //
+    // 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_dereference_proxy
+    {
+        friend class writable_postfix_increment_proxy<Iterator>;
+
+        typedef typename iterator_value<Iterator>::type value_type;
+
+     public:
+        explicit writable_postfix_increment_dereference_proxy(Iterator const& x)
+          : stored_iterator(x)
+          , stored_value(*x)
+        {}
+
+        // Provides readability of *r++
+        operator value_type&() const
+        {
+            return this->stored_value;
+        }
+
+        template <class OtherIterator>
+        writable_postfix_increment_dereference_proxy const&
+        operator=(writable_postfix_increment_dereference_proxy<OtherIterator> const& x) const
+        {
+            typedef typename iterator_value<OtherIterator>::type other_value_type;
+            *this->stored_iterator = static_cast<other_value_type&>(x);
+            return *this;
+        }
+
+        // Provides writability of *r++
+#if !defined(BOOST_NO_CXX11_RVALUE_REFERENCES)
+        template <class T>
+        typename iterators::enable_if<
+            is_not_writable_postfix_increment_dereference_proxy< T >,
+            writable_postfix_increment_dereference_proxy const&
+        >::type operator=(T&& x) const
+        {
+            *this->stored_iterator = static_cast< T&& >(x);
+            return *this;
+        }
+#else
+        template <class T>
+        typename iterators::enable_if<
+            is_not_writable_postfix_increment_dereference_proxy< T >,
+            writable_postfix_increment_dereference_proxy const&
+        >::type operator=(T const& x) const
+        {
+            *this->stored_iterator = x;
+            return *this;
+        }
+
+        // This overload just in case only non-const objects are writable
+        template <class T>
+        typename iterators::enable_if<
+            is_not_writable_postfix_increment_dereference_proxy< T >,
+            writable_postfix_increment_dereference_proxy const&
+        >::type operator=(T& x) const
+        {
+            *this->stored_iterator = x;
+            return *this;
+        }
+#endif
+
+     private:
+        Iterator stored_iterator;
+        mutable value_type stored_value;
+    };
+
+    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)
+          : dereference_proxy(x)
+        {}
+
+        writable_postfix_increment_dereference_proxy<Iterator> const&
+        operator*() const
+        {
+            return dereference_proxy;
+        }
+
+        // Provides X(r++)
+        operator Iterator const&() const
+        {
+            return dereference_proxy.stored_iterator;
+        }
+
+        // Provides (r++)->foo()
+        value_type* operator->() const
+        {
+            return boost::addressof(dereference_proxy.stored_value);
+        }
+
+     private:
+        writable_postfix_increment_dereference_proxy<Iterator> dereference_proxy;
     };
 
 # ifdef BOOST_NO_TEMPLATE_PARTIAL_SPECIALIZATION

test/function_input_iterator_test.cpp

@@ -99,6 +99,15 @@ int main()
     for(std::size_t i = 0; i != 10; ++i)
         BOOST_TEST_EQ(generated[i], static_cast<int>(42 + i));
 
+    // Test that incrementing the iterator returns a reference to the iterator type
+    {
+        typedef boost::iterators::function_input_iterator<counter, int> function_counter_iterator_t;
+        function_counter_iterator_t it1(counter_generator, 0);
+        function_counter_iterator_t it2(++it1);
+        function_counter_iterator_t it3(it2++);
+        BOOST_TEST_EQ(*it3, 54);
+    }
+
 #if !defined(BOOST_NO_CXX11_LAMBDAS) && !defined(BOOST_NO_CXX11_AUTO_DECLARATIONS) \
     && defined(BOOST_RESULT_OF_USE_DECLTYPE)
     // test the iterator with lambda expressions

test/iterator_facade.cpp

@@ -10,7 +10,7 @@
 #include <boost/call_traits.hpp>
 #include <boost/polymorphic_cast.hpp>
 #include <boost/type_traits/is_convertible.hpp>
-#include <boost/utility/enable_if.hpp>
+#include <boost/core/enable_if.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
@@ -63,7 +63,23 @@ struct proxy
 
 struct value
 {
-    void mutator() {} // non-const member function
+    int increment_count;
+    int private_mutator_count;
+    int& shared_mutator_count;
+
+    explicit value(int& shared_mutator_count) :
+        increment_count(0),
+        private_mutator_count(0),
+        shared_mutator_count(shared_mutator_count)
+    {
+    }
+
+    // non-const member function
+    void mutator()
+    {
+        ++private_mutator_count;
+        ++shared_mutator_count;
+    }
 };
 
 struct input_iter
@@ -75,21 +91,25 @@ struct input_iter
     >
 {
  public:
-    input_iter() {}
+    explicit input_iter(value& val) : state(&val) {}
 
     void increment()
     {
+        ++(state->increment_count);
     }
     value
     dereference() const
     {
-        return value();
+        return *state;
     }
 
     bool equal(input_iter const&) const
     {
         return false;
     }
+
+ private:
+    value* state;
 };
 
 template <class T>
@@ -198,13 +218,30 @@ int main()
     {
         // test for a fix to http://tinyurl.com/zuohe
         // These two lines should be equivalent (and both compile)
-        input_iter p;
+        int shared_mutator_count = 0;
+        value val(shared_mutator_count);
+        input_iter p(val);
         (*p).mutator();
         p->mutator();
+        BOOST_TEST_EQ(val.increment_count, 0);
+        BOOST_TEST_EQ(val.private_mutator_count, 0); // mutator() should be invoked on an object returned by value
+        BOOST_TEST_EQ(shared_mutator_count, 2);
 
         same_type<input_iter::pointer>(p.operator->());
     }
 
+    {
+        // Test that accessing dereferenced value of a post-incremented iterator works
+        int shared_mutator_count = 0;
+        value val(shared_mutator_count);
+        input_iter p(val);
+        (*p++).mutator();
+        (p++)->mutator();
+        BOOST_TEST_EQ(val.increment_count, 2);
+        BOOST_TEST_EQ(val.private_mutator_count, 0); // mutator() should be invoked on an object returned by value
+        BOOST_TEST_EQ(shared_mutator_count, 2);
+    }
+
     {
         int x = 0;
         iterator_with_proxy_reference i(x);