1

Related?: Why is the destructor called for an object that is not deleted?

As a minimal case, I have a templated heap-allocated move-only value class template. What I don't understand is why a function that just uses its noexcept move-constructor seems to want to use its d'tor:

#include <cassert>
#include <memory>

// Roughly a unique_ptr:
template <typename T>
class move_only_heap_value {
    T* ptr;
public:
    move_only_heap_value() : ptr(new T()) {}
    move_only_heap_value(move_only_heap_value&& x) noexcept : ptr(x.ptr) { x.ptr = nullptr; }
    // ...
    T& operator*() { assert(ptr); return *ptr; }
    const T& operator*() const { assert(ptr); return *ptr; }
    // ...

    ~move_only_heap_value() {
        if (ptr) {
            delete ptr;
        }
    }
};

//struct S {    S();    ~S();  }; // I don't see ~S() called anywhere.
struct S;

move_only_heap_value<S> foo(move_only_heap_value<S>&& x) {
    return std::move(x); // Error here due to missing ~move_only_heap_value<S>()
}

produces

<source>: In instantiation of 'move_only_heap_value<T>::~move_only_heap_value() [with T = S]':
<source>:27:21:   required from here
<source>:18:13: error: possible problem detected in invocation of 'operator delete' [-Werror=delete-incomplete]
   18 |             delete ptr;
      |             ^~~~~~~~~~
<source>:18:20: error: invalid use of incomplete type 'struct S' [-Werror]
   18 |             delete ptr;
      |                    ^~~
<source>:24:8: note: forward declaration of 'struct S'
   24 | struct S;
      |        ^
<source>:18:13: note: neither the destructor nor the class-specific 'operator delete' will be called, even if they are declared when the class is defined
   18 |             delete ptr;
      |             ^~~~~~~~~~
cc1plus: all warnings being treated as errors
ASM generation compiler returned: 1
<source>: In instantiation of 'move_only_heap_value<T>::~move_only_heap_value() [with T = S]':
<source>:27:21:   required from here
<source>:18:13: error: possible problem detected in invocation of 'operator delete' [-Werror=delete-incomplete]
   18 |             delete ptr;
      |             ^~~~~~~~~~
<source>:18:20: error: invalid use of incomplete type 'struct S' [-Werror]
   18 |             delete ptr;
      |                    ^~~
<source>:24:8: note: forward declaration of 'struct S'
   24 | struct S;
      |        ^
<source>:18:13: note: neither the destructor nor the class-specific 'operator delete' will be called, even if they are declared when the class is defined
   18 |             delete ptr;
      |             ^~~~~~~~~~
cc1plus: all warnings being treated as errors
Execution build compiler returned: 1

https://godbolt.org/z/KoMrcM1n4

I understand why this translation unit can't call ~move_only_heap_value<S>() (because S is incomplete), but what possess it to want to call that? If I instead define S but leave its c'tor and d'tor undefined, I get a link error as expected but I don't see the d'tor called in the assembly, so why does it think it needs to instantiate it?

A simpler case, but possibly subtly different:

#include <utility>

struct Indestructible {
    Indestructible() = default;
    Indestructible(Indestructible&& x) noexcept = default;
    ~Indestructible() = delete;
};

Indestructible foo(Indestructible&& x) {
    return std::move(x);
}

produces

<source>: In function 'Indestructible foo(Indestructible&&)':
<source>:10:21: error: use of deleted function 'Indestructible::~Indestructible()'
   10 |     return std::move(x);

https://godbolt.org/z/MbKhqddxd but I worry that's different since maybe the returned result has automatic storage duration and so has to be destructed at some sequence point in the future but is not destructible, whereas the original version can be destructed, just not in this translation unit.

