What the proper way to return a copy of yourself inside yourself?

Question

Node*Clone(){
    numClones++;    
    Node*cloned=new Node(*this);
    return cloned;
}

I have a default constructor (taking no arguments) and I have no declared copy constructors, so I'd expect this to simply copy the entire contents of the memory of *this and return a pointer to it.

However, I am getting error on the new line above:

Call to implicitly deleted copy constructor

Two possible issues but I don't think they should affect this:

this has a unique_ptr attached to it. That is it contains a container of unique_ptr and is itself stored in another container of unique_ptr. But I am not moving that pointer, I am creating new object not attached to the prior object, right?
this is actually a subclass of Node but I want to return a pointer to a base class

unique_ptr can not be copy constructed, hence the copy ctor of the Node class is not generated, you have to write one by hand. Would be useful to see the definition of Node class. Have you tried "new (*this)" ? — Akanksh, May 17 '13 at 15:04
What do you mean "attached to it?" Is it held in a `unique_ptr`, or does the class have a `unique_ptr` member? The latter prevents generation of an implicit copy ctor, because `unique_ptr` is noncopyable. — Angew is no longer proud of SO, May 17 '13 at 15:07
@Akanksh: How did you know what `this has a unique_ptr attached to it` means? — Sebastian Mach, May 17 '13 at 15:09
@Angew actually both, it contains a container of unique_ptr and is itself stored in another container of unique_ptr. — johnbakers, May 17 '13 at 15:09
@phresnel: I don't, but the error mentioned seems to indicate that the copy ctor is being suppressed in some way. My note about unique_ptr was only to hint the possibility, and not state that it by itself is the issue. — Akanksh, May 17 '13 at 15:10
@Fellowshee A container of `unique_ptr`s can't be copied, because the `unique_ptr`s can't be copied. You need to provide a custom copy ctor for your class and deal with this (perhaps deep-copy the uniquely-pointed-to objects). — Angew is no longer proud of SO, May 17 '13 at 15:14
I'd have thought the name 'unique pointer' would be somewhat of a give-away, it *has* to be unique, only one unique pointer can point to any given object. — jleahy, May 17 '13 at 15:15

Andrew · Accepted Answer · 2013-05-17T15:23:23.240

2

As Jeffrey said, std::unique_ptr cannot be copy-constructed.

Also your Clone methods is not logically correct. Because you are instantiating a Node object there - a base class. So it will not be a copy actually

As a solution you may declare a Clone method in the base class and make in pure virtual:

class Node
{
    virtual Node * Clone() const = 0;
}

And make your subclasses implement it

PS: don't forget about virtual or protected dtor in the interface

edited May 17 '13 at 15:23

answered May 17 '13 at 15:12

Andrew

24,218
13
61
90

I'd also recommend declaring a defaulted virtual destructor for this "interface", or else there might be a problem with objects not being destroyed. – May 17 '13 at 15:20

score 1 · Answer 2 · answered May 17 '13 at 15:11

The error message "Call to implicitly deleted copy constructor" means that your class, one of its base classes and/or some of the data members of that class or any of its base classes are declared as non-copyable (or are treated as non-copyable by the compiler which may happen, for example, when at least one of data members is a reference). Therefore, compiler cannot come up with a "default" copy constructor that you are attempting to call with new Node(*this);

The proper way in such a case is to actually find what objects exactly are non-copyable and either make them copyable so that compiler could create an implicitly defined copy constructor, or implement your very own "copying logic" and not use a copy constructor.

Another alternative is to use some sort of a "smart" pointer pointer (i.e. std::shared_ptr). But in that case you get a shallow copy instead of a deep copy.

In C++, you should strive to put your custom copy logic **into** the (custom) copy ctor, instead of "not using" it. — Angew is no longer proud of SO, May 17 '13 at 15:13
@Angew: Agree, though it might not always be possible. So you should strive, but as a last resort... — , May 17 '13 at 15:15

score 0 · Answer 3 · edited May 23 '17 at 11:43

0

Since you are doing a copy of yourself, I would really suggest you implement a copy constructor. It's best practice to control, plus if you add memory points in the class, you will really mock things up..

Don't rely on this in your case, here is why not:

Conditions for automatic generation of default/copy/move ctor and copy/move assignment operator?

edited May 23 '17 at 11:43

Community

1
1

answered May 17 '13 at 15:08

Dory Zidon

10,497
2
25
39

Shoe · Answer 4 · 2013-05-17T15:15:42.440

0

The problem is caused by your attempt to copy construct a std::unique_ptr (by copying a Node) which, by definition, does not have a copy constructor.

You probably meant to use std::shared_ptr.

edited May 17 '13 at 15:15

answered May 17 '13 at 15:10

Shoe

74,840
36
166
272

What the proper way to return a copy of yourself inside yourself?

4 Answers4