21
error: use of deleted function 'A::A(const A&)'
 return tmp;
        ^~~

Why is the copy constructor called only when there is a virtual destructor in A? How to avoid this?

struct B {};

struct A{
    std::unique_ptr<B> x;
    virtual ~A() = default;
};

A f() {
    A tmp;
    return tmp;
}
Sobuch
  • 313
  • 1
  • 8
  • 1
    see: [In which situations is the C++ copy constructor called?](https://stackoverflow.com/questions/21206359/in-which-situations-is-the-c-copy-constructor-called) – kmdreko Mar 21 '19 at 20:29
  • 5
    C++ handles objects different than C#/Java. When an instance goes out of scope (`tmp` here) its destructor must be called. Therefore, when you `return tmp` then you're asking it to make a copy of `tmp` to be return to whomever calls the function. Once copied, `tmp` will be destroyed and its copy will be available for use. – Everyone Mar 21 '19 at 20:29
  • 3
    @Everyone except that it is usually a move rather than a copy, which is what the question is about. – Quentin Mar 22 '19 at 09:24
  • A little surprising as I would have thought that RVO would have been invoked, resulting in no move or copy. – Adrian Apr 01 '19 at 19:53

1 Answers1

33

virtual ~A() = default; is a user declared destructor. Because of that, A no longer has a move constructor. That means return tmp; can't move tmp and since tmp is not copyable, you get a compiler error.

There are two ways you can fix this. You can add a move constructor like

struct A{
    std::unique_ptr<B> x;

    A() = default; // you have to add this since the move constructor was added
    A(A&&) = default; // defaulted move
    virtual ~A() = default;
};

or you can create a base class that has the virtual destructor and inherit from that like

struct C {
    virtual ~C() = default;
};

struct A : C {
    std::unique_ptr<B> x;
};

This works because A no longer has a user declared destructor (Yes, C does but we only care about A) so it will still generate a move constructor in A. The important part of this is that C doesn't have a deleted move constructor, it just doesn't have one period, so trying to move it will cause a copy. That means C's copy constructor is called in A's implicitly generated move constructor since C(std::move(A_obj_to_move_from)) will copy as long as it doesn't have a deleted move constructor.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • 11
    Better to follow the Rule of Zero/Five. Either add all of (copy ctor, move ctor, copy assignment, move assignment, destructor) or add none of them. In this example, none of them are necessary. – 0x5453 Mar 21 '19 at 20:33
  • 3
    @0x5453 Unless this is a parent class and the OP wants the derived classes to get destroyed properly. You need a virtual destructor if you have polymorphism. – NathanOliver Mar 21 '19 at 20:34
  • @NathanOliver You only need a virtual destructor if the base or derived class has unmanaged heap allocations you need to call `delete` or `delete[]` on. – Tzalumen Mar 21 '19 at 20:37
  • @eerorika `std::unique_ptr` is a smart pointer, so it's managed. No `delete` is required, the object it references will be deleted when the `std::unique_ptr` is deleted. – Tzalumen Mar 21 '19 at 20:39
  • Must agree with @0x5453 because the code does not need the `dtor` at all in this case. Removing it allows compiler to create (instead of deleting) the `default move ctor`. If you need one, write all (and will show the ones you wish to `delete` as well) – Everyone Mar 21 '19 at 20:40
  • 3
    @Tzalumen no delete is required (because that's what the unique pointer does for you), but a virtual destructor is required so that the unique pointer won't have UB. – eerorika Mar 21 '19 at 20:40
  • 3
    @Tzalumen If you `delete` an object of class `X` through a pointer to a base class of `X` and that base class doesn't have a virtual dtor, it's Undefined Behaviour. Regardless of what the destructor does. – Angew is no longer proud of SO Mar 21 '19 at 20:41
  • 1
    @eerorika No, it isn't. The default will destruct the `std::unique_ptr` which will destruct the object it references. – Tzalumen Mar 21 '19 at 20:41
  • 3
    @Tzalumen If you have polymorphism, you **must** have a virtual destructor. If you don't the destructor for the derived class won't be called and you have UB. – NathanOliver Mar 21 '19 at 20:41
  • @NathanOliver true, but the code in the question shows no polymorphism. The destructor is not needed, why have it to begin with? If you need it, write it along with the rest of the constructors and operators.. – Everyone Mar 21 '19 at 20:43
  • 2
    @Everyone The OP doesn't need to show us the rest of their code. Let's assume they need one, and give them an answer to why it breaks and how to fix it. You can ask them if they actually need a virtual destructor via a comment on the question. – NathanOliver Mar 21 '19 at 20:45
  • Right. The *derived* classes don't need a `virtual` destructor, but the base class does. My bad. – Tzalumen Mar 21 '19 at 20:46
  • @0x5453 None of them except the move constructor. The point of the question is that it fails to compile because there is no move constructor. The problem is the move constructor was deleted but it is still perfectly valid to move the object even though it **CAN'T** be copied. The rule of 5/3 is good but don't apply it blindly without thinking about the context. Remember `std::unique_ptr` does not implement the rule of 5/3 (so classes that use it are unlikely to implement it either). – Martin York Mar 21 '19 at 20:47
  • @MartinYork True, but the default move constructor should work as expected in this case. And if the user-defined destructor is removed, then the default move constructor won't be suppressed. – 0x5453 Mar 21 '19 at 20:51
  • @0x5453 But in this situation a virtual constructor is needed. – Martin York Mar 21 '19 at 20:52
  • @NathanOliver that isn't the point. If they need the virtual destructor, it's better to implement the other constructors and operators as well. The answer suggests enforcing the default move constructor which fixes the problem. So a better practice would be to implement all or none depending on what you need. \ – Everyone Mar 21 '19 at 20:53
  • 1
    @Everyone Why is it better? If the defaults work, why explicitly say you're using the defaults. The lack of them also states your intent to use the defaults. Really the only problem here is there is no way to get a defaulted virtual destructor without using the inheritance trick. There is a reason a lot of people (including myself) advocate for the rule of zero. – NathanOliver Mar 21 '19 at 20:55
  • @NathanOliver It would be convenient if the presence of a derived class would force the base class constructor to be virtual be default, but that brings up an order of compilation issue. – Tzalumen Mar 21 '19 at 20:58
  • @NathanOliver Regarding explicitly `= default`ing those functions, the [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all) list a few good arguments. Primarily "to avoid unwanted effects" and to "make it more obvious to the reader". Of course the rule of zero is still preferred. – 0x5453 Mar 21 '19 at 21:08
  • 1
    @AndreasDM `vector` is copyable, where `unique_ptr` is not, so it is okay if `tmp` gets copied if it has a `vector` member. – NathanOliver Mar 21 '19 at 21:29
  • I'm not following your debate. Is there a good resource to read up on these defaulting and virtual function rules any of you could recommend? – The Nate Mar 21 '19 at 21:34
  • 1
    @TheNate The [C++ core guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) has good information. So does the FAQ on isocpp.org. What part are you not getting? – NathanOliver Mar 21 '19 at 21:38
  • Thanks. To answer, the inheritance rules not maintaining defaults seems weird to me, but I'm *way* rusty on C++. (preprocessing was part of the C++ I learned on...) – The Nate Mar 21 '19 at 21:44
  • 1
    @TheNate What do you mean by *the inheritance rules not maintaining defaults*? – NathanOliver Mar 21 '19 at 21:46
  • Well, I'll go read up on virtual functions and see if it makes more sense. I was referencing your comment about the need to declare a virtual destructor for it to work. – The Nate Mar 21 '19 at 21:52
  • 1
    @TheNate Ah. Have a read of this: https://stackoverflow.com/questions/461203/when-to-use-virtual-destructors – NathanOliver Mar 21 '19 at 21:54
  • Note that if you do apply the Rule of Five to this class containing a `unique_ptr`, then `A(const A&) = default;` will mean exactly the same thing as `A(const A&) = delete;`, and the same for the copy assignment operator. The `default` version would not need to be changed if the `unique_ptr` member is ever removed or changed to a copyable type, but the `delete` version would be clearer to most human readers. – aschepler Mar 21 '19 at 23:31