Sreevatsank Kadaveru
Sreevatsank Kadaveru

Reputation: 21

How to do Operator overloading with move semantics in c++? (Elegantly)

class T {
    size_t *pData;          // Memory allocated in the constructor
    friend T operator+(const T& a, const T& b);
};
T operator+(const T& a, const T& b){        // Op 1
        T c;                            // malloc()
        *c.pData = *a.pData + *b.pData;
        return c;
}

T do_something(){
    /* Implementation details */
    return T_Obj;
}

A simple class T with dynamic memory. Consider

T a,b,c;
c = a + b;                                      // Case 1
c = a + do_something(b);            // Case 2
c = do_something(a) + b;            // Case 3
c = do_something(a) + do_something(b);           // Case 4

We can do better by addiitonally defining,

T& operator+(const T& a, T&& b){           // Op 2
                    // no malloc() steeling data from b rvalue
        *b.pData = *a.pData + *b.pData;
        return b;
}

Case 2 now only uses 1 malloc(), but what about Case 3? do we need to define Op 3?

T& operator+(T&& a, const T& b){            // Op 3
                    // no malloc() steeling data from a rvalue
        *b.pData = *a.pData + *b.pData;
        return b;
}

Further, if we do define Op 2 and Op 3, given the fact that an rvalue reference can bind to an lvalue reference, the compiler now has two equally plausible function definitions to call in Case 4

T& operator+(const T& a, T&& b);        // Op 2 rvalue binding to a
T& operator+(T&& a, const T& b);        // Op 3 rvalue binding to b

the compiler would complain about an ambiguous function call, would defining Op 4 help work around the compiler's ambiguous function call problem? as we gain no additional performance with Op 4

T& operator+(T&& a, T&& b){          // Op 4
                    // no malloc() can steel data from a or b rvalue
        *b.pData = *a.pData + *b.pData;
        return b;
}

With Op 1, Op 2, Op 3 and Op 4, we have

If all my understanding is correct, we will need four function signatures per operator. This somehow doesn't seem right, as it is quite a lot of boilerplate and code duplication per operator. Am I missing something? Is there an elegant way of achieving the same?

Upvotes: 0

Views: 362

Answers (3)

Sreevatsank Kadaveru
Sreevatsank Kadaveru

Reputation: 21

This is performant and elegant but makes use of a macro.


#include <type_traits>
#include <iostream>

#define OPERATOR_Fn(Op)         \
template<typename T1, typename T2>          \
friend auto operator Op (T1&& a, T2&& b)          \
           -> typename std::enable_if<std::is_same<std::decay_t<T1>,std::decay_t<T2>>::value,std::decay_t<T1>>::type \
{                                                           \
    constexpr bool a_or_b = !std::is_reference<T1>::value;            \
    std::decay_t<T1> c((a_or_b? std::forward<T1>(a) : std::forward<T2>(b)));  \
            \
   *c.pData = *c.pData Op (!a_or_b? *a.pData : *b.pData);           \
    return c;                           \
}                   \

struct T {
    T(): pData(new size_t(1)) {std::cout << "malloc" << '\n';}
    ~T() {delete pData;}
    T(const T& b): pData(new size_t(1)) { *pData = *b.pData; std::cout << "malloc" << '\n';}
    T(T&& b){
        pData = b.pData;
        b.pData = nullptr;
        std::cout<< "move constructing" << '\n';
    }

    size_t *pData;          // Memory allocated in the constructor              

    OPERATOR_Fn(+);
    OPERATOR_Fn(-);
    OPERATOR_Fn(&);
    OPERATOR_Fn(|);
};

You can simplify the type_traits expression to make the code more readable by defining something like this

template <typename T1, typename T2>
struct enable_if_same_on_decay{
    static constexpr bool value = std::is_same<std::decay_t<T1>, std::decay_t<T2>>::value;
    typedef std::enable_if<value,std::decay_t<T>>::type type;
};

template <typename T1, typename T2>
using enable_if_same_on_decay_t = typename enable_if_same_on_decay<T1,T2>::type;

The complex type_traits expression

