3

This is not a dupe of std::unique_ptr with an incomplete type won't compile.

Consider the code below:

#include <memory>

struct X
{
    X();
    ~X();    

    struct Impl; 
    std::unique_ptr<Impl> up_;
};

struct Impl {}; // fully visible here

X::X() : up_{nullptr}{}
X::~X() = default;

int main()
{
    X x;
}

Live on Coliru

gcc/clang both spit an error saying that Impl is incomplete. However, I provide a default destructor for X after Impl is fully visible, so IMO the code should compile. Why doesn't? Now comes the surprise: If I make Impl an inner class, i.e. define

struct X::Impl{};

instead, then the code compiles, even without providing a destructor. Why is this happening? Shouldn't we provide such a default destructor, at least according to the link mentioned in the first line?

Community
  • 1
  • 1
vsoftco
  • 55,410
  • 12
  • 139
  • 252
  • 2
    Where you say "// fully visible here"; this is a different class from the one you declared in `struct X`. Try moving `struct Impl;` before `struct X`. – Richard Critten Jan 28 '16 at 00:46
  • @RichardCritten I think the structure should be fully visible in the destructor of `X`, which is defined after. At least that was my understanding of implementing PIMPL with `unique_ptr`. – vsoftco Jan 28 '16 at 00:55
  • 1
    In the code as originally written, there are two distinct unrelated things named `Impl`. You have `X::Impl` that's declared but never defined - that's the one `up_` uses. Then there's `::Impl` that's defined but never used. When you write `struct X::Impl{};` instead, you now have a single type named `X::Impl` that's declared and defined. – Igor Tandetnik Jan 28 '16 at 01:05
  • 1
    Split the code up into multiple cpp files as you normally would, and it [fails to compile](http://coliru.stacked-crooked.com/a/f002dc469e859f10). If you uncomment the lines in the command edit box in my example, it'll compile. I think your second example above compiles because the end of the translation unit is a valid point of instantiation, so the `unique_ptr` destructor is not instantiated until then (not sure about this part). – Praetorian Jan 28 '16 at 01:05
  • @IgorTandetnik Ohhh I see, I though `struct Impl;` is a forward declaration and will be considered as outside the class, but is treated by the compiler as `X::Impl`. You are right, if I move e.g. `struct Impl;` above `struct X{}` then it works. – vsoftco Jan 28 '16 at 01:06
  • @Praetorian I believe you're right, I haven't tried splitting in multiple cpp/h files, although I clearly remember that we **need** to provide the destructor after in that case. Feel free to post an answer. – vsoftco Jan 28 '16 at 01:09

1 Answers1

12

You have two different structs named Impl.

struct X
{
    struct Impl; // Here you declare X::Impl
    std::unique_ptr<Impl> up_;  // Here you create a pointer to a X::Impl
};

struct Impl {};  // Here you declare and define ::Impl

...

int main()
{
    X x;  // Here X::Impl is still incomplete
}
Miles Budnek
  • 28,216
  • 2
  • 35
  • 52