Call default copy constructor from within overloaded copy constructor
Asked Answered
D

7

19

I need to write a copy constructor that deep copies the contents of a std::shared_ptr. However, there are a bunch of variable int a, b, c, d, e; also defined in the class. Is there a way to generate the default copy constructor code (or call the default copy constructor) inside my new overloaded one.

Here is a code snippet with a comment that hopefully clarifies the issue.

class Foo {
public:
     Foo() {}
     Foo(Foo const & other);
     ...
private:
     int a, b, c, d, e;
     std::shared_ptr<Bla> p;
};

Foo::Foo(Foo const & other) {
    p.reset(new Bla(*other.p));

    // Can I avoid having to write the default copy constructor code below
    a = other.a;
    b = other.b;
    c = other.c;
    d = other.d;
    e = other.e;
}
Daryldaryle answered 4/9, 2011 at 2:17 Comment(3)
Why do you need to deep-copy a shared pointer? Isn't its whole purpose to share a common resource? If you have unique ownership, consider using a unique_ptr.Investigator
Because each instance of Foo needs a shared_ptr to pass around to several other classes.Daryldaryle
The "..." in the above code provides a public interface to p.Daryldaryle
C
6

Here’s the question code as I’m writing this:

class Foo {
public:
     Foo() {}
     Foo(Foo const & other);
     ...
private:
     int a, b, c, d, e;
     std::shared_ptr<Bla> p;
};

Foo::Foo(Foo const & other) {
    p.reset(new Bla(other.p));

    // Can I avoid having to write the default copy constructor code below
    a = other.a;
    b = other.b;
    c = other.c;
    d = other.d;
    e = other.e;
}

The above code is most likely wrong, because

  1. the default constructor leaves a, b, c, d and e uninitialized, and

  2. the code does not take charge of assignment copying, and

  3. the expression new Bla(other.p) requires that Bla has a constructor taking a std::shared_ptr<Bla>, which is extremely unlikely.

With std::shared_ptr this would have to be C++11 code in order to be formally correct language-wise. However, I believe that it’s just code that uses what’s available with your compiler. And so I believe that the relevant C++ standard is C++98, with the technical corrections of the C++03 amendment.

You can easily leverage the built-in (generated) copy initialization, even in C++98, e.g.

namespace detail {
    struct AutoClonedBla {
        std::shared_ptr<Bla> p;

        AutoClonedBla( Bla* pNew ): p( pNew ) {}

        AutoClonedBla( AutoClonedBla const& other )
            : p( new Bla( *other.p ) )
        {}

        void swap( AutoClonedBla& other )
        {
            using std::swap;
            swap( p, other.p );
        }

        AutoClonedBla& operator=( AutoClonedBla other )
        {
            other.swap( *this );
            return *this;
        }
    };
}

class Foo {
public:
     Foo(): a(), b(), c(), d(), e(), autoP( new Bla ) {}
     // Copy constructor generated by compiler, OK.

private:
     int                      a, b, c, d, e;
     detail::AutoClonedBla    autoP;
};

Note that this code does initialize correctly in the default constructor, does take charge of copy assignment (employing the swap idiom for that), and does not require a special smart-pointer-aware Bla constructor, but instead just uses the ordinary Bla copy constructor to copy.

Cachucha answered 4/9, 2011 at 6:43 Comment(1)
The Bla copy constructor is not intended to take a shared_ptr. Good catch. The answer still stands, and is very good, thank you.Daryldaryle
I
11

I always think that questions like this should have at least one answer quote from the standard for future readers, so here it is.

§12.8.4 of the standard states that:

If the class definition does not explicitly declare a copy constructor, one is declared implicitly.

This implies that when a class definition does explicitly declare a copy constructor, one is not declared implicitly. So if you declare one explicitly, the implicit one does not exist, so you can't call it.

