4

From Is std::unique_ptr required to know the full definition of T?, I know that if a class A has a member unique_ptr<T>, then T shall be a complete type in destructor ~A(). However, I came across a situation that the constructor A() also requires complete type of T, see code below:

// a.h -------------------------------
#pragma once
#include <memory>
struct B;
struct A {
  A(); // <---
  ~A();
  std::unique_ptr<B> ptr;
};

// a.cpp -------------------------------
#include "a.h"
struct B {};
A::A() = default; // <---
A::~A() = default;

// main.cpp -------------------------------
#include "a.h"
int main() {A a;}

If the definition of constructor A::A() is moved to the header a.h, the compiler will complain error: invalid application of ‘sizeof’ to incomplete type ‘B’. Why is this happening? Is there any reference material about this?

BTW, I'm using gcc-7.5.0 on Ubuntu 18.04, with c++17 enabled.


Edit for @463035818_is_not_a_number in the comments. The complete error message is:

[1/2] Building CXX object CMakeFiles/t.dir/main.cpp.o
FAILED: CMakeFiles/t.dir/main.cpp.o 
/usr/bin/c++   -g -fdiagnostics-color=always -std=gnu++1z -MD -MT CMakeFiles/t.dir/main.cpp.o -MF CMakeFiles/t.dir/main.cpp.o.d -o CMakeFiles/t.dir/main.cpp.o -c /home/user/Tests/UniquePtrTest/main.cpp
In file included from /usr/include/c++/7/memory:80:0,
                 from /home/user/Tests/UniquePtrTest/a.h:2,
                 from /home/user/Tests/UniquePtrTest/main.cpp:1:
