c++: operator = is ambiguous when implementing move assignment

Question

I am trying to implement a rule of five for the first time. After reading a lot of recommendation about best practices, I end up with a solution where copy/move assignment operators seem to be in some conflict.

Here is my code.

#include <vector>   
#include <memory>

template<class T> class DirectedGraph {
public:
    std::vector<T> nodes;
    DirectedGraph() {}
    DirectedGraph(std::size_t n) : nodes(n, T()) {}
    // ... Additional methods ....
};

template<class T>
DirectedGraph<T> Clone(DirectedGraph<T> graph) {
    auto clone = DirectedGraph<T>();
    clone.nodes = graph.nodes;
    return clone;
}

template<class T> class UndirectedGraph
{
    using TDirectedG = DirectedGraph<T>;
    using TUndirectedG = UndirectedGraph<T>;

    std::size_t numberOfEdges;
    std::unique_ptr<TDirectedG> directedGraph;
public:
    UndirectedGraph(std::size_t n)
        : directedGraph(std::make_unique<TDirectedG>(n))
        , numberOfEdges(0) {}

    UndirectedGraph(TUndirectedG&& other) {
        this->numberOfEdges = other.numberOfEdges;
        this->directedGraph = std::move(other.directedGraph);
    }

    UndirectedGraph(const TUndirectedG& other) {
        this->numberOfEdges = other.numberOfEdges;
        this->directedGraph = std::make_unique<TDirectedG>
            (Clone<T>(*other.directedGraph));
    }

    friend void swap(TUndirectedG& first, TUndirectedG& second) {
        using std::swap;
        swap(first.numberOfEdges, second.numberOfEdges);
        swap(first.directedGraph, second.directedGraph);
    }

    TUndirectedG& operator=(TUndirectedG other) {
        swap(*this, other);
        return *this;
    }

    TUndirectedG& operator=(TUndirectedG&& other) {
        swap(*this, other);
        return *this;
    }

    ~UndirectedGraph() {}
};

int main()
{
    UndirectedGraph<int> graph(10);
    auto copyGraph = UndirectedGraph<int>(graph);
    auto newGraph = UndirectedGraph<int>(3);
    newGraph = graph;            // This works.
    newGraph = std::move(graph); // Error here!!!
    return 0;
}

Most of the recommendations I took from here, and I implemented copy assignment operator= to accept parameter by value. I think this might be a problem, but I don't understand why.

Additionally, I would highly appreciate if someone point out whether or not my copy/move ctor/assignments are implemented in the correct way.

With the copy-and-swap idiom, you don't need an additional move-assignment operator — Piotr Skotnicki
– Piotr Skotnicki, Commented Dec 6, 2018 at 13:39
copy-and-swap covers both const& and &&. Either do copy-and-swap (only one overload taking param by value) or implement const& and && overloads for operator= — An0num0us
– An0num0us, Commented Dec 6, 2018 at 13:40
Since you decided (why?) to do assignment by value, the argument can be passed through the move constructor. (By-value assignment only causes unnecessary copying. Don't do it.) — molbdnilo
– molbdnilo, Commented Dec 6, 2018 at 13:42
@molbdnilo that's an idiom, whether it's a copy or a move depends on the usage — Piotr Skotnicki
– Piotr Skotnicki, Commented Dec 6, 2018 at 13:44
@Dejan it's "four and a half" because an assignment operator accepting a parameter by value can be used for copy- and move-assignment. this is because the argument expression can be either an lvalue or rvalue, and the parameter will be either move- or copy-constructed from that argument expression. as such, you don't need to provide a copy-assignment operator nor a move-assignment operator, because the one accepting parameter by value allows you to construct that parameter itself though move or copy construction. once the parameter is constructed,its content is swapped with the implicit object — Piotr Skotnicki
– Piotr Skotnicki, Commented Dec 6, 2018 at 17:48

Jarod42 · Accepted Answer · 2018-12-06 17:26:50Z

6

You should have:

TUndirectedG& operator=(const TUndirectedG&);
TUndirectedG& operator=(TUndirectedG&&);

or

TUndirectedG& operator=(TUndirectedG);

Having both

TUndirectedG& operator=(TUndirectedG);   // Lead to ambiguous call
TUndirectedG& operator=(TUndirectedG&&); // Lead to ambiguous call

would lead to ambiguous call with rvalue.

answered Dec 6, 2018 at 17:26

Jarod42

227k15 gold badges214 silver badges350 bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

c++: operator = is ambiguous when implementing move assignment

1 Answer 1

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related