0
MyObject& MyObject::operator++(int)
{

MyObject e;
e.setVector(this->vector);

...

return &e;
}

invalid initialization of non-const reference of type 'MyObject&' from an rvalue of type 'MyObject*'
     return &e;
             ^

I am not sure what it's saying. Is it saying that e is a pointer, because it's not a pointer. Also, if you'd make a pointer to the address of e, it would get wiped out at the end of the bracket and the pointer would be lost.

user2967016
  • 133
  • 1
  • 9
  • 1
    `e` is not a pointer, but `&e` is a pointer. – Brian Bi Mar 05 '14 at 03:27
  • I did something like this MyObject * pointer = &e; return *pointer, but the pointer disappears at some point. – user2967016 Mar 05 '14 at 03:33
  • 1
    Yes, you should not return a reference to a local variable. Return a copy instead. – Brian Bi Mar 05 '14 at 03:35
  • So, I should do away with the & since I can't call the constructor with this, right? That's what I wanted to do, but I wasn't sure it would be a good idea since most people told me to return the address. I don't see what difference it would make though. – user2967016 Mar 05 '14 at 03:43
  • Copying an object makes a new object. The original one, the local variable, is destroyed. – Brian Bi Mar 05 '14 at 03:45

3 Answers3

1

You're correct that e is not a pointer, but &e very much is a pointer.

I'm reasonably certain that returning a reference to a stack variable that will be out of scope before you can use it is also not such a good idea.

The general way to implement postfix operator++ is to save the current value to return it, and modify *this with the prefix variant, such as:

Type& Type::operator++ ()   {   // ++x
    this->addOne();             // whatever you need to do to increment
    return *this;
}
Type Type::operator++ (int) {   // x++
    Type retval (*this);
    ++(*this);
    return retval;
}

Especially note the fact that the prefix variant returns a reference to the current object (after incrementing) while the postfix variant returns a non-reference copy of the original object (before incrementing).

That's covered in the C++ standard. In C++11 13.6 Built-in operators /3:

For every pair (T, VQ), where T is an arithmetic type, and VQ is either volatile or empty, there exist candidate operator functions of the form:

VQ T & operator++(VQ T &);

T operator++(VQ T &, int);

If, for some reason, you can't use the constructor to copy the object, you can still do it the way you have it (creating a local e and setting its vector) - you just have to ensure you return e (technically a copy of e) rather than &e.

Community
  • 1
  • 1
paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
  • I can't use the constructor though, because it has a vector inside it and it takes a string as an argument. – user2967016 Mar 05 '14 at 03:38
  • @user2967016: are you saying you don't have a constructor that takes another object of the same type? That strikes me as ... less than ideal ... design (though you may of course have good reasons for it, feel free to elucidate). – paxdiablo Mar 05 '14 at 03:41
1

Your return type is MyObject&, a reference to a (non-temporary) MyObject object. However, your return expression has a type of MyObject*, because you are getting the address of e.

return &e;
       ^

Still, your operator++, which is a postfix increment operator due to the dummy int argument, is poorly defined. In accordance to https://stackoverflow.com/a/4421719/1619294, it should be defined more or less as

MyObject MyObject::operator++(int)
{
   MyObject e;
   e.setVector(this->vector);

   ...

   return e;
}

without the reference in the return type.

Community
  • 1
  • 1
Mark Garcia
  • 17,424
  • 4
  • 58
  • 94
0

Change return &e; to return e;. In the same way that a function like

void Func(int &a);

isn't called with Func(&some_int) you don't need the & in the return statement. &e takes the location of e and is of type MyObject*.

Also note, MyObject& is a reference to the object, not a copy. You are returning a reference to e, which will be destroyed when the function finishes and as such will be invalid when you next make use of it.

Cramer
  • 1,785
  • 1
  • 12
  • 20