Returning by value calls the copy constructor?

Question

template<class T>
T Stack<T>::pop()
{
   if (vused_ == 0)
   {
      throw "Popping empty stack";
   }
   else
   {
      T result = v_[used_ - 1];
      --vused_;
      return result;
   }
}

I didn't understand all of it, or rather I understood none of it, but it was said that this code doesn't work, because it returns by value, I am guessing he was referring to result, and that calls the copy constructor and I have no idea how that's even possible. Can anyone care to explain?

Answer 1

Unlike the code in the question's example, std::stack<T>::pop does not return a value.

That's because if the item type needs to be copied, and the copying throws, then you have an operation failure that has changed the state of the object, with no means of re-establishing the original state.

I.e. the return-a-value-pop does not offer a strong exception guarantee (either succeed or no change).

Similarly, throwing a literal string is unconventional to say the least.

So while the code doesn't have any error in itself (modulo possible typing errors such as vused_ versus v_ etc.), it's weak on guarantees and so unconventional that it may lead to bugs in exception handling elsewhere.

---

A different viewpoint is that the non-value-returning pop of std::stack is impractical, leading to needlessly verbose client code.

And for using a stack object I prefer to have a value-returning pop.

But it's not either/or: a value-returning convenience method popped can be easily defined in terms of pop (state change) and top (inspection). This convenience method then has a weaker exception guarantee. But the client code programmer can choose. :-)

---

An improvement within the existing design would be to support movable objects, that is, replace

    return result;

with

    return move( result );

helping the compiler a little.

↑ Correction:
Actually, the above deleted text has the opposite effect of the intended one, namely, it inhibits RVO (guaranteeing a constructor call). Somehow my thinking got inverted here. But as a rule, don't use move on a return expression that is just the name of a non-parameter automatic variable, because the default is optimization, and the added move can not improve things, but can inhibit an RVO optimization.

Answer 2

Yes, returning by value formally calls the copy constructor. But that's not a problem at all, because in practice, compilers will typically be able to optimize away the additional copy. This technique is called "Return-Value Optimization".

Answer 3

More than the return statement (which can work if the class is movable but not copyable, e.g. you can return std::unique_ptrs), the problem is the copy you do here:

 T result = v_[used_ - 1];

To make this copy possible, the type T must be copyable (e.g. T should have public copy constructor - required by the above statement - and copy assignment operator=).

---

As a side note, throwing a string is really bad: you should throw an exception class, e.g.

throw std::runtime_error("Popping empty stack.");

or just define an ad hoc class for this case and throw it, e.g.:

class StackUnderflowException : public std::runtime_error
{
public:
     StackUnderflowException()
         : std::runtime_error("Popping empty stack.")
     { }
};

....
  throw StackUnderflowException();