0

Sorry for the larger amount of the source code. There three abstract classes P, L, PL. The third class PL is derived from classes P and L using the virtual inheritance:

template <typename T>  //Abstract class
class P
{

    public:
            virtual ~P() = 0;
    virtual P <T> *clone() const  = 0;
};

template <typename T>
P<T>::~P() {}

template <typename T>
P<T> * P <T>:: clone() const { return new P <T> ( *this ); }

template <typename T>  //Abstract class
class L
{
    public:
            virtual ~L() = 0;
    virtual L <T> *clone() const = 0;
};

template <typename T>
L<T>::~L() {}

template <typename T>
L<T> *L <T> ::clone() const { return new L <T> ( *this );}

template <typename T>
class PL: virtual public P <T>, virtual public L <T>  //Abstract class
{
    public:
            PL() : P <T>(), L<T>() {}
    virtual ~PL() = 0;
    virtual PL <T> *clone() const  = 0;
};

template <typename T>
PL<T>::~PL() {}

template <typename T>
PL<T> * PL <T> :: clone() const { return new PL <T> ( *this );}

Every class has its own implementation of the clone method.

Two next classes PA, PC are derived from class P using the virtual inheritance:

template <typename T>
class PC : virtual public P <T>
{
    public:
            PC() : P <T> () {}
            virtual ~PC() {}
            virtual PC <T> *clone() const {return new PC <T> ( *this );}
};

template <typename T>
class PA : virtual public P <T>
{
    public:
            PA() : P <T> () {}
            virtual ~PA() {}
            virtual PA <T> *clone() const {return new PA <T> ( *this );}
};

The last two classes PCL and PAL are dirived using the virtual inheritance from PC and PL , PA and PL.

template <typename T>
class PCL : public PC <T>, public PL <T>
{
    public:
            PCL() : P <T> (), PC <T> (), PL <T> ()  {}
            virtual ~PCL() {}
            virtual PCL <T> *clone() const {return new PCL <T> ( *this );}
};

template <typename T>
class PAL : public PA <T>, public PL <T>
{
    public:
            PAL() : P <T> (), PA <T> (), PL <T> () {}
            virtual ~PAL() {}
            virtual PAL <T> *clone() const {return new PAL <T> ( *this );}

};

There is the diagram of the class dependencies:

.......... P .... L.....
........../|\..../......
........./.|.\../.......
......../..|..\/........
.......PC..PA..PL.......
.......|...|.../|.......
.......|...|../.|.......
.......|...PAL..|.......
.......|........|.......
.......PCL_____/........  

Please, do not discuss this proposal :-))). I have the following 3 questions:

1) Was this class dependency rewritten in C++ correctly (first of all the placemement of "virtual")?

2) I am not sure what is wrong, see the code, please:

int main(int argc, _TCHAR* argv[])
{
PCL <double> * pcl = new PCL <double>(); //Object of abstract class not allowed
PAL <double> * pal = new PAL <double>(); //Object of abstract class not allowed
PL <double> *pl1 = pcl->clone(); //Polymorphism
PL <double> *pl2 = pal->clone(); //Polymorphism
return 0;
} 

It is not possible to create new objects of PAL / PCL classes, both classes are marked as abstract. But they are not abstract. Where is the problem?

3) Is it possible to use polymorphism together with clone() method? See the code above, please...

Thanks for your help...


UPDATED QUESTION

I corrected the code. But the following error using VS 2010 compiler appear:

template <typename T>  //Abstract class
class P
{
    public:
    virtual ~P() = 0;
    virtual P <T> *clone() const  = 0;
};

template <typename T>
P<T>::~P() {}

template <typename T>
P<T> * P <T>:: clone() const { return new P <T> ( *this ); }


template <typename T>  //Abstract class
class L
{
    public:
    virtual ~L() = 0;
    virtual L <T> *clone() const = 0;
};

template <typename T>
L<T>::~L() {}

template <typename T>
L<T> *L <T> ::clone() const { return new L <T> ( *this );}


template <typename T>
class PL: virtual public P <T>, virtual public L <T>  //Abstract class
{
    public:
            PL() : P <T>(), L<T>() {}
    virtual ~PL() = 0;
    virtual PL <T> *clone() const  = 0; 
};

template <typename T>
PL<T>::~PL() {}

template <typename T>
PL<T> * PL <T> :: clone() const { return new PL <T> ( *this );}


template <typename T>
class PC : virtual public P <T>
{
    protected:
            T pc;
    public:
            PC() : P <T> () {}
    virtual ~PC() {}
            virtual PC <T> *clone() const {return new PC <T> ( *this );}
};

template <typename T>
class PA : virtual public P <T>
{
    public:
            PA() : P <T> () {}
    virtual ~PA() {}
            virtual PA <T> *clone() const {return new PA <T> ( *this );}
};