/usr/include/c++/7/bits/unique_ptr.h: In instantiation of ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = B]’:
/usr/include/c++/7/bits/unique_ptr.h:263:17:   required from ‘std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = B; _Dp = std::default_delete<B>]’
/home/user/Tests/UniquePtrTest/a.h:5:3:   required from here
/usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘B’
  static_assert(sizeof(_Tp)>0,
                      ^
ninja: build stopped: subcommand failed.
VictorBian
  • 675
  • 1
  • 4
  • 17
  • 1
    The constructor needs to know how to destruct all the fields, in case at any moment during the construction an exception is thrown. – Matteo Italia Feb 10 '23 at 08:43
  • @MatteoItalia seems like your explanation should apply to using the same A() = default in the class declaration (in the h file). But the compiler issues no error if you do it like that. – wohlstad Feb 10 '23 at 08:48
  • 1
    @wohlstad - Compile as C++20, then you won't be dealing with silly aggregate. – StoryTeller - Unslander Monica Feb 10 '23 at 08:48
  • 2
    btw `#include` is basically just replacing text, hence it should be possible to reproduce it with code in a single file. Would make it simpler to reproduce for others – 463035818_is_not_an_ai Feb 10 '23 at 08:50
  • @StoryTeller-Unslander Monica I don't follow. I was referring to defining the `A` ctor as `=default`. This works in the class declaration (`A() = default;`) but not outside as `A::A() = default;`. In both cases I removed the line `struct B {};`. – wohlstad Feb 10 '23 at 08:51
  • @wohlstad - What's to follow in "compile as C++20"? – StoryTeller - Unslander Monica Feb 10 '23 at 08:52
  • @StoryTeller-UnslanderMonica you mentioned _"silly aggregate"_. I didn't understand how it is related. And BTW - I am compiling as C++20. – wohlstad Feb 10 '23 at 08:55
  • @wohlstad - Interesting. `struct A { A() = default; };` is an aggregate pre c++20, and not later. That *would* affect there not actually being a c'tor body to error out on. I imagine your testing has something else in the mix. – StoryTeller - Unslander Monica Feb 10 '23 at 08:59
  • Now I understand your mention of _aggregate_. However - my test is minimal - similar to the OP's code, and I get the same behavior when I compile both as C++17 and as C++20. I am using VS2022, version 17.2.5. – wohlstad Feb 10 '23 at 09:04
  • @463035818_is_not_a_number Sorry, I cannot reproduce it with code in a single file. Could you provide an example? – VictorBian Feb 10 '23 at 09:09
  • 1
    See also https://stackoverflow.com/questions/71821115/incomplete-types-with-shared-ptr-and-unique-ptr – Passer By Feb 10 '23 at 09:11
  • I also didnt manage to reproduce. I admit I don't understand, must have something to do with `A a;` being in a different translation unit, I guess if you remove that from `main` the error is also gone – 463035818_is_not_an_ai Feb 10 '23 at 09:18
  • @463035818_is_not_a_number Yes, different translation unit matters. – VictorBian Feb 10 '23 at 09:27
  • hm well, then I'd ask you to include the complete error message in the question. It seems like the error is from main, but thats not clear from the question. – 463035818_is_not_an_ai Feb 10 '23 at 09:29
  • @StoryTeller-UnslanderMonica I posted this question regarding the issue I menioned above: https://stackoverflow.com/questions/75409177/difference-between-constructor-definitoins-of-a-class-holding-a-stdunique-ptr. – wohlstad Feb 10 '23 at 09:33

2 Answers2

9

The issue is that A::A() needs to know how to destroy ptr in case the constructor throws.

An example:

#include <memory>
struct B {};

struct X{
    X(){throw 42;}
};

struct A {
  A() {}
  ~A() {};
  std::unique_ptr<B> ptr;
  X x;
};


int main() {
    A a;
}

generates:

A::A() [base object constructor]:
        push    rbp
        mov     rbp, rsp
        push    rbx
        sub     rsp, 24
        mov     QWORD PTR [rbp-24], rdi
        mov     rax, QWORD PTR [rbp-24]
        mov     rdi, rax
        call    std::unique_ptr<B, std::default_delete<B> >::unique_ptr<std::default_delete<B>, void>()
        mov     rax, QWORD PTR [rbp-24]
        add     rax, 8
        mov     rdi, rax
        call    X::X() [complete object constructor]
        jmp     .L6
        mov     rbx, rax
        mov     rax, QWORD PTR [rbp-24]
        mov     rdi, rax
        call    std::unique_ptr<B, std::default_delete<B> >::~unique_ptr() [complete object destructor]
        mov     rax, rbx
        mov     rdi, rax
        call    _Unwind_Resume

showing the call to std::unique_ptr<B, std::default_delete<B> >::~unique_ptr().

Is there any reference material about this?

Technically, yes, you can read the Standard which defines which functions/expressions require a complete type.

Practically, not so much, since you have to read the Standard which defines which functions/expressions require a complete type.

Of course cppreference is of high quality and actually readable, although I did not find this use case there.

In particular, this issue is mentioned in a note 20.11.1.3.3 Destructor [unique.ptr.single.dtor]

[Note 1 : The use of default_delete requires T to be a complete type. — end note]

Quimby
  • 17,735
  • 4
  • 35
  • 55
  • 1
    Thanks. It seems the problem about my case is that: If `A::A()` is defined in `a.h`, the ctor won't be compiled in `a.o`. Then `main.o` compiles `A::A()` from the header, while `B` is a incomplete type here. – VictorBian Feb 10 '23 at 10:06
  • @VictorBian Yes, exactly because then the definition is inline inside the header with the incomplete `B`. Sorry, should have said that explicitly. Had you had `main` inside `a.cpp`, it would have worked I think. – Quimby Feb 10 '23 at 10:15
3

The error you see is std::default_deleter guarding against undefined behaviour for you.

When you instantiate the definition of the constructor std::unique_ptr<B>::unique_ptr, the definition of std::default_delete<B>::operator() is also instantiated. Within which is an assertion

static_assert(sizeof(B) > 0);

which checks for incomplete types. This prevents any possible deletion of an incomplete type, which is undefined behaviour. See also incomplete types with shared_ptr and unique_ptr.

But why does moving the definition of A::A() to the header cause an error but not if it's in the implementation file?

As it turns out, simply declaring the member std::unique_ptr<B> only instantiates the declaration of its constructor but not the definition. Therefore if A::A() is defined in the implementation file, the definition of std::default_delete<B>::operator() is also only instantiated then, by which B is a complete type.

Passer By
  • 19,325
  • 6
  • 49
  • 96