Automatically implement operator+ using CRTP

Question

I have multiple classes that should support addition. For each of them, the operator += is implemented as T& operator+=(const T& rhs). I also want to add the operator +, but this implementation will use += and will therefore be similiar in all classes. My idea was to implement a template class using the Curiously Recurring Template Pattern that provides an implementation. The result is code like this:

template<class T>
class Sumable {
private:
 Sumable() = default;
public:
 Sumable(const Sumable<T>&) = delete;
 Sumable(Sumable<T>&&) = delete;
 T operator+(const T&) const&;
 T operator+(const T&) && ;
 T operator+(T&&) const&;
 T operator+(T&&) && ;
 friend T;
};
template<class T>
inline T Sumable<T>::operator+(const T & rhs) const&
{
 T result(*static_cast<const T*>(this));
 return std::move(result += rhs);
}
template<class T>
inline T Sumable<T>::operator+(const T & rhs) &&
{
 return std::move<T&>(*static_cast<T*>(this) += rhs);
}
template<class T>
inline T Sumable<T>::operator+(T && rhs) const&
{
 return std::move(rhs += *this);
}
template<class T>
inline T Sumable<T>::operator+(T && rhs) &&
{
 return std::move(rhs += *this);
}

Classes that support addition would extend from Sumable<T> like for example class Vector : public Sumable<Vector>

The operator is overloaded four times to prevent the creation of unnecessary temporary objects. The idea is described here https://stackoverflow.com/questions/6006527/overloading-on-r-value-references-and-code-duplication, but I return T by value to prevent the problems described in this question. I also follow this idea https://stackoverflow.com/questions/11224838/prevent-user-from-deriving-from-incorrect-crtp-base/11241079 to prevent incorrect use of the class.

In a simple test, this worked like expected. Are there any edge cases in which this design can cause problems? And is it a good idea to use this design in production code?

Edit The code I used to test this, compiles under MSVS 2017:

#include "stdafx.h"
#include <iostream>
#include "Sumable.h"
using namespace std;
struct Test : public Sumable<Test>{
 int data;
 Test(int data) : data(data) {};
 Test(const Test& that) : data(that.data) 
 {
 cout << "copy" << endl;
 };
 Test(Test&& that) : data(that.data) 
 {
 cout << "move" << endl;
 };
 Test& operator+=(const Test& that) {
 this->data += that.data;
 return *this;
 }
};
int main()
{
 Test a{ 1 };
 Test b{ 2 };
 Test c{ 3 };
 cout << (a + b + c + a + b + c).data << endl;
 return 0;
}

• Emily L.: What you're trying to create is kind of common and has a name: Barton-Nackman Trick. I think the article might provide some insight and a Google with the name might give some code to look at. Best wishes.

Answer 1

Missing header

I had to #include <utility> to get a definition for std::move().

Don't delete your destructors

You didn't show us your tests, but I struggled to get my obvious code to compile. Part of the reason was the deleted constructors; I changed the default constructor to protected and made the copy/move constructors to protected and = default, and then I was able to inherit without having to explicitly write copy/move construct/assign in my subclass:

struct Test : public Sumable<Test>
{
 int i;
 Test(int i) : i(i) {}
 Test& operator+=(const Test& t) { i += t.i; return *this; }
};

There's no need to move() when returning

I know the question is tagged C++11, but even there, any benefit is questionable (and particularly unlikely to exist when the function is inlined). When you move the code to a more recent standard, it's actively harmful.

Pass by value instead of copying from a reference

If you can insist on the usual x + y == y + x (which seems to be assumed already, given the third and fourth methods, which compute rhs += *this), then you can pass rhs by value (since we need to copy one of the arguments), which will be move-constructed as and when appropriate:

template<class T>
inline T Sumable<T>::operator+(T rhs) const
{
 return rhs += *static_cast<const T*>(this);
}

That's now one method instead of four.

N.B. I had to introduce the static_cast there, as your code wouldn't compile with GCC 8.0 without it.

Consider free functions

Inheritance isn't necessarily the best solution to everything in C++. You may be better writing free functions instead (in the same namespace as your classes, or in a specific includable namespace, along the lines of std::rel_ops).

Here's a start, including a basic test:

#include <type_traits>
namespace arithmetic_ops
{
 template<typename T, typename U>
 auto operator+(T t, U&& u)
 -> typename std::remove_reference<decltype(t+=u)>::type
 {
 return t += u;
 }
 template<typename T, typename U>
 auto operator+(T&& t, U u)
 -> typename std::enable_if<!std::is_assignable<T,U>::value,
 decltype(operator+(u,t))>::type
 {
 return u + t;
 }
}

namespace test {
 struct Test
 {
 int i;
 Test(int i) : i(i) {}
 Test& operator+=(const Test& t) { i += t.i; return *this; }
 };
}
int main()
{
 using namespace arithmetic_ops;
 test::Test a(4);
 test::Test b(-4);
 return (0 + a + b + 0).i;
}

I've used forwarding references above for the non-copied argument - that allows us to have only one method for the class-first case (the second version being an override for cases such as 0 + a where the first argument must be converted).

The enable_if avoids ambiguity when t + u and u + t are both possible.

The ::type and ::value members get shortened from C++14, with _t and _v suffixes to the names (e.g. std::remove_reference_t<decltype(t+=u)> and std::enable_if_t<!std::is_assignable_v<T,U>, decltype(operator+(u,t))>.

a and/or b can be made const in this test without breaking it.

• Feanor: There's no need to move() when returning: When I tested this code and logged calls to copy-/move-constructor, removing the std::move resulted in the creation of a temporary copy.
• Toby Speight: That might be a Quality of Implementation issue with your compiler and optimisation level. If you find you need it on some platforms, I recommend you at least wrap it in a macro so that it's not inflicted on better systems. (i.e. #define RETURN_MAYBE_MOVE(x) return (x), with #define RETURN_MAYBE_MOVE(x) return std::move(x) where you must.