template <typename T>
class PCL : public PC <T>, public PL <T>
{
public:
            PCL() : P <T> (), PC <T> (), PL <T> () {}
            virtual ~PCL() {}
            virtual PCL <T> *clone() const {return new PCL <T> ( *this   );}
}; //Error using VS 2010: Error 1   error C2250: 'PCL<T>' : ambiguous inheritance of 'PC<T> *P<T>::clone(void) const'


template <typename T>
class PAL : public PA <T>, public PL <T>
{
public:
            PAL() : P <T> (), PA <T> (), PL <T> ()  {}
            virtual ~PAL() {}
            virtual PAL <T> *clone() const {return new PAL <T> ( *this );}
}; //Error VS 2010: Error   1   error C2250: 'PAL<T>' : ambiguous inheritance of 'PA<T> *P<T>::clone(void) const'


int main(int argc, char* argv[])
{
PCL <double> * pcl = new PCL <double>();
PAL <double> * pal = new PAL <double>();
PL <double> *pl1 = pcl->clone();
PL <double> *pl2 = pal->clone();
return 0;
}

Maybe i overlooked something... But g++ compiles this code OK.

Johnas
  • 1,263
  • 3
  • 12
  • 23
  • 1
    Do your pure virtual destructors in `P`, `L` and `PL` have a default implementation? – Chad Aug 03 '11 at 13:49
  • I think you cannot add definitions to pure function declarations; say `= 0;`. Also there are parentheses missing after `PL { }` (should be `PL () { }`). As @Chad says, you don't need to make the destructors pure virtual if you already have other pure virtual functions. – Kerrek SB Aug 03 '11 at 13:49
  • @Chad: Yes, I corrected the code... – Johnas Aug 03 '11 at 13:53

5 Answers5

3

Just some small errors in your code:

  • Get the base initializers right: PAL() : PC <T>(), PL <T>() {} There is no initialization of P<T>, which isn't a direct base; but[Sorry, that was wrong -- you do have to call the virtual base constructor because of the virtual inheritance.] you do have to say the round parentheses.

  • Declare pure virtual functions without definition: virtual L <T> *clone() const = 0;

  • Double-check that you want PL to inherit virtually from P but non-virtually from L (but that's fine).

  • Double-check that all your clone()s have the same constness.

  • If you already have one pure virtual function in your abstract base class, you should not make the destructor pure. Instead, say, virtual ~T() { }. Otherwise you should still provide a definition of the pure virtual destructor (because the destructor always needs to be callable): virtual T::~T() { }

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • Indeed the pure error must come from the clone() not being const. http://ideone.com/3bTty is a cleaned up version of the code, and will compile (though not link). Since the code has so many errors, it is questionable whether it is really corresponding to the code that the OP experiences problems with. – PlasmaHH Aug 03 '11 at 14:00
  • I actually didn't get any warning or errors regarding the constness. I'm not 100% sure right now, but there's a possibility that erroneous *hiding* is happening instead of overriding. I don't want to spend more time with this now, but it's very important to get the signatures right when overriding. – Kerrek SB Aug 03 '11 at 14:01
  • I would not expect any warnings or errors regarding the constness too, it is just another signature, just see it as if it was a typo in the function name. – PlasmaHH Aug 03 '11 at 14:05
  • So, what's the problem? I get it to compile fine, is there any other question? – Kerrek SB Aug 03 '11 at 14:09
  • I have got the following error: Error 1 error C2250: 'PCL' : ambiguous inheritance of 'PC *P::clone(void) const'... See the updated code, please... ; – Johnas Aug 03 '11 at 14:24
  • Thanks for your patience. But I do not see any other problem, maybe I am blind :-). This code does not compile using MSVS but OK with g++.. – Johnas Aug 03 '11 at 14:37
  • I got the virtual base construction wrong, you're right to have `P()` in the initializer. I don't know, it does compile fine in GCC with all warnings. Which version of MSVS do you have? 2010SP1? – Kerrek SB Aug 03 '11 at 14:40
  • I am using MSVS2010 sp1. I post the final version of my code in updated question... Could you check it, please? – Johnas Aug 03 '11 at 14:50
  • Sorry, I only have GCC! Perhaps someone else here could take a look. – Kerrek SB Aug 03 '11 at 14:57
  • Hm, just out of curiosity, and not that you should have to, what happens if you make `PAL` and `PCL` inherit virtually? – Kerrek SB Aug 03 '11 at 15:01
2

