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?

Clino

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. – Drupe

#3181711 – Euphonious

Answer 1

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

Drupe

Thanks for reading the question properly and being to only one the actually answer what I was looking for. – Clino

Answer 2

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;}

Bradlybradman

+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. – Drupe

I know. Thats code from cplusplus.com. I'm really looking for the definition of the postfix increment operator for STL iterators... – Clino

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. – Bradlybradman

Answer 3

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).

Scarito

Answer 4

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.

Bohon