implementing a C++ postfix increment operator

Question

I compiled the following example:

#include <iostream>
#include <iterator>
using namespace std;

class myiterator : public iterator<input_iterator_tag, int>
{
  int* p;
public:
  myiterator(int* x) :p(x) {}
  myiterator(const myiterator& mit) : p(mit.p) {}
  myiterator& operator++() {++p;return *this;}
  myiterator& operator++(int) {myiterator tmp(*this); operator++(); return tmp;}
  bool operator==(const myiterator& rhs) {return p==rhs.p;}
  bool operator!=(const myiterator& rhs) {return p!=rhs.p;}
  int& operator*() {return *p;}
};

int main () {
  int numbers[]={10,20,30,40,50};
  myiterator beginning(numbers);
  myiterator end(numbers+5);
  for (myiterator it=beginning; it!=end; it++)
      cout << *it << " ";
  cout << endl;

  return 0;
}

from cplusplus.com/reference and I get the compiler warning:

iterator.cpp: In member function 'myiterator& myiterator::operator++(int)':
iterator.cpp:13: warning: reference to local variable 'tmp' returned

What's wrong here? Is the postfix signature supposed to be myiterator operator++(int) i.e. return by value?

Is there somewhere defined what the postfix signature should look like on STL iterators?

cplusplus.com is useful, but not authoritative. In this case it hurt you. If you look at actual STL code, you'll find that the iterator is often returned by value, which cplusplus.com didn't tell you that you could do. — John Dibling, Dec 03 '10 at 16:39
http://stackoverflow.com/questions/3181211/prefix-postfix-increment-operators — Ciro Santilli OurBigBook.com, Jun 17 '15 at 10:22

score 5 · Accepted Answer · answered Dec 03 '10 at 16:45

5

Is there somewhere defined what the postfix signature should look like on STL iterators?

The Standard.

The Standard dictates such things. In the case of this operation, the standard basically says "you have to return something that is convertible to const X&" where X is the iterator. In practice, this means you can return by reference if that applies to you (it doesn't), or return by value.

See 24.1.3/1

answered Dec 03 '10 at 16:45

John Dibling

99,718
31
186
324

Thanks for reading the question properly and being to only one the actually answer what I was looking for. – chris Dec 05 '10 at 01:01

score 4 · Answer 2 · answered Dec 03 '10 at 16:35

4

You don't want to return the reference: by doing so, you're returning a reference to a variable that, by the time the function has returned, no longer exists. All you need is:

myiterator operator++(int) {myiterator tmp(*this); operator++(); return tmp;}

answered Dec 03 '10 at 16:35

Thanatos

42,585
14
91
146

+1 This is the answer. I'd only add that the error message told you exactly what the problem was, and the logical solution to the given error is in fact the correct answer. Return the thing by value, not by reference. – John Dibling Dec 03 '10 at 16:37
I know. Thats code from cplusplus.com. I'm really looking for the definition of the postfix increment operator for STL iterators... – chris Dec 03 '10 at 16:38
I missed the link my first read through that. Yes - cplusplus.com is wrong here. Except for perhaps some exceedingly odd case, the above will be what a postfix operator looks like. There's no rule saying that it can't return a reference - it just usually doesn't because doing so is inappropriate. – Thanatos Dec 03 '10 at 16:42

Martin York · Answer 3 · 2010-12-03T16:53:43.533

This line:

myiterator& operator++(int) {myiterator tmp(*this); operator++(); return tmp;}

Should be:

myiterator  operator++(int) {myiterator tmp(*this); operator++(); return tmp;}
//      ^^^^ Not return by reference.
//           Don't worry the cost is practically nothing for your class
//           And will probably be optimized into copying the pointer back.

As a side note:

You don't actually need the copy constructor:

myiterator(const myiterator& mit) : p(mit.p) {}

The compiler generated version will work perfectly (as the rule of three/four does not apply as you do not own the RAW pointer contained by your class).

Your comparison operators should probably be marked as const and I personally prefer to define the != operator in terms of the the == operator and let the compiler optimize away any ineffecency (though this is just a personal thing).

bool operator==(const myiterator& rhs) const {return p==rhs.p;}
bool operator!=(const myiterator& rhs) const {return !(*this == rhs);}
                            //        ^^^^^^^ Added const

The operator * should probably have two versions. A normal and a const version.

int&       operator*()       {return *p;}
int const& operator*() const {return *p;}

As a last note: A pointer by itself Is an iterator. SO you don't actually need to wrap pointers to make them iterators they will behave correctly as iterators (and not just input iterators but random access iterators).

score 0 · Answer 4 · answered Dec 03 '10 at 16:35

0

you're returning a reference to a variable that gets destroyed when the method exits. the compiler is warning you of the consequences of this. by the time the reference is received by the caller, the variable it references no longer exists.

answered Dec 03 '10 at 16:35

Simon Courtenage

406
3
9

implementing a C++ postfix increment operator

4 Answers4

Linked