error: ambiguous overload for 'operator=' in swap function using the copy-and-swap idiom

Question

While using the copy-and-swap idiom in a class that has constant references as members, the above error occurs.

Example code:

#include <iostream>
#include <functional>

using std::reference_wrapper;

class I_hold_reference;
void swap(I_hold_reference& first, I_hold_reference& second);

class I_hold_reference{
    inline I_hold_reference(const int& number_reference) : my_reference(number_reference){}
    friend void swap(I_hold_reference& first, I_hold_reference& second);
    inline I_hold_reference& operator=(I_hold_reference other){
        swap(*this, other);
        return *this;
    }
    inline I_hold_reference& operator=(I_hold_reference&& other){
        swap(*this, other);
        return *this;
    }
private:
    reference_wrapper<const int> my_reference;
};

void swap(I_hold_reference& first, I_hold_reference& second){
    first = I_hold_reference(second.my_reference); //error: use of overloaded operator '=' is ambiguous (with operand types 'I_hold_reference' and 'I_hold_reference')
}

When the Copy assignment operator is being changed to take its argument by-reference, instead of by-value, the error is fixed.

    inline I_hold_reference& operator=(I_hold_reference& other){ ... }

Why does this fix the error? One possible implication would be that the Important optimization possibility referenced in the linked question is lost. Was that ever true for references? What are the other implications of this change?

There is a codebase relying on this operator, no other members are present, only the mentioned references. Is there a need to adapt the codebase to this change somehow, or is it safe as-is?

You usually don't (never?) want one overload by-value and one overload with rvalue-reference because these will be ambiguous most of the time (always?). See, e.g., https://stackoverflow.com/questions/28701039/ambiguous-call-with-overloaded-r-value-reference-function — Holt, Nov 04 '19 at 09:12
By the way, this is not the copy-and-swap idiom at all... You are basically going to run in infinite recursion here. You cannot use `swap` in `operator=` and `operator=` in swap. You want your swap to actually perform the swap. In your case, the body of swap should be `std::swap(first.my_reference, second.my_reference)`. — Holt, Nov 04 '19 at 09:13
Member function definitions inside a class definition are implicitly `inline`. Not a bug, but clutter decreases readability. — molbdnilo, Nov 04 '19 at 09:18
The swapping and copying and type of the member have nothing to do with it. This is the same situation as `void f(int); void f(int&&); int main{ f(1);}` (i.e. equally "good" overloads). — molbdnilo, Nov 04 '19 at 09:28

score 4 · Accepted Answer · edited Nov 04 '19 at 10:16

If you carefully follow the description you linked, you will see that you must have only one overload of operator= and that one needs to take its argument by value. So, simply removing the operator=(I_hold_reference&&) overload will make your code compilable.

However, this is not the only issue. Your swap doesn't swap! Instead, it assigns a copy of second to first and leaves second untouched.

This is what you want:

class I_hold_reference
{
    I_hold_reference(const int& number_reference)
     : my_reference(number_reference){}

    friend void swap(I_hold_reference& first, I_hold_reference& second)
    {
        using std::swap;
        swap(first.my_reference, second.my_reference);
    }

    I_hold_reference& operator=(I_hold_reference other)
    {
        swap(*this, other);
        return *this;
    }
private:
    reference_wrapper<const int> my_reference;
};

Note: I removed unnecessary inlines as member functions are implicitly inline. Also I declared the swap function inside of your class. You can find an explanation for this in the link you shared.

Also, in this particular example, using the copy-and-swap idiom is unnecessary in the first place. std::reference_wrapper is not a manually maintained resource, meaning it has proper copy and move semantics built in. So, in this particular example, the compiler generated copy and move operators will have the exact same behavior as the manually created one here. So, you should use those and not write you own ones in whatever way. On the other hand, if this is just a toy example and there are more resource in the real class that do need manual management, then this is the way to go.

Oh I made mistakes, thank you for pointing that out! In the actual code, I was not using reference-wrapper, but a const reference. `std::swap` isn't actually compatible with that, but the assignment works there. Additionally in the code I am not using an int reference, but an actual object ( with move assignment constructor ). Would you say the code is valid with these additions? — Dávid Tóth, Nov 04 '19 at 10:48
@DavidTóth References can only be initialized, but not be reassigned thereafter. That's why `swap` doesn't work with it. Also, any assignment operator won't work for a class having a plain reference as a member. Bypassing this limitation is precisely what `reference_wrapper` is good for, so that idea of using it was not bad. But either way, if I understand your scenario correctly, you don't need the copy-and-swap idiom. You only need it if, for example, you manually allocate memory (using `new` and `delete`) or open files or database connections. — sebrockm, Nov 04 '19 at 11:23

darune · Answer 2 · 2019-11-04T10:46:01.537

2

Using forwarding reference && will bind to any rvalue's (temporaries / forwarding references). These categories can also be passed as value and is then ambiguous. That is, a temporary for example is ambiguous where as an lvalue will not be (uses value overload).

Where as the non-const reference can never bind to a temporary or forwarding reference. A const reference can though, but isn't ambiguous.

edited Nov 04 '19 at 10:46

answered Nov 04 '19 at 09:31

darune

10,480
2
24
62

*"If you changed to a const reference [...]"* - You should clarify this part. If you replace the by-value overload by a const-reference overload, there is no ambiguity, and it's basically what we always do when we overload both `operator=(A const&)` and `operator=(A&&)`. If you replace the rvalue-overload with const-reference overload, then there is ambiguity, but OP mentioned replacing the by-value overloads, not the rvalue one. – Holt Nov 04 '19 at 10:26
@Holt thanks a lot - I guess i need to read up on the rules concerning these overloads. – darune Nov 04 '19 at 10:47
Thank you for the clarification! This explains the ambiguity! – Dávid Tóth Feb 17 '21 at 19:45

error: ambiguous overload for 'operator=' in swap function using the copy-and-swap idiom

2 Answers2