6

I was trying to use unique_ptr class member with forward declaration. As numerous sources says e.g. Forward declaration with unique_ptr? it should be sufficient to declare non inline destructor, but it seems not to be a case in VS2013 and GCC 5.3.1. I didn't test other compilers.

Example:

#include <memory>

class B;

class A { 
public:
    //A();
    ~A();
private:
    std::unique_ptr<B> b;
};

//class B { };

int main() {
    A a;
}

I can make this code compile only after uncommenting the ctor declaration or the class B declaration. Otherwise on VS2013 I get error

error C2338: can't delete an incomplete type

on GCC error:

In file included from /usr/local/include/c++/5.3.0/memory:81:0,
                 from main.cpp:1:
/usr/local/include/c++/5.3.0/bits/unique_ptr.h: In instantiation of 'void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = B]':
/usr/local/include/c++/5.3.0/bits/unique_ptr.h:236:17:   required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = B; _Dp = std::default_delete<B>]'
main.cpp:5:7:   required from here
/usr/local/include/c++/5.3.0/bits/unique_ptr.h:74:22: error: invalid application of 'sizeof' to incomplete type 'B'
  static_assert(sizeof(_Tp)>0,
                      ^

Why is this?

Community
  • 1
  • 1
pdondziak
  • 253
  • 1
  • 7
  • 1
    Compiles ok for me with VS2015 – wally Mar 16 '16 at 13:42
  • 1
    Seems [ok on GCC](http://coliru.stacked-crooked.com/a/b9593b25976bec62) also. What exactly is the problem? – wally Mar 16 '16 at 13:43
  • @flatmouse thanks for checking other compilers it seems to be already solved compiler bug which version of GCC you used exactly? Mine was gcc (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2) – pdondziak Mar 16 '16 at 13:50
  • 3
    @flatmouse The gcc code you used doesn't actually correspond to OP's code. – Barry Mar 16 '16 at 13:52
  • Can you give all errors generated, including line numbers and warnings? – Yakk - Adam Nevraumont Mar 16 '16 at 13:59
  • 1
    Well of course it doesn’t work: in `main` you are *instantiating* `A`. At that point (and no earlier) do you need a definition of `B`. Uncommenting `class B{};` after `A`’s definition therefore works. I’m not sure why declaring a constructor for `A` is also sufficient. – Konrad Rudolph Mar 16 '16 at 14:02
  • No, you do not need class B definition if instantiating class A. – knivil Mar 16 '16 at 14:03
  • @KonradRudolph No, you don't. You can have a `unique_ptr` to an incomplete type - otherwise you couldn't do pimpl with `unique_ptr`. – Barry Mar 16 '16 at 14:03
  • @Barry I understand that, and I didn’t say differently. However, you cannot *instantiate* a `unique_ptr`. Try it: `class X; int main() { std::unique_ptr{}; }`. There’s no reason why this should work, pimpl notwithstanding. – Konrad Rudolph Mar 16 '16 at 14:07
  • 3
    @Barry Didn't you break his example by converting to single file? Konrad is right about why the current example is failing. The original was failing because the implicitly defined default constructor was attempting to destroy members in case construction threw an exception, so a user defined, out of line definition is required for both ctor and dtor. – Praetorian Mar 16 '16 at 14:07
  • Seems clear to me. `unique_ptr` calls `B`'s destructor, therefore, if you use `A`, you will need a complete `B` type. – zdf Mar 16 '16 at 14:08
  • @Praetorian Oh, right. I didn’t even see the original question. Yes, my explanation fits the current code, not the original. – Konrad Rudolph Mar 16 '16 at 14:09
  • @KonradRudolph That's not the same thing - there `main()` needs to destroy `X`. Here `A::~A()` is the one that's destroying it. – Barry Mar 16 '16 at 14:10
  • @Praetorian I don't believe so. I reduced the example down to what would've been directly included into `main.cpp` as one translation unit. – Barry Mar 16 '16 at 14:10
  • @Praetorian Also `unique_ptr()` is `noexcept`, so what exception? – Barry Mar 16 '16 at 14:12
  • @Barry It’s not exactly the same thing but close enough. `A::~A` is a red herring here: the problem is that `A`’s constructor may throw and need to rewind construction of the member. – Konrad Rudolph Mar 16 '16 at 14:13
  • 1
    @Barry You can't edit the question after I posted the link and then say it doesn't correspond. All I did was take the original code and put it into one file. OP said it doesn't compile when the lines are uncommented, which is what I did and it did compile. – wally Mar 16 '16 at 14:13
  • @flatmouse Your one file included things that the original `main.cpp` did not: the constructor for `A()` and the definition for `B()`. Look again. – Barry Mar 16 '16 at 14:15
  • 1
    @Barry Well, now that's a good point. But look at the standard quote [here](http://stackoverflow.com/q/27336779/241631), there are no such nuances about cases where members are all `noexcept`. That's also a dupe of this question. – Praetorian Mar 16 '16 at 14:15
  • @Praetorian Yep, that looks like the answer here. Good find. – Barry Mar 16 '16 at 14:16
  • @Barry You wrong about the translation unit B.h is only included in A.cpp which means that B class declaration will not be in "main.cpp" translation unit – pdondziak Mar 16 '16 at 14:39
  • @Barry Yes, I took away the essence of OP's question by having everything in one file even if the lines correspond closely. – wally Mar 16 '16 at 14:41
  • 1
    @pdondziak Which is precisely why I have the declaration of `B` commented out. – Barry Mar 16 '16 at 14:42
  • @flatmouse That's... what I said. Your code inadvertently fixed the problem. That's why I said it didn't correspond. – Barry Mar 16 '16 at 14:44
  • @KonradRudolph I think you nailed the problem with answer that ctor of A needs the dtor of B to handle exceptions. But it seems to work before without a ctor as stated in a link I gave in question. – pdondziak Mar 16 '16 at 14:51
  • @Barry You made a valid point. I was trying to agree with you. Good thing I didn't post an answer. I learnt something today. :) – wally Mar 16 '16 at 14:51
  • @flatmouse Me too :) – Barry Mar 16 '16 at 14:53
  • @Praetorian - Could you kindly write a formal answer for others do not question this line of comments? – SChepurin Mar 17 '16 at 12:14

1 Answers1

2

The destructor of class A must know the definition of class B. A forward declaration of class B is okay as longs as the implementation file of the constructor/destructor of A knows the definition of class B. If your implementation is (implicitly) in a header file then you need a definition of B in your header file. You may study Pimpl from Herb Sutter.

knivil
  • 787
  • 3
  • 11
  • Your answer is fine. The source of confusion might be the error message: it does not say `destructor`. – zdf Mar 16 '16 at 14:15
  • This isn't the issue. The destructor of `A` isn't even defined, just declared, so it isn't the problem. And you do not need a definition of `B` in the header file at all - that's the point of pimpl. – Barry Mar 16 '16 at 14:18
  • "header" is guarded by an if. – knivil Mar 16 '16 at 14:49