unique_ptr, pimpl/forward declaration and complete definition

Question

I already checked out the questions here and here, but still cannot figure out what is wrong.

This is the calling code:

#include "lib.h"

using namespace lib;

int
main(const int argc, const char *argv[]) 
{
    return 0;
}

This is the lib code:

#ifndef lib_h
#define lib_h

#include <string>
#include <vector>
#include <memory>

namespace lib
{

class Foo_impl;

class Foo
{
    public:
        Foo();
        ~Foo();

    private:
        Foo(const Foo&);
        Foo& operator=(const Foo&);

        std::unique_ptr<Foo_impl> m_impl = nullptr;

        friend class Foo_impl;
};

} // namespace

#endif

clang++ gives me this error:

invalid application of 'sizeof' to an incomplete type 'lib::Foo_impl'
note: in instantiation of member function 'std::default_delete::operator()' requested

You can see I already specifically declared Foo destructor. What else am I missing here?

Answer 1

The implementation of Foo_impl must be complete prior to the instantiation required in std::unique_ptr<Foo_impl> m_impl = nullptr.

Leaving the type declared (but not initialised) will fix the error (std::unique_ptr<Foo_impl> m_impl;), you would then need to initialise it later on in the code.

The error you are seeing is from the implementation of a technique used to test for this; the incomplete type. Basically, sizeof will result in an error with types that are only forward declared (i.e. lack definition when used at that point in the code/compilation).

A possible fix here would look like;

class Foo_impl;

class Foo
{
  // redacted
  public:
    Foo();
    ~Foo();

  private:
    Foo(const Foo&);
    Foo& operator=(const Foo&);

    std::unique_ptr<Foo_impl> m_impl;// = nullptr;
};

class Foo_impl {
  // ...
};

Foo::Foo() : m_impl(nullptr)
{
}

Why is the complete type required?

The instantiation via = nullptr uses copy initialisation and requires the constructor and destructor to be declared (for unique_ptr<Foo_impl>). The destructor requires the deleter function of the unique_ptr which, by default, calls delete on the pointer to Foo_impl hence it requires the destructor of Foo_impl, and the destructor of Foo_impl is not declared in the incomplete type (the compiler doesn't know what it looks like). See Howard's answer on this as well.

Key here is that calling delete on an incomplete type results in undefined behaviour (§ 5.3.5/5) and hence is explicitly checked for in the implementation of unique_ptr.

Another alternative for this situation may be to use direct initialisation as follows;

std::unique_ptr<Foo_impl> m_impl { nullptr };

There seems to be some debate on the non-static data member initialiser (NSDMI) and whether this is a context that requires the member definition to exist, at least for clang (and possibly gcc), this seems to be such a context.

Answer 2

The statement:

std::unique_ptr<Foo_impl> m_impl = nullptr;

invokes copy-initialization. This has the same semantics as:

std::unique_ptr<Foo_impl> m_impl = std::unique_ptr<Foo_impl>(nullptr);

I.e. it constructs a temporary prvalue. This temporary prvalue must be destructed. And that destructor needs to see the complete type of Foo_impl. Even if the prvalue and move construction is elided, the compiler must behave "as if".

You can instead use direct-initialization, and the unique_ptr destructor will no longer be required at this point:

std::unique_ptr<Foo_impl> m_impl{nullptr};

Update

Casey points out that gcc-4.9 currently instantiates ~unique_ptr() even for the direct-initialization form. However in my tests clang does not. I do not know what other compilers may do. I believe that clang is conforming in this regard, at least with the most recent core defect reports factored in.