[boost] [iterator] [iterator_facade] operator-> question

Subject: [boost] [iterator] [iterator_facade] operator-> question
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2011-06-29 16:09:04

I'm looking at boost::iterator_facade's implementation of operator->, and
I'm not convinced it is correct when using proxy references.

I ran into this problem while using iterator_facade as (sketch only)

struct derived_iterator
    : boost::iterator_facade<
          derived_iterator,
          pair< Index, Value >, // value_type
          CategoryOrTraversal,
          pair< Index, Value& >, // reference
          Difference
      >
{ /*...*/ };

where pair here is similar to std::pair, but also handles reference types.
Thus, the reference type of derived_iterator is a proxy reference.  Indeed,
this might be the basic structure of an iterator to an associative
container, particularly when the keys can be generated "on the fly".

The problem (at least as I see it) is in lines 295 - 329 in
boost/iterator/iterator_facade.hpp (as of Boost version 1.46.1), copied
below for convenience:

----------------
    // 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 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
    {
        operator_arrow_proxy(T const* px) : m_value(*px) {}
        T* operator->() const { return &m_value; }
        // This function is needed for MWCW and BCC, which won't call
operator->
        // again automatically per 13.3.1.2 para 8
        operator T*() const { return &m_value; }
        mutable T m_value;
    };

    // A metafunction that gets the result type for operator->.  Also
    // has a static function make() which builds the result from a
    // Reference
    template <class ValueType, class Reference, class Pointer>
    struct operator_arrow_result
    {
        // CWPro8.3 won't accept "operator_arrow_result::type", and we
        // need that type below, so metafunction forwarding would be a
        // losing proposition here.
        typedef typename mpl::if_<
            is_reference<Reference>
          , Pointer
          , operator_arrow_proxy<ValueType>
        >::type type;

        static type make(Reference x)
        {
            return boost::implicit_cast<type>(&x);
        }
    };
----------------

Now, iterator_facade< /*...*/ >::operator->'s return type is
operator_arrow_result< /*...*/ >::type, which, for proxy references,
according to the logic above in operator_arrow_result, is
operator_arrow_proxy<T>, where T, in this case, is pair< Index, Value >.
The error occurs when operator_arrow_proxy<T> is constructed from &x within
operator_arrow_result< /*...*/ >::make, as it tries to convert a pair<
Index, Value& >* to a pair< Index, Value >*, which is a no-go.

It seems to me that a more correct implementation of operator_arrow_result
would use operator_arrow_proxy< Reference > rather than
operator_arrow_proxy< ValueType >.  I would further rename and change
operator_arrow_proxy's member variable to be stored as const T m_ref, rather
than mutable T m_value.

What are others' opinions on this?  Am I doing something wrong?

Thanks,

- Jeff