Iscariot answered 4/9, 2011 at 2:40 Comment(6)
No, that quote from the standard doesn't imply that.Lalalalage
@wilhelm why not? If the if of any statement is not satisfied, the then isn't true. Even if that generality is wrong, this particular application of it seems correct. I've never been wrong before though.Iscariot
"If not A then B" does not imply "if A then not B". But I know what you mean and in this case "if A then not B" is also true.Daryldaryle
@Lex well I was thinking about English. For instance, if your mom says to you "If you're good I'll take you to the toy store" you can assume that if you are bad she will not take you to the toy store.Iscariot
Yes, that's a good guess on your part (in terms of Mom), but it's not a guarantee, and the standard is all about defining guarantees.Daryldaryle
@SethCarnegie I believe, that that Mom will also (eventually) take the child to the toy store, even, if he is not so good directly after saying that. So this mother's talk is exactly one example, that out of a condition IF A then B, one can not conclude, that IF not A then never ever B.Compressed
C
6

Here’s the question code as I’m writing this:

class Foo {
public:
     Foo() {}
     Foo(Foo const & other);
     ...
private:
     int a, b, c, d, e;
     std::shared_ptr<Bla> p;
};

Foo::Foo(Foo const & other) {
    p.reset(new Bla(other.p));

    // Can I avoid having to write the default copy constructor code below
    a = other.a;
    b = other.b;
    c = other.c;
    d = other.d;
    e = other.e;
}

The above code is most likely wrong, because

  1. the default constructor leaves a, b, c, d and e uninitialized, and

  2. the code does not take charge of assignment copying, and

  3. the expression new Bla(other.p) requires that Bla has a constructor taking a std::shared_ptr<Bla>, which is extremely unlikely.

With std::shared_ptr this would have to be C++11 code in order to be formally correct language-wise. However, I believe that it’s just code that uses what’s available with your compiler. And so I believe that the relevant C++ standard is C++98, with the technical corrections of the C++03 amendment.

You can easily leverage the built-in (generated) copy initialization, even in C++98, e.g.

namespace detail {
    struct AutoClonedBla {
        std::shared_ptr<Bla> p;

        AutoClonedBla( Bla* pNew ): p( pNew ) {}

        AutoClonedBla( AutoClonedBla const& other )
            : p( new Bla( *other.p ) )
        {}

        void swap( AutoClonedBla& other )
        {
            using std::swap;
            swap( p, other.p );
        }

        AutoClonedBla& operator=( AutoClonedBla other )
        {
            other.swap( *this );
            return *this;
        }
    };
}

class Foo {
public:
     Foo(): a(), b(), c(), d(), e(), autoP( new Bla ) {}
     // Copy constructor generated by compiler, OK.

private:
     int                      a, b, c, d, e;
     detail::AutoClonedBla    autoP;
};

Note that this code does initialize correctly in the default constructor, does take charge of copy assignment (employing the swap idiom for that), and does not require a special smart-pointer-aware Bla constructor, but instead just uses the ordinary Bla copy constructor to copy.

Cachucha answered 4/9, 2011 at 6:43 Comment(1)
The Bla copy constructor is not intended to take a shared_ptr. Good catch. The answer still stands, and is very good, thank you.Daryldaryle
K
4

It would be easier to write a variation on shared_ptr that has deep copying built into it. That way, you don't have to write a copy constructor for your main class; just for this special deep_copy_shared_ptr type. Your deep_copy_shared_ptr would have a copy constructor, and it would store a shared_ptr itself. It could even have an implicit conversion to shared_ptr, to make it a bit easier to use.

Krystinakrystle answered 4/9, 2011 at 2:33 Comment(1)
+1 for pointing out a solution to the OP's question. I was just slightly surprised that this answer had 0 votes (before mine), and was not the one marked as solution.Cachucha
L
2

That's not possible. It's either you write a custom copy constructor (entirely on your own) or the compiler writes it for you.

Note that if you write a copy constructor then you probably need a copy assignment and a destructor as well, because writing any of these three resource-management functions implies you're managing a resource. With the copy-and-swap idiom, however, you only need to write the copy logic once, in the copy constructor, and you then define the assignment operator in terms of the copy constructor.

Aside from that, I'm not entirely sure why you're using a shared_ptr<>. The point of a shared_ptr<> is to allow multiple pointers to safely point at the same object. But you're not sharing the pointee, you deep-copy it. Maybe you should use a raw pointer instead, and free it in the destructor. Or, better yet, replace the shared_ptr<> with a clone_ptr, and then eliminate the copy constructor, copy assignment and destructor altogether.

