9

I have the numeric vector template class below (vector for numerical calculations). I am trying make it possible to write D=A+B+C where all variables are Vector objects. A, B and C should not be modified. My idea is to use Vector operator+(Vector&& B) so that after (hopefully) an Rvalue Vector has been returned from B+C, all subsequent additions are stored in that object i.e. steal the storage of the Rvalue for all subsequent additions. This is in order to eliminate creation of new objects and required storage.

My problem is that I can see from output statements from each function called that Vector operator+(Vector&& B) is never called. I cannot understand why since if I have an overloaded dummy function foo(Vector& B) and foo(Vector&& B) and try foo(A+B+C), then the second function is called exactly as I hoped.

Sorry for the long winded question but this is my first question here and I want to try to be as clear as possible.

Any suggestions as to what I am obviously doing wrong or why I should not be trying this, would be appreciated.

template <typename T>
class Vector
{
        int n;
        T* v;
        Vector();
        ~Vector();
        Vector(const Vector& B);
        Vector(Vector&& B);
        inline Vector operator+(const Vector& B) const;
        inline Vector operator+(Vector&& B) const;
};

template <typename T>
Vector<T>::Vector(const Vector<T>& B)
{
        ...
}

template <typename T>
Vector<T>::Vector(Vector<T>&& B)
{
        ...
}

template <typename T>
Vector<T> Vector<T>::operator+(const Vector<T>& B) const
{
        Vector<T> C;
        ...
        return C;
}

template <typename T>
Vector<T> Vector<T>::operator+(Vector<T>&& B) const
{
        ...do stuff to B
        return B;
}
Lars Chr
  • 947
  • 1
  • 9
  • 12
  • Sorry, got a bit sloppy there. If I added all the implementations the code be huge so I hoped the snippets above would be enough. The functions go through and add element by element two vectors. – Lars Chr Jul 30 '12 at 17:14

2 Answers2

7

In the expression:

D=A+B+C

A and B are lvalues, so the call A+B calls Vector::operator(const Vector&)

That returns an rvalue, let's call it tmp, so the next sub-expression is tmp+C.

C is also an lvalue, so it calls Vector::operator(const Vector&) again. That returns another rvalue, lets call it tmp2

The final sub-expression is D=tmp2, but your type doesn't have a move-assignment operator, so the implicitly-defined copy-assignment operator is used.

i.e. you never invoke operator+ with an rvalue on the right-hand side, and the only expression which does have an rvalue argument is an assignment which you haven't defined for rvalues.

It would be better to define overloaded non-member operators:

Vector operator+(const Vector&, const Vector&);
Vector operator+(Vector&&, const Vector&);
Vector operator+(const Vector&, Vector&&);
Vector operator+(Vector&&, Vector&&);

This will work for any combination of rvalues and lvalues. (In general operator+ should usually be a non-member anyway.)

Edit: the alternative suggestion below doesn't work, it results in ambiguities in some cases.

Another alternative, if your compiler supports it (I think only clang does,) would be to keep your existing Vector::operator+(Vector&&) but replace your Vector::operator+(const Vector&) with two overloads distinguished by a ref-qualifier:

Vector Vector::operator+(const Vector& v) const&
{
  Vector tmp(*this);
  tmp += v;
  return tmp;
}

Vector Vector::operator+(const Vector& v)&&
{
  *this += v;
  return std::move(*this);
}

This reuses *this when it's known to be an rvalue, i.e. it uses move semantics when the left-hand side of the addition is an rvalue, compared to your original code which can only use move semantics when the right-hand side is an rvalue. (N.B. the code above assumes you've defined a member operator+= as suggested in David Rodriguez's answer)

Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
  • Ahhh... So a `Vector operator+(Vector&& A, const Vector& B)` would give me the desired behaviour? – Lars Chr Jul 30 '12 at 17:20
  • yes, because that allows you to use move semantics when the left-hand side of the addition is an rvalue – Jonathan Wakely Jul 30 '12 at 17:22
  • 2
    @larsc: You don't need (nor want) the rvalue-reference on the first argument of the free function version. You should take the first argument *by value*, and let the compiler deal with it. The problem with the *rvalue-reference* in the signature, is that (and because it is a template) it will also bind to an *lvalue* and then you will probably do the wrong thing internally (unless, of course, you provide two overloads). – David Rodríguez - dribeas Jul 30 '12 at 17:23
  • 2
    I disagree that the rvalue-reference is not wanted, my solution shows four overloads that will handle any combination of lvalues and rvalues and use avoid copies when possible. A single function taking an argument by value can't do that. – Jonathan Wakely Jul 30 '12 at 17:40
  • @JonathanWakely: I meant that you don't want *only* that overload, as it will be used for both *rvalue* and *lvalues* (I believe would have to double check, but type deduction in templates is a bit trickier...) also in the member function options, the one taking a reference should take a *const* reference and suffers from the same problem that my solution has: it will not be optimal in the case of `a + (b+c)` (*rvalue* as second argument) – David Rodríguez - dribeas Jul 30 '12 at 17:45
  • @DavidRodríguez-dribeas, ah gotcha. I meant the two members to be in addition to the existing one taking an rvalue reference, I'll edit to clarify that, and add the const, thanks – Jonathan Wakely Jul 30 '12 at 18:03
2

I would suggest that you provide operator+= as a member method and then reuse that from a free function operator+ defined as:

template <typename T>
Vector<T> operator+( Vector<T> lhs,              // by value
                     Vector<T> const & rhs ) {
    lhs += rhs;
    return lhs;
}

Given a call a + b + c, which groups as (a+b) + c, the compiler will create a copy of a for the first call to operator+, modify that in place (lhs += rhs) and then move it into the returned value. It will then move that returned value (unless it elides the move) into the argument of the second operator+, where it will be modified in place again, and then moved to the return value.

Overall, a single new object will be created, holding the result of a+b+c, providing semantics equivalent to:

Vector<T> tmp = a;
tmp += b;
tmp += c;

But with the nicer, more compact syntax a + b + c.


Note that this will not handle a + (b+c) graciously, and will create two objects, if you want to support that, you will need to produce multiple overloads.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • Your final sentence alludes to it, but doesn't make it entirely clear that your single non-member `operator+` is sub-optimal for all `lvalue + rvalue` cases, such as your `a + (b+c)` and also any `lvalue + func_returning_rvalue()` – Jonathan Wakely Jul 30 '12 at 17:38
  • @JonathanWakely: I was actually thinking what is the minimum subset of overloads that need to be provided for an optimal solution... I will think about that tonight. – David Rodríguez - dribeas Jul 30 '12 at 17:40
  • There are four cases: l+l, r+l, l+r, r+r, and only the first should need to allocate memory. I think you can reduce it to three functions if you use member functions with ref-qualifiers: `V::operator+(const V&) const&` deals with the l+l case and allocates, `V::operator+(const V&)&&` handles r+l and reuses `*this`, and `V::operator+(V rv)` handles `l+r` and `r+r` and reuses `rv`. I'll need to test that with clang to see if it's ambiguous though. – Jonathan Wakely Jul 30 '12 at 18:10
  • 1
    Bah, both r+l and r+r are ambiguous, so I think you need all four overloads, which can (and should) be non-members – Jonathan Wakely Jul 30 '12 at 18:17