-> typename std::enable_if<std::is_same<std::decay_t<T1>,std::decay_t<T2>>::value,std::decay_t<T1>>::type

simply becomes

-> enable_if_same_on_decay_t<T1,T2>

Upvotes: 0

Alexander
Alexander

Reputation: 758

technically it's feasible. but probably you should consider a design change. the code is just a POC. it has a UB but it works on gcc and clang...

#include <type_traits>
#include <iostream>

    struct T {
        T()
         : pData (new size_t(1))
         , owner(true)
        { 
            
            std::cout << "malloc" << std::endl; 
        }
        ~T()
        {
            if (owner)
            {
                delete pData;
            }
        }
        T(const T &) = default;
        size_t *pData;          // Memory allocated in the constructor              
        bool   owner;           // pData ownership
        
        template <class T1, class T2>
        friend T operator+(T1 && a, T2 && b){
            
            T c(std::forward<T1>(a), std::forward<T2>(b));
            *c.pData = *a.pData + *b.pData; //UB but works
            return c;
        }
        
        private:
        template <class T1, class T2>
        T(T1 && a, T2 && b) : owner(true)
        {  
            static_assert(std::is_same_v<T, std::decay_t<T1>> && std::is_same_v<T, std::decay_t<T2>>, "only type T is supported");
            if (!std::is_reference<T1>::value)
            {
                pData = a.pData;
                a.owner = false;
                std::cout << "steal data a" << std::endl;   
            }
            else if (!std::is_reference<T2>::value)
            {
                pData = b.pData;
                b.owner = false;
                std::cout << "steal data b" << std::endl;   
            }
            else
            {
                std::cout << "malloc anyway" << std::endl;
                pData = new size_t(0);
            }            
        }
    };

int main()
{
    T a, b;
    T r = a +b; // malloc
    std::cout << *r.pData << std::endl;
    T r2 = std::move(a) + b; // no malloc
    std::cout << *r2.pData << " a: " << *a.pData << std::endl;
    T r3 = a + std::move(b); // no malloc
    std::cout << *r3.pData << " a: " << *a.pData << " b: " << *b.pData << std::endl;
    return 0;
}

Upvotes: 0

Bitwize
Bitwize

Reputation: 11220

It would be better to not try to steal resources with operator+ (or any binary operators) and design a more appropriate that may reuse the data in some way1. This should be your APIs idiomatic way for building, if not the only way (if you want to avoid the issue altogether).


Binary operators in C++ like operator+ have the general expectation/convention that it returns a different object without mutating any of its inputs. Defining an operator+ to operate with Rvalues in addition to Lvalues introduces an unconventional interface that will raise confusion for most C++ developers.

Consider your case 4 example:

c = do_something(a) + do_something(b);           // Case 4

Which resource is stolen, a or b? What if a is not big enough to support the result needed from b as well (assuming this uses a resizing buffer)? There's no general case that makes this a simple solution.

Additionally, there is no way to distinguish different kinds of Rvalues on an API like Xvalues (the result of std::move) from PRvalues (the result of a function that returns a value). This means that your same API could be called:

c = std::move(a) + std::move(b);

And in such a case, depending on your above heuristic, only one of a or b might have its resource stolen, which is strange. This would result in the lifetime of the underlying resource not being extended to c, which may go against the developers intuition (consider, for example, if the resource in a or b has observable side-effects, like logging or other system interactions)

Note: It's worth noting that std::string in C++ has the same issue, where operator+ is inefficient. The general recommendation to reuse buffers is to make use of operator+= in such a case


1 A better solution to such a problem is to create a proper method of building in some way, and using this consistently. This could be through well-named functions, a proper builder class of some kind, or just using the compound operators like operator+=

This could even be done through a template helper function that folds a series of arguments into a += concatenation series. Assuming this is in or above, this can be done easily:

template <typename...Args>
auto concat(Args&&...args) -> SomeType
{
    auto result = SomeType{}; // assuming default-constructible

    (result += ... += std::forward<Args>(args));
    return result;
}

Upvotes: 1

Related Questions