MSVC doesn't support co-variant returns properly. Where you think you've been overriding the function, actually you've just been hiding it. That's the problem at hand.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • @Johnas: Can you remove `PL *PL ::clone()`? Because then it compiles for me. (That is, when I remove the implementation of the pure virtual `clone()` functions that attempt to instantiate abstract classes.) – sbi Aug 03 '11 at 15:25
  • Could you give me please an advice how to modify the code as to be translatable using MSVS 2010? – Johnas Aug 03 '11 at 15:26
  • @Johnas: As I said, it compiles for me if I remove `PL *PL ::clone()`. – sbi Aug 03 '11 at 15:28
  • I sent my post at the same moment like you :-) After removing PL * PL :: clone() const { return new PL ( *this );} nothing has changed, the same error... – Johnas Aug 03 '11 at 15:31
  • @Johnas: Do not just remove the _definition_ of the member function, but also its _declaration_. This compiles for me. – sbi Aug 03 '11 at 15:38
  • I am sorry, I do not exactly, what you mean... PL *PL ::clone() is a part of definition, the declaration is virtual PL *clone() const = 0... – Johnas Aug 03 '11 at 15:48
1

This compiles for me using VC10:

template <typename T>  //Abstract class
class P
{
public:
    virtual ~P() = 0;
    virtual P <T> *clone() const  = 0;
};

template <typename T>
P<T>::~P() {}


template <typename T>  //Abstract class
class L
{
public:
    virtual ~L() = 0;
    virtual L <T> *clone() const = 0;
};

template <typename T>
L<T>::~L() {}


template <typename T>
class PL: virtual public P <T>, virtual public L <T>  //Abstract class
{
public:
    PL() : P <T>(), L<T>() {}
    virtual ~PL() = 0;
//  virtual PL <T> *clone() const  = 0; // REMOVED!
};

template <typename T>
PL<T>::~PL() {}


template <typename T>
class PC : virtual public P <T>
{
protected:
    T pc;
public:
    PC() : P <T> () {}
    virtual ~PC() {}
    virtual PC <T> *clone() const {return new PC <T> ( *this );}
};

template <typename T>
class PA : virtual public P <T>
{
public:
    PA() : P <T> () {}
    virtual ~PA() {}
    virtual PA <T> *clone() const {return new PA <T> ( *this );}
};

template <typename T>
class PCL : public PC <T>, public PL <T>
{
public:
    PCL() : P <T> (), PC <T> (), PL <T> () {}
    virtual ~PCL() {}
    virtual PCL <T> *clone() const {return new PCL <T> ( *this   );}
};


template <typename T>
class PAL : public PA <T>, public PL <T>
{
public:
    PAL() : P <T> (), PA <T> (), PL <T> ()  {}
    virtual ~PAL() {}
    virtual PAL <T> *clone() const {return new PAL <T> ( *this );}
};


int main()
{
    PCL <double> * pcl = new PCL <double>();
    PAL <double> * pal = new PAL <double>();
    PL <double> *pl1 = pcl->clone();
    PL <double> *pl2 = pal->clone();
    return 0;
}

Note that I had to remove PL <T> *PL <T>::clone() to get VC to accept the code. The problem with that is that, if you have a PL<T>, and call it's clone(), it will statically return an L<T>*, rather than a PL<T>*, even though the dynamic type is of a class derived from PL<T>.

sbi
  • 219,715
  • 46
  • 258
  • 445
  • OK, it works. But you removed all definitions of clone methods in three abstract classes. – Johnas Aug 03 '11 at 15:57
  • @Johnas: These methods were trying to instantiate objects of their own, _abstract_ class. That certainly won't ever work. But you don't need them anyway, because you will never have _objects_ of these classes (because they are abstract). You will always ever only have objects of derived classes, and those do implement `clone()`. Really, the only problem with the code from my answer is as I have written in my answer underneath the code. – sbi Aug 03 '11 at 16:00
  • I forgot, that I am able to create pointer to abstract class however not the object (nor dynamically)... Thanks for your help... – Johnas Aug 03 '11 at 16:04
0

Because the clone method in your P class is abstract as it is defined

virtual P <T> *clone() const = 0 {};

(btw that {} is incorrect). What you have to understand is that once this template is instantiated this is a separate method with a completely different signature from the derive classes clone method. template instantiation creates behaves as if it generates new code. Thus this is a pure abstract method that never got implemented. Thus making everyone that inherits this method (every derived class) an abstract class.


Edit: As for the third question. Run time and compile time polymorphism dont mix well. I have no idea why on earth you want to use such a complex structure but I am sure that the design can be simplified far far more.

Osada Lakmal
  • 891
  • 2
  • 8
  • 22
  • _Thus this is a pure abstract method that never got implemented_ is not quite right, since those functiosn are not member templates, their return type just depends on the template parameter of the class template, which is just fine. Also the covariant returns of the deriving classes implementations are ok, although hard to spot as such (which is one of the many reasons one should rethink about that design) – PlasmaHH Aug 03 '11 at 14:03
0
  1. There are a couple of typos which need fixing in your code, but the virtuals in the inheritance lattice are correct. (They're the minimum. You can add more; in the rare cases where such lattices occur, it's often best to make all inheritance virtual, to protect against future evolution.)

  2. There's nothing wrong with your test code. Once the typos in the original are fixed, it compiles fine with g++.

  3. It's possible, and it should work the way you've done it.

James Kanze
  • 150,581
  • 18
  • 184
  • 329