Overloading on R-value references and code duplication

Question

Consider the following:

struct vec
{
    int v[3];

    vec() : v() {};
    vec(int x, int y, int z) : v{x,y,z} {};
    vec(const vec& that) = default;
    vec& operator=(const vec& that) = default;
    ~vec() = default;

    vec& operator+=(const vec& that)
    {
        v[0] += that.v[0];
        v[1] += that.v[1];
        v[2] += that.v[2];
        return *this;
    }
};

vec operator+(const vec& lhs, const vec& rhs)
{
    return vec(lhs.v[0] + rhs.v[0], lhs.v[1] + rhs.v[1], lhs.v[2] + rhs.v[2]);
}
vec&& operator+(vec&& lhs, const vec& rhs)
{
    return move(lhs += rhs);
}
vec&& operator+(const vec& lhs, vec&& rhs)
{
    return move(rhs += lhs);
}
vec&& operator+(vec&& lhs, vec&& rhs)
{
    return move(lhs += rhs);
}

Thanks to r-value references, with these four overloads of operator+ I can minimize the number of objects created, by reusing temporaries. But I don't like the duplication of code this introduces. Can I achieve the same with less repetition?

Answer 1

Recycling temporaries is an interesting idea and you're not the only one who wrote functions that return rvalue references for this reason. In an older C++0x draft operator+(string&&,string const&) was also declared to return an rvalue reference. But this changed for good reasons. I see three issues with this kind of overloading and choice of return types. Two of them are independent of the actual type and the third argument refers to the kind of type that vec is.

1. Safety issues. Consider code like this:

   vec a = ....;
   vec b = ....;
   vec c = ....;
   auto&& x = a+b+c;
   

   If your last operator returns an rvalue reference, x will be a dangling reference. Otherwise, it won't. This is not an artificial example. For example, the auto&& trick is used in the for-range loop internally to avoid unnecessary copies. But since the life-time extension rule for temporaries during reference binding does not apply in case of a function call that simply returns a reference, you'll get a dangling reference.

   string source1();
   string source2();
   string source3();
   
   ....
   
   int main() {
     for ( char x : source1()+source2()+source3() ) {}
   }
   

   If the last operator+ returned an rvalue reference to the temporary that is created during the first concatenation, this code would invoke undefined behaviour because the string temporary would not exist long enough.

2. In generic code, functions that return rvalue references force you to write

   typename std::decay<decltype(a+b+c)>::type
   

   instead of

   decltype(a+b+c)
   

   simply because the last op+ might return an rvalue reference. This is getting ugly, in my humble opinion.

3. Since your type vec is both "flat" and small, these op+ overloads are hardly useful. See FredOverflow's answer.

Conclusion: Functions with an rvalue reference return type should be avoided especially if these references may refer to short-lived temporary objects. std::move and std::forward are special-purpose exceptions to this rule of thumb.

Answer 2

Since your vec type is "flat" (there is no external data), moving and copying do exactly the same thing. So all your rvalue references and std::moves gain you absoutely nothing in performance.

I would get rid of all additional overloads and just write the classic reference-to-const version:

vec operator+(const vec& lhs, const vec& rhs)
{
    return vec(lhs.v[0] + rhs.v[0], lhs.v[1] + rhs.v[1], lhs.v[2] + rhs.v[2]);
}

In case you have little understanding of move semantics yet, I recommend studying this question.

Thanks to r-value references, with these four overloads of operator+ I can minimize the number of objects created, by reusing temporaries.

With a few exceptions, returning rvalue references is a very bad idea, because calls of such functions are xvalues instead of prvalues, and you can get nasty temporary object lifetime problems. Don't do it.

Answer 3

This, which already works wonderfully in current C++, will use move semantics (if available) in C++0x. It already handles all cases, but relies on copy elision and inlining to avoid copies – so it may make more copies than desired, particularly for the second parameter. The nice bit about this is it works without any other overloads and does the right thing (semantically):

vec operator+(vec a, vec const &b) {
  a += b;
  return a;  // "a" is local, so this is implicitly "return std::move(a)",
             // if move semantics are available for the type.
}

And this is where you would stop, 99% of the time. (I am likely underestimating that figure.) The rest of this answer only applies once you know, such as through the use of a profiler, that extra copies from op+ are worth further optimization.

---

To completely avoid all possible copies/moves, you would indeed need these overloads:

// lvalue + lvalue
vec operator+(vec const &a, vec const &b) {
  vec x (a);
  x += b;
  return x;
}

// rvalue + lvalue
vec&& operator+(vec &&a, vec const &b) {
  a += b;
  return std::move(a);
}

// lvalue + rvalue
vec&& operator+(vec const &a, vec &&b) {
  b += a;
  return std::move(b);
}

// rvalue + rvalue, needed to disambiguate above two
vec&& operator+(vec &&a, vec &&b) {
  a += b;
  return std::move(a);
}

You were on the right track with yours, with no real reduction possible (AFAICT), though if you need this op+ often for many types, a macro or CRTP could generate it for you. The only real difference (my preference for separate statements above is minor) is yours make copies when you add two lvalues in operator+(const vec& lhs, vec&& rhs):

return std::move(rhs + lhs);

Reducing duplication through CRTP

template<class T>
struct Addable {
  friend T operator+(T const &a, T const &b) {
    T x (a);
    x += b;
    return x;
  }

  friend T&& operator+(T &&a, T const &b) {
    a += b;
    return std::move(a);
  }

  friend T&& operator+(T const &a, T &&b) {
    b += a;
    return std::move(b);
  }

  friend T&& operator+(T &&a, T &&b) {
    a += b;
    return std::move(a);
  }
};

struct vec : Addable<vec> {
  //...
  vec& operator+=(vec const &x);
};

Now there's no longer a need to define any op+ specifically for vec. Addable is reusable for any type with op+=.

Answer 4

I coded up Fred Nurk's answer using clang + libc++. I had to remove the use of initializer syntax because clang doesn't yet implement that. I also put a print statement in the copy constructor so that we could count copies.

#include <iostream>

template<class T>
struct AddPlus {
  friend T operator+(T a, T const &b) {
    a += b;
    return a;
  }

  friend T&& operator+(T &&a, T const &b) {
    a += b;
    return std::move(a);
  }

  friend T&& operator+(T const &a, T &&b) {
    b += a;
    return std::move(b);
  }

  friend T&& operator+(T &&a, T &&b) {
    a += b;
    return std::move(a);
  }

};

struct vec
    : public AddPlus<vec>
{
    int v[3];

    vec() : v() {};
    vec(int x, int y, int z)
    {
        v[0] = x;
        v[1] = y;
        v[2] = z;
    };
    vec(const vec& that)
    {
        std::cout << "Copying\n";
        v[0] = that.v[0];
        v[1] = that.v[1];
        v[2] = that.v[2];
    }
    vec& operator=(const vec& that) = default;
    ~vec() = default;

    vec& operator+=(const vec& that)
    {
        v[0] += that.v[0];
        v[1] += that.v[1];
        v[2] += that.v[2];
        return *this;
    }
};

int main()
{
    vec v1(1, 2, 3), v2(1, 2, 3), v3(1, 2, 3), v4(1, 2, 3);
    vec v5 = v1 + v2 + v3 + v4;
}

test.cpp:66:22: error: use of overloaded operator '+' is ambiguous (with operand types 'vec' and 'vec')
    vec v5 = v1 + v2 + v3 + v4;
             ~~~~~~~ ^ ~~
test.cpp:5:12: note: candidate function
  friend T operator+(T a, T const &b) {
           ^
test.cpp:10:14: note: candidate function
  friend T&& operator+(T &&a, T const &b) {
             ^
1 error generated.

I fixed this error like so:

template<class T>
struct AddPlus {
  friend T operator+(const T& a, T const &b) {
    T x(a);
    x += b;
    return x;
  }

  friend T&& operator+(T &&a, T const &b) {
    a += b;
    return std::move(a);
  }

  friend T&& operator+(T const &a, T &&b) {
    b += a;
    return std::move(b);
  }

  friend T&& operator+(T &&a, T &&b) {
    a += b;
    return std::move(a);
  }

};

Running the example outputs:

Copying
Copying

Next I tried a C++03 approach:

#include <iostream>

struct vec
{
    int v[3];

    vec() : v() {};
    vec(int x, int y, int z)
    {
        v[0] = x;
        v[1] = y;
        v[2] = z;
    };
    vec(const vec& that)
    {
        std::cout << "Copying\n";
        v[0] = that.v[0];
        v[1] = that.v[1];
        v[2] = that.v[2];
    }
    vec& operator=(const vec& that) = default;
    ~vec() = default;

    vec& operator+=(const vec& that)
    {
        v[0] += that.v[0];
        v[1] += that.v[1];
        v[2] += that.v[2];
        return *this;
    }
};

vec operator+(const vec& lhs, const vec& rhs)
{
    return vec(lhs.v[0] + rhs.v[0], lhs.v[1] + rhs.v[1], lhs.v[2] + rhs.v[2]);
}

int main()
{
    vec v1(1, 2, 3), v2(1, 2, 3), v3(1, 2, 3), v4(1, 2, 3);
    vec v5 = v1 + v2 + v3 + v4;
}

Running this program produced no output at all.

These are the results I got with clang++. Interpret them how you may. And your milage may vary.