8

I'm trying to implement a Clonable class with the CRTP. However, I need to have abstract class that have a pure virtual clone method, overridden by child classes. To make this happen, I need the clone function to return a covariant return type. I made this code below, and the compiler shout at me this error:

main.cpp:12:5: error: return type of virtual function 'clone' is not covariant with the return type of the function it overrides ('B *' is not derived from 'AbstractClonable *')

The class 'B' seems to be a child class of AbstractClonable, and even by two way! How can I solve this? Thank you very much. I tried with both with clang 3.6 and GCC 4.9.2

struct AbstractClonable {
    virtual AbstractClonable* clone() const = 0;
};

template<typename T>
struct Clonable : virtual AbstractClonable {
    T* clone() const override {
        return new T{*dynamic_cast<const T*>(this)};
    }
};

struct A : virtual AbstractClonable {

};

struct B : A, Clonable<B> {

};
Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
  • Do you really need AbstractClonable? Why? I'm really curious. Are there legitimate use cases? You can clone it, and what would you do with the result? Clone it again? – n. m. could be an AI May 15 '15 at 06:56
  • Yes. If I have a collection of A and A is abstract and I need to clone each object, I need something that says that I need to implement clone in subclasses. Since A is abstract, it cannot inherits from Clonable, because of the new in the clone function. I came up with AbstractClonable. – Guillaume Racicot May 15 '15 at 14:11

3 Answers3

7

Even if B is indeed derived from Clonable<B>, the problem here is that Clonable<B> construction is not valid, as it defines 

B* clone() const override

which of course is not an override of AbstractClonable::clone(), since the compiler doesn't see B at this point as a child of AbstractClonable. So I believe the issue lays in the fact that the compiler cannot build the Clonable<B> base of B.

A workaround (but not really the same as what you want) is to define

Clonable* clone() const override

in Clonable. As you mentioned in the comment, you can also define a free function 

template<typename T> 
T* clone(const T* object) 
{ 
    return static_cast<T*>(object->clone()); 
}

Related: Derived curiously recurring templates and covariance

Community
  • 1
  • 1
vsoftco
  • 55,410
  • 12
  • 139
  • 252
  • shouldn't virtual inheritance take care of that? – W.F. May 15 '15 at 05:49
  • AFAIK, the virtual derivation just removes the "duplicate" base from the picture. I don't think it makes this construction work. – vsoftco May 15 '15 at 05:51
  • B is a child of A and A is a child of AbstractClonable. And we can see that B is a child of the class Clonable, which is a child of AbstractClonable. B is related to AbstractClonable two times. How's that B is not a child of AbstractClonable? – Guillaume Racicot May 15 '15 at 05:52
  • 1
    `B` is a child of `AbstractClonable`, but when first constructing `Clonable`, `B` is an incomplete type, and `Clonable` is not seen as a child of `AbstractClonable`. I don't think the compiler can successfully fully define `B` without first trying to resolve its base classes. I have to say I am not 100% sure of this. Related (but without an accepted answer) http://stackoverflow.com/q/17201268/3093378 – vsoftco May 15 '15 at 05:53
  • It certainly not the semantic I was looking for, but it will do the trick I guess – Guillaume Racicot May 15 '15 at 06:05
  • It did the trick with your workaround and this function below: `template T* clone(const T* object) { return static_cast(object->clone()); }` Can you add this to your answer? I'll mark it as accepted answer. – Guillaume Racicot May 15 '15 at 06:38
  • @GuillaumeRacicot Is the updated edit what you meant? It works for me. – vsoftco May 15 '15 at 06:41
  • I added a new function clone outside the scope of the class to automatically cast down the object. I use it like this: `clone(myB)` – Guillaume Racicot May 15 '15 at 06:45
6

Yes, B is derived from AbstractClonable, but the compiler doesn't know that during the instantiation of Clonable<B> because B is still incomplete at that point.

C++14 §10.3/8:

If the class type in the covariant return type of D::f differs from that of B::f, the class type in the return type of D::f shall be complete at the point of declaration of D::f or shall be the class type D.

A class has special permission to use itself in a covariant return type. Other classes, including CRTP bases, need to wait until the class is complete before declaring a covariant function.

You can solve the problem using the non-virtual interface idiom (NVI):

class AbstractClonable {
protected:
    virtual AbstractClonable* do_clone() const = 0;
public:
    AbstractClonable *clone() const {
        return do_clone();
    }
};

template<typename T>
class Clonable : public virtual AbstractClonable {
    Clonable* do_clone() const override { // Avoid using T in this declaration.
        return new T{*dynamic_cast<const T*>(this)};
    }
public:
    T *clone() const { // But here, it's OK.
        return static_cast< T * >( do_clone() );
    }
};
Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
0

I think the problem is that

T* clone() const override{
    return new T{*dynamic_cast<const T*>(this)};
}

returns B* instead of AbstractClonable *.

trbvm
  • 141
  • 7
  • Yes it returns a B*, and it's my goal! Shouldn't they be valid covariant types since B inherits from AbstractClonable? – Guillaume Racicot May 15 '15 at 05:47
  • Since there is a multiple virtual inheritance involved casting from B* to AbstractClonable* requires additional code to be emitted. This code will perform base class lookup using pointer to a virtual base class. If you call clone() method using a pointer to AbstractClonable compiler probably is not able to perform typecasting properly. So I don't think B* and AbstractClonable* can be considered covariant. – trbvm May 15 '15 at 06:30