Lalalalage answered 4/9, 2011 at 2:25 Comment(7)
Destructor and assignment should be fine here because the shared pointer basically already takes care of everything, but it's a very unusual design choice for sure.Investigator
He probably needs at the very least an assignment operator, or else assigning to an object and copy-constructing an object would do different things -- which is counter-intuitive. But indeed, this is an unusual case, so much so that I just got confused myself into thinking he needs a destructor. And that's a sign for a design that could do better (in the name of "least surprise").Lalalalage
What exactly is unusual here? The assignment operator is also defined in the "...". That's not the point of the question. The point is that it would be nice to not have to re-write copying code for int a, b, c, d, e; that could already be generated by the compiler.Daryldaryle
@LexFridman it would be nice, yes, but again: that's not possible. You're asking to write some of the copy logic, and delegate the rest for the compiler to write. C++ doesn't allow that: you either don't say a word, and the compiler generates the copy constructor for you, or you explicitly ask the compiler not to generate a copy ctor, and the compiler will write none of it for you.Lalalalage
@LexFridman what's unusual is that your code as it is doesn't really manage a resource, and yet it relies on a custom copy logic. That is unusual, and you could probably improve this design.Lalalalage
My code does not show the full class, so how can you say that it doesn't manage a resource? It may allocate memory to the smart pointer, it may expose that smart pointer to the world, etc.Daryldaryle
@LexFridman a smart pointer is a not a resource you manage. Hence "smart".Lalalalage
F
1

Not to my knowledge, but what you can (and should) do is use an initializer list, anyway:

Foo::Foo(Foo const & other)
    : a(other.a), b(other.b), c(other.c), 
      d(other.d), e(other.e), p(new Bla(other.p))
{}

This won't save you from the writing, but it will save you from the possible performance penalty of assigning (unneccessarily) default-constructed members (although in this case it might be fine) and the many other pitfalls this could bring. Always use initializer lists in constructors, if possible.

And by the way, Kerrek's comment is right. Why do you need a shared_ptr, if you make a deep copy anyway. In this case a unique_ptr might be more appropriate. Besides it's benefits shared_ptr is not a general no-more-to-think-about-deallocation solution and you should always think if you need a smart pointer and what type of smart pointer is most appropriate.

Fastback answered 4/9, 2011 at 2:21 Comment(1)
Generally speaking it's not a good idea to allocate (dynamic) resources in an initializer list. If you allocate two resources in an initializer list and the second one fails constructing then you have a leak (in the first resource) which you cannot possibly fix.Lalalalage
H
0

The default assignment operator has the same code as the default copy constructor. Though you can't call the default copy constructor from your overload, the same code exists in the assignemnt so.

Foo::Foo(Foo const & other) 
{
    *this = other;
    p.reset(new Bla(other.p));
}

should get you what you need.

Edit: Nevermind it is in fact the same code, and explicitly declaring the copy construtor prevents it generation :(

Halfmoon answered 25/5, 2018 at 19:17 Comment(2)
What does the assignment operator do to p?Genitive
The default assignment operator simply default to calling the default assignment operator on all the members. So for p it is the equivalent of calling p = other.p p is a shared_ptr so it grabs the value and increments the ref count. Assuming by above code were made to work, the p.reset would then just decrement that ref count for the original object.Halfmoon
C
0

One trick can be to put those "simple" fields into a base class

class FooBase {
protected:
    int a, b, c, d, e;
};

class Foo : public FooBase {
public:
     Foo() {}
     Foo(const Foo& other)
       : FooBase(other),         // This copies all the simple fields
         p(new Bla(other.p))     // Manually do the hard part
     {}
     ...
private:
     std::shared_ptr<Bla> p;
};

Then later, if you add any simple fields to Foo, you can put them to FooBase and don't need to worry about updating the copy constructor.

Don't forget to add a similiar copy assignment operator, if needed. And on modern compilers, also add move constructor and move assignement operator. These two can probably be defaulted, as you don't need deep copy on move operations.

Compressed answered 10/4, 2021 at 15:6 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.