Ben
  • 9,184
  • 1
  • 43
  • 56
  • 1
    "move" doesn't actually move any object. It constructs a **new separate object** by stealing some fraction (possibly all, possibly none) of the internal resources owned by the source object. The source object is left alive and its destructor must be run at its death. – Ben Voigt Apr 07 '22 at 14:49
  • 1
    dont use `return std::move(x);` you undermine the compiler's RVO – Taekahn Apr 07 '22 at 14:50
  • 1
    I expanded the abbreviation "d'tor" in the title so that searches can find this question more easily. – Nate Eldredge Apr 07 '22 at 14:52
  • "I don't see the destructor called in the assembly"... because the compiler is smart enough to figure out that `ptr` is always null in the moved-from object being destructed.... but **the removal of dead code happens after access checking** for all function calls in the removed branch. – Ben Voigt Apr 07 '22 at 14:54
  • 1
    @BenVoigt but isn't the passed-in `move_only_heap_value&& x` by (rvalue) reference and so it's moved-from self lives on past the function call for the caller to deal with destructing? – Ben Apr 07 '22 at 15:01
  • The C++ standard specifies when various types of objects cease to exist (e.g. an object of automatic storage duration ceases to exist at the end of its enclosing scope, an object created using a `new` expression ceases to exist during the corresponding `delete` expression). Part of the process of an object ceasing to exist is calling its destructor. The implementation (i.e. the compiler) takes care of ensuring the destructor is called when needed. The programmer is not required to take any explicit action to call the destructor, so the calling of the constructor is said to occur implicitly. – Peter Apr 07 '22 at 15:04
  • @Ben: It is the return value's temporary object that is the issue, not the parameter. – Ben Voigt Apr 07 '22 at 15:09
  • @Taekahn the argument is passed by rvalue reference so it isn't subject to RVO. I could take it by value and just return, but then the local copy *would* have to be destructed at the end of the function. – Ben Apr 07 '22 at 15:09
  • @BenVoigt I was under the impression (at the edge of my understanding) that the calling convention is for the function to construct the result in the right place and for the callee to be responsible for destructing it. Are you saying this isn't doing RVO and instead making a temporary that the caller would have to move-construct into the result location but that this function is responsible for destructing the temporary? – Ben Apr 07 '22 at 15:13
  • @Ben [You *just* commented that RVO doesn't apply here](https://stackoverflow.com/questions/71784284/why-is-the-destructor-implicitly-called?noredirect=1#comment126856892_71784284)... Anyway, optimizations, RVO and NRVO included, occur after access checks for constructor and destructor. In C++17, mandatory copy elision avoids those access checks, but mandatory copy elision is separate from RVO. – Ben Voigt Apr 07 '22 at 15:14
  • @BenVoigt I'm saying that it's my understanding that `S foo(S&& x) { return x; }` in general doesn't do RVO in the sense that the `x` gets copied. But my understanding (maybe wrong!) is that calling `S foo(S&& x) { return std::move(x); }` is guaranteed to create exactly one move-constructed `S` at the call site, as opposed to a temporary. So `S y = foo(std::move(x));` would result in `foo` directly move-constructing `y`. That's what I meant by RVO in my later comment. I know the compiler can do copy elision, but this at least agrees with my understanding: https://godbolt.org/z/41Kjjn5rT – Ben Apr 07 '22 at 15:30
  • @BenVoigt I guess I was conflating mandatory copy elision with RVO. Are you saying it's a Catch-22, where it wants to be able to destruct the result because the standard says so, but it wouldn't have to (in C++17) if mandatory copy elision took hold, but for that to happen I'd have to return a local value but that local value would then have to be destructed? – Ben Apr 07 '22 at 15:37
  • @ben https://godbolt.org/z/Mv14eoKxv – Taekahn Apr 07 '22 at 15:40
  • @Taekahn that's Clang. I know Clang doesn't mind. But GCC and MSVC do: https://godbolt.org/z/a16Mcb1vT https://godbolt.org/z/zqhrhfs5Y – Ben Apr 07 '22 at 16:01
  • Those two links wouldn't compile even with `std::move` since you deleted the destructor. but these compile, gcc and msvc respectively. https://godbolt.org/z/fx4EaGfTq https://godbolt.org/z/fv1eGoGoq – Taekahn Apr 07 '22 at 16:10
  • OK, but if I make things depend on an incomplete type, it still wants to be able to have a destructor in the function that feels like it should just need to move-construct: https://godbolt.org/z/Y5PoxacTo (Clang doesn't mind: https://godbolt.org/z/8sfjbrPs5) – Ben Apr 07 '22 at 16:16
  • 1
    @Ben: `return std::move(x);` makes your life worse. If `x` is an x-value, then simply `return x;` will be a candidate for mandatory copy elision, and if it doesn't fall into a mandatory elision case, you still get a move. `return std::move(x);` prevents copy elision (mandatory or permissive) and always semantically performs a move to a return temporary (which the compiler may then optimize away under the as-if rule, after performing access checks on the move constructor and destructor). – Ben Voigt Apr 07 '22 at 16:47
  • What I still don't get is: Why does move-constructing a return value require the d'tor when the argument is by (rvalue) reference? Isn't the only thing the function has to do move-constructing the result, and if that throws (although it's `noexcept`) then result doesn't exist yet and so doesn't need to be destructed? – Ben Apr 07 '22 at 17:00
  • OK, here's the truly minimal example: https://godbolt.org/z/Wx63abYTh `struct X; std::unique_ptr foo() { return nullptr; }`. In my mind, that is essentially the same as `void constructAt(std::unique_ptr* ptr) { new (&ptr) std::unique_ptr{nullptr}; }`, which it's fine with. Why does returning by value mean it needs to create the destructor? – Ben Apr 07 '22 at 17:07
  • OK, I refined that question here: https://stackoverflow.com/questions/71787027/requirements-on-returned-type-that-may-have-some-member-functions-sfinaed-away – Ben Apr 07 '22 at 18:09

