Title
reverse_iterator::operator-> should also support smart pointers
Status
resolved
Section
[reverse.iter.elem]
Submitter
Alisdair Meredith

Created on 2009-03-12.00:00:00 last changed 73 months ago

Messages

Date: 2018-11-14.18:46:39

Proposed resolution:

This wording is relative to N4762.

  1. Modify [reverse.iter.elem] as indicated:

    constexpr pointer operator->() const;
    

    -2- Returns: addressof(operator*()).Effects: If Iterator is a pointer type, as if by:

    Iterator tmp = current;
    return --tmp;
    

    Otherwise, as if by:

    Iterator tmp = current;
    --tmp;
    return tmp.operator->();
    
Date: 2018-11-15.00:00:00

[ 2018-11-13; Casey Carter comments ]

The acceptance of P0896R4 during the San Diego meeting resolves this issue: The wording in [reverse.iter.elem] for operator-> is equivalent to the PR for LWG 1052.

Date: 2018-08-15.00:00:00

[ 2018-08-06; Daniel rebases to current working draft and reduces the proposed wording to that preferred by LEWG ]

Date: 2018-08-06.16:42:21

[ Original rationale: ]

The LWG did not reach a consensus for a change to the WP.

Previous resolution [SUPERSEDED]:

  1. In the class template reverse_iterator synopsis of [reverse.iterator] change as indicated:

    namespace std {
    template <class Iterator>
    class reverse_iterator : public
                 iterator<typename iterator_traits<Iterator>::iterator_category,
                 typename iterator_traits<Iterator>::value_type,
                 typename iterator_traits<Iterator>::difference_type,
                 typename iterator_traits<Iterator&>::pointer,
                 typename iterator_traits<Iterator>::reference> {
    public:
      [..]
      typedef typename iterator_traits<Iterator&>::pointer pointer;
      [..]
    protected:
      Iterator current;
    private:
      mutable Iterator deref_tmp; // exposition only
    };
    
  2. Change [reverse.iter.opref]/1 as indicated:
    pointer operator->() const;
    

    1 Returns Effects: &(operator*()).

    deref_tmp = current;
    --deref_tmp;
    return deref_tmp;
    
Alternate Proposed Resolution from 2775, which was closed as a dup of this issue

This wording is relative to N4606.

  1. Modify [reverse.iter.opref] as indicated:

    constexpr pointer operator->() const;
    

    -1- Returns: addressof(operator*()).Effects: If Iterator is a pointer type, as if by:

    Iterator tmp = current;
    return --tmp;
    

    Otherwise, as if by:

    Iterator tmp = current;
    --tmp;
    return tmp.operator->();
    
Date: 2018-06-22.06:38:21

[ LEWG Kona 2017 ]

Recommend that we accept the "Alternate Proposed Resolution" from 2775.

Date: 2012-10-21.13:19:07

[ 2010 Pittsburgh: ]

Moved to NAD Future, rationale added.

Date: 2010-03-10.00:00:00

[ 2010-03-10 Howard adds: ]

Here are three tests that the current proposed wording passes, and no other solution I've seen passes all three:

  1. Proxy pointer support:

    #include <iterator>
    #include <cassert>
    
    struct X { int m; };
    
    X x;
    
    struct IterX {
        typedef std::bidirectional_iterator_tag iterator_category;
        typedef X& reference;
        struct pointer
        {
            pointer(X& v) : value(v) {}
            X& value;
            X* operator->() const {return &value;}
        };
        typedef std::ptrdiff_t difference_type;
        typedef X value_type;
        // additional iterator requirements not important for this issue
    
        reference operator*() const { return x; }
        pointer operator->() const { return pointer(x); }
        IterX& operator--() {return *this;}
    
    };
    
    int main()
    {
        std::reverse_iterator<IterX> ix;
        assert(&ix->m == &(*ix).m);
    }
    
  2. Raw pointer support:

    #include <iterator>
    #include <utility>
    
    int main() {
      typedef std::pair<int, double> P;
      P op;
      std::reverse_iterator<P*> ri(&op + 1);
      ri->first; // Error
    }
    
  3. Caching iterator support:

    #include <iterator>
    #include <cassert>
    
    struct X { int m; };
    
    struct IterX {
        typedef std::bidirectional_iterator_tag iterator_category;
        typedef X& reference;
        typedef X* pointer;
        typedef std::ptrdiff_t difference_type;
        typedef X value_type;
        // additional iterator requirements not important for this issue
    
        reference operator*() const { return value; }
        pointer operator->() const { return &value; }
        IterX& operator--() {return *this;}
    
    private:
        mutable X value;
    };
    
    int main()
    {
        std::reverse_iterator<IterX> ix;
        assert(&ix->m == &(*ix).m);
    }
    
Date: 2012-10-21.13:19:07

[ 2010 Pittsburgh: ]

We prefer to make to use a local variable instead of deref_tmp within operator->(). And although this means that the mutable change is no longer needed, we prefer to keep it because it is needed for operator*() anyway.

Here is the proposed wording that was replaced:

Change [reverse.iter.opref]:

pointer operator->() const;

Returns:

&(operator*());
deref_tmp = current;
--deref_tmp;
return deref_tmp::operator->();
Date: 2010-03-03.00:00:00

[ 2010-03-03 Daniel opens: ]

  1. There is a minor problem with the exposition-only declaration of the private member deref_tmp which is modified in a const member function (and the same problem occurs in the specification of operator*). The fix is to make it a mutable member.
  2. The more severe problem is that the resolution for some reasons does not explain in the rationale why it was decided to differ from the suggested fix (using deref_tmp instead of tmp) in the [ 2009-10 Santa Cruz] comment:

    this->deref_tmp = current;
    --this->deref_tmp;
    return this->deref_tmp;
    

    combined with the change of

    typedef typename iterator_traits<Iterator>::pointer pointer;
    

    to

    typedef Iterator pointer;
    

    The problem of the agreed on wording is that the following rather typical example, that compiled with the wording before 1052 had been applied, won't compile anymore:

    #include <iterator>
    #include <utility>
    
    int main() {
      typedef std::pair<int, double> P;
      P op;
      std::reverse_iterator<P*> ri(&op + 1);
      ri->first; // Error
    }
    

    Comeau online returns (if a correspondingly changed reverse_iterator is used):

    "error: expression must have class type
         return deref_tmp.operator->();
                ^
             detected during instantiation of "Iterator
                       reverse_iterator<Iterator>::operator->() const [with
                       Iterator=std::pair<int, double> *]""
    

    Thus the change will break valid, existing code based on std::reverse_iterator.

IMO the suggestion proposed in the comment is a necessary fix, which harmonizes with the similar specification of std::move_iterator and properly reflects the recursive nature of the evaluation of operator-> overloads.

Suggested resolution:

  1. In the class template reverse_iterator synopsis of [reverse.iterator] change as indicated:

    namespace std {
    template <class Iterator>
    class reverse_iterator : public
                 iterator<typename iterator_traits<Iterator>::iterator_category,
                 typename iterator_traits<Iterator>::value_type,
                 typename iterator_traits<Iterator>::difference_type,
                 typename iterator_traits<Iterator>::pointer,
                 typename iterator_traits<Iterator>::reference> {
    public:
      [..]
      typedef typename iterator_traits<Iterator>::pointer pointer;
      [..]
    protected:
      Iterator current;
    private:
      mutable Iterator deref_tmp; // exposition only
    };
    
  2. Change [reverse.iter.opref]/1 as indicated:
    pointer operator->() const;
    

    1 Returns Effects: &(operator*()).

    deref_tmp = current;
    --deref_tmp;
    return deref_tmp;
    
Date: 2012-10-21.13:19:07

[ 2009-10 Santa Cruz: ]

We can't think of any reason we can't just define reverse iterator's pointer types to be the same as the underlying iterator's pointer type, and get it by calling the right arrow directly.

Here is the proposed wording that was replaced:

template <class Iterator>
class reverse_iterator {
  […]
  typedef typename iterator_traits<Iterator>::pointer pointer;

Change [reverse.iter.opref]:

pointer operator->() const;

Returns:

&(operator*());
this->tmp = current;
--this->tmp;
return this->tmp;
Date: 2009-08-01.00:00:00

[ 2009-08-01 Howard deconceptized: ]

Date: 2012-10-21.13:19:07

[ 2009-07 post-Frankfurt: ]

Howard to deconceptize. Move to Review after that happens.

Date: 2012-10-21.13:19:07

[ Summit: ]

move_iterator avoids this problem by returning a value of the wrapped Iterator type. study group formed to come up with a suggested resolution.

move_iterator solution shown in proposed wording.

Date: 2009-03-12.00:00:00

Addresses UK 281 [CD1]

The current specification for return value for reverse_iterator::operator-> will always be a true pointer type, but reverse_iterator supports proxy iterators where the pointer type may be some kind of 'smart pointer'.

History
Date User Action Args
2018-11-25 18:36:54adminsetstatus: open -> resolved
2018-11-14 18:46:39adminsetmessages: + msg10224
2018-06-22 06:38:21adminsetmessages: + msg9956
2018-06-22 06:38:21adminsetstatus: lewg -> open
2016-10-08 04:58:13adminsetmessages: + msg8543
2014-11-24 15:11:58adminsetstatus: nad future -> lewg
2012-10-21 13:19:07adminsetmessages: + msg6212
2012-10-21 13:19:07adminsetmessages: + msg6211
2012-10-21 13:19:07adminsetmessages: + msg6210
2012-10-21 13:19:07adminsetmessages: + msg6209
2012-10-21 13:19:07adminsetmessages: + msg6208
2012-10-21 13:19:07adminsetmessages: + msg6207
2012-10-21 13:19:07adminsetmessages: + msg6206
2012-10-21 13:19:07adminsetmessages: + msg6205
2012-10-21 13:19:07adminsetmessages: + msg6204
2012-10-21 13:19:07adminsetmessages: + msg6203
2009-03-12 00:00:00admincreate