6
votes

The code would work file as long as I don't move the definition of constructor (of B) to the header B.h.

B.h

class Imp;  //<--- error here
class B{
    public:
    std::unique_ptr<Imp> imp;
    B();     //<--- move definition to here will compile error
    ~B();
    //// .... other functions ....
};

B.cpp

#include "B.h"
#include "Imp.h"
B::B(){ }
~B::B(){ }

Imp.h

class Imp{};

Main.cpp (compile me)

#include "B.h"

Error: deletion of pointer to incomplete type
Error: use of undefined type 'Imp' C2027

I can somehow understand that the destructor must be moved to .cpp, because destructure of Imp might be called :-

delete pointer-of-Imp;  //something like this

However, I don't understand why the rule also covers constructor (question).

I have read :-

2
It's the destructor that matters.Kerrek SB
@Kerrek SB the constructor does not matters?javaLover
Can't reproduceWhiZTiM
@WhiZTiM I believe it has to be in different files. Your test case have definition of "Imp", so there will be no error. In my test case, "Main.cpp" doesn't include "Imp.h".javaLover
@Dan I can't, because my objective is to use Pimpl-idiom. ... or I misunderstand what you stated?javaLover

2 Answers

8
votes

The constructor needs to destroy the class members, in the case that it exits by exception.

I don't think that making the constructor noexcept would help, though maybe it should.

-2
votes

Preprocessed out of b.cpp can be generated by following command,
g++ -E b.cpp >> b.preprocessed
And it is as follows,

# 1 "b.cpp"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "b.cpp"
# 1 "b.h" 1

class Imp;

class B
{
    std::unique_ptr< Imp> imp;
public:
    B(){}
    ~B();
};
# 2 "b.cpp" 2
# 1 "imp.h" 1

class Imp
{
};
# 3 "b.cpp" 2

B::~B(){}

It is clearly visible here, declaration of class Imp comes after constructor.
So how constructor can create something which doesn't exist for it? (Only forward declaration is not enough) and It makes clear that constructor definition must be in b.cpp file so that it will come after class Imp declaration and it becomes complete type.
Other point is, i don't think it is the correct way of using pimple-idiom. Implementation class should be declared and defined in source file which is not accessible from outside rather then keeping it in separate header file.