1 Answers1

3

but what possess it to want to call that?

You delete a pointer to an object of that type here.

~move_only_heap_value() {
    if (ptr) {
        delete ptr;
//      ^^^^^^^^^^
    }
}

That invokes the destructor, and the type must be defined at that point. ~move_only_heap_value of the template in turn has been instantiated because you have defined a function that returns an object of that template instance.

You can remove the definition of this destructor, and it will compile. You can define it elsewhere in a place where S is defined.

but I don't see the d'tor called in the assembly

Just because something won't end up in the assembly, doesn't necessarily mean that it won't need to be well-defined.


P.S. if (ptr) delete ptr; can always be simplified into delete ptr; The condition is redundant.

eerorika
  • 232,697
  • 12
  • 197
  • 326
  • 1
    _"can always be simplified into `delete ptr;`"_ That's only true if you implement the move constructor/assignment correctly, right? Else you can get double delete. – JHBonarius Apr 07 '22 at 14:58
  • 1
    @JHBonarius `if (ptr)` doesn't prevent a double delete. – eerorika Apr 07 '22 at 14:59
  • Oh yeah! So bottom line, it's essential you need to implement the copy/move constructor correctly, and set the `ptr` to `nullptr`. – JHBonarius Apr 07 '22 at 15:01
  • 1
    @JHBonarius Indeed. If you have a custom destructor, then you probably need to define (or delete, at least implicitly such as in the example) copy/move assignment/destructor i.e. follow the rule of 5. And my point is that `if (ptr)` neither solves the problem that you have by not following rule of 5, and it isn't needed if you do follow rule of 5. That's because it does nothing. – eerorika Apr 07 '22 at 15:03
  • @JHBonarius For reference : [Rule of 3/5/0](https://en.cppreference.com/w/cpp/language/rule_of_three) – François Andrieux Apr 07 '22 at 15:14
  • I was skipping the rule of 3/5/0 for brevity for the example. I can `=delete` the copy-c'tor and the two `operator=`s and get the same error. – Ben Apr 07 '22 at 15:16
  • 2
    @Ben Rule of 5 is only relevant to JHBonarius' counter-argument regarding my sidenote about `if (ptr)` being redundant. It's not solution to the problem in your question. The solutions are to define `S` before defining `~move_only_heap_value()`, or define `S` before defining `foo`. – eerorika Apr 07 '22 at 15:18