Copy assignment operator with multiple inheritance

Viewed 440

My copy constructor below works fine, but I don't understand what is wrong with my copy assignment operator.

#include <iostream>

template <typename... Ts> class foo;

template <typename Last>
class foo<Last> {
    Last last;
public:
    foo (Last r) : last(r) { }
    foo() = default;
    foo (const foo& other) : last(other.last) { }

    foo& operator= (const foo& other) {
        last = other.last;
        return *this;
    }
};

template <typename First, typename... Rest>
class foo<First, Rest...> : public foo<Rest...> {
    First first;
public:
    foo (First f, Rest... rest) : foo<Rest...>(rest...), first(f) { }
    foo() = default;
    foo (const foo& other) : foo<Rest...>(other), first(other.first) { std::cout << "[Copy constructor called]\n"; }

    foo& operator= (const foo& other) {  // Copy assignment operator
        if (&other == this)
            return *this;
        first = other.first;
        return foo<Rest...>::operator= (other);
    }
};

int main() {
    const foo<int, char, bool> a(4, 'c', true);
    foo<int, char, bool> b = a;  // Copy constructor works fine.
    foo<int, char, bool> c;
//  c = a;  // Won't compile.
}

Error message:

error: invalid initialization of reference of type 'foo<int, char, bool>&' from expression of type 'foo<char, bool>'
         return foo<Rest...>::operator= (other);
                                              ^

Can someone point out the problem here?

3 Answers

Your return statement

return foo<Rest...>::operator= (other);

Returns a foo<Rest...> (that's the type of reference operator= is defined with). But it does so from an operator that is supposed to return a foo<First, Rest...>&.

Essentially, you return a Base where a Derived& reference is expected. The reference simply won't bind.

Fortunatly the fix is easy: don't return the result of foo<Rest...>::operator=, return *this instead.

foo& operator= (const foo& other) {  // Copy assignment operator
    if (&other == this)
        return *this;
    first = other.first;
    foo<Rest...>::operator= (other);
    return *this;
}

Looks like return from operator= in the derived class is incorrect:

return foo<Rest...>::operator= (other);

It returns base class, while it should be *this. Change it to

this -> foo<Rest...>::operator= (other);
return *this;

Thanks to StoryTeller, here is an optimized fully compiling solution (operator= delegated to another named member name copy_data that doesn't check for self-assignment, and is implemented recursively):

#include <iostream>

template <typename... Ts> class foo;

template <typename Last>
class foo<Last> {
    Last last;
public:
    foo (Last r) : last(r) { }
    foo() = default;
    foo (const foo& other) : last(other.last) { }

    foo& operator= (const foo& other) {
        if (&other == this)
            return *this;
        last = other.last;
        return *this;
    }
protected:
    void copy_data (const foo& other) {
        last = other.last;
    }
};

template <typename First, typename... Rest>
class foo<First, Rest...> : public foo<Rest...> {
    First first;
public:
    foo (First f, Rest... rest) : foo<Rest...>(rest...), first(f) { }
    foo() = default;
    foo (const foo& other) : foo<Rest...>(other), first(other.first) { std::cout << "[Copy constructor called]\n"; }

    foo& operator= (const foo& other) {  // Copy assignment operator
        if (&other == this)
            return *this;
        first = other.first;
//      foo<Rest...>::operator= (other);
        foo<Rest...>::copy_data(other);
        std::cout << "[Assignment operator called]\n";
        return *this;
    }
protected:
    void copy_data (const foo& other) {
        first = other.first;
        foo<Rest...>::copy_data(other);
    }
};

int main() {
    const foo<int, char, bool> a(4, 'c', true);
    foo<int, char, bool> b = a;  // Copy constructor works fine.
    foo<int, char, bool> c;
    c = b;  // Copy assignment operator works fine (and optimized).
}
Related