2

UPDATE at the bottom.

I want make void tree::operator = ( tree t ) to use the rvalue one.(in this case, in general I want to handle them differently because of efficiency)

I've coded it, used std::move to ensure it will use rvalue, but compiler is saying it doesn't know which operator to choose. Shouldn't he choose that one using rvalues?

Code:

void tree::operator = ( tree&& t )
{
  std::swap(this->pntr, t.pntr);
}

void tree::operator = ( tree t )
{
  *this = std::move(t);
}

Compiler error:

tree.cpp:23:9: error: use of overloaded operator '=' is ambiguous (with operand types 'tree' and 'typename remove_reference<tree &>::type' (aka 'tree'))
  *this = std::move(t);
  ~~~~~ ^ ~~~~~~~~~~~~
tree.cpp:16:12: note: candidate function
void tree::operator = ( tree&& t )
           ^
tree.cpp:21:12: note: candidate function
void tree::operator = ( tree t )
           ^
1 error generated.

I'm using clang-503.0.38. (but with gcc 4.8 is the same error)

UPDATE

Ok, now I have:

tree& tree::operator = ( tree&& t )
{
  std::swap(this->pntr, t.pntr);
}

tree& tree::operator = ( const tree & t )
{
  *this = tree(t); // tree:tree( const tree& t )
}

And it's working. Tommorow I will post what I've learned from this as an Answer.

Marqin
  • 1,096
  • 8
  • 17
  • 2
    You actually have a worse problem than the compilation error. If you didn't have the error you would have an infinite recursive function call in that assignment, as the assignment would call itself to do the assignment. – Some programmer dude Mar 27 '14 at 11:06
  • 1
    You should consider reading [this][1]. [1]: http://stackoverflow.com/questions/9747406/ramification-of-assignment-operators-with-values-instead-of-references – galop1n Mar 27 '14 at 11:07

2 Answers2

3

You can code just one version tree::operator=(tree t) which is implemented with copy-and-swap idiom, while copy constructor and move constructor are both provided.

Thus, the class client can choose copy assignment for example:

tree1 = tree2;

, while can also choose move assignment for example:

tree1 = std::move(tree2);
cbel
  • 1,027
  • 8
  • 15
  • I want to have both copy and move assignements. It's C++11, we have move for purpouse. – Marqin Mar 27 '14 at 23:00
  • The code you updated can work. But copy-and-swap is considered the best implementation, please read this [link](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). With this idiom, you need just one but not two operator=(). – cbel Mar 27 '14 at 23:59
  • I need different operators to differently handle rvalues and lvalues. It's my C++ class assignment. – Marqin Mar 28 '14 at 00:14
2

Your code has several problems:

  • Assigment operator should return a reference to the object itself. That is, should return tree&, not void.

  • Both tree::operator=( tree other ) and tree::ooperator( tree&& other ) could take rvalues thus the overloads are ambiguous. If you need/want rvalue assigment only, your copy-and-swap based rvalue assigment is correct, but if you need both rvalues and lvalues you should provide only by value assigment and use the copy and swap idiom on it:

    tree& tree::operator=( tree other )
    {
        using std::swap; //Enable ADL (Not strictly neccesary, but good practice)
    
        swap( *this , other );
    
        return *this;
    }
    

    Note that you should write a custom swap() function to break the potential infinite recursion on the std::swap() default implementation. Read this thread for more considerations about the copy and swap idiom.

Community
  • 1
  • 1
Manu343726
  • 13,969
  • 4
  • 40
  • 75
  • I want to have both copy and move assignements. It's C++11, we have move for purpouse. – Marqin Mar 27 '14 at 23:00
  • @markin As I said in the answer, by value assigment could take both rvalues and lvalues: **Thats why your implementation is ambiguous, and only by value assigment is neccesary to implement rvalue and lvalue assigment at the same time.** – Manu343726 Mar 27 '14 at 23:35
  • See Update. I've corrected the copy assignment. But also thanks for pointing that void assignment is wrong. – Marqin Mar 27 '14 at 23:49
  • @Marqin don't do that. The point of the by value assigment and copy and swap idiom is to reduce code duplicación and let the compiler perform better optimizations. **Provide only a by value assigment instead of a rvalue one and an lvalue one which copies the input and calls the rvalue assigment. If you want to copy, let the compiler decide how to copy (If its neccesary, which is not in this case) instead of doing it yourself by hand. Thats the key point of the by value pass for both rvalues and lvalues: Let the compiler perform copy ellisions whenever possible** – Manu343726 Mar 27 '14 at 23:58
  • I need different operators to differently handle rvalues and lvalues. It's my C++ class assignment. I just made it for SO to use the rvalue one by lvalue. **But in my implementation the lvalue one works more efficient in some cases than rvalue one, because of avoiding unnecessary memory allocation.** – Marqin Mar 28 '14 at 00:17