Does the Gotw #101 "Solution" Actually Solve Anything

Does the GotW #101 solution actually solve anything?

You are correct; the example seems to be missing an explicit template instantiation. When I try to run the example with a constructor and destructor for widget::impl on MSVC 2010 SP1, I get a linker error for pimpl<widget::impl>::pimpl() and pimpl<widget::impl>::~pimpl(). When I add template class pimpl<widget::impl>;, it links fine.

In other words, GotW #101 eliminates all boilerplate from GotW #100, but you need to add an explicit instantiation of the pimpl<...> template with the implementation of the pimpl impl. At least with #101 the boiler plate you need is straightforward.

Is there a way to combine the benefits of compiler firewalls (Pimpl) and default-copyability?

I think the best solution here is to build your own deep-copying smart pointer. If you tailor it to storing Pimpls only, it shouldn't be too difficult:

template <class P>
class pimpl_ptr
{
  std::unique_ptr<P> p_;

public:
  ~pimpl_ptr() = default;

  pimpl_ptr(const pimpl_ptr &src) : p_(new P(*src.p_)) {}
  pimpl_ptr(pimpl_ptr &&) = default;
  pimpl_ptr& operator= (const pimpl_ptr &src) { p_.reset(new P(*src.p_)); return *this; }
  pimpl_ptr& operator= (pimpl_ptr &&) = default;

  pimpl_ptr() = default;
  pimpl_ptr(P *p) : p_(p) {}

  P& operator* () const { return *p_; }
  P* operator-> () const { return &**this; }

  // other ctors and functions as deemed appropriate
};

Just document that it doesn't support pointing to base class subobjects, and you're set. You could enforce this by not giving it a constructor taking a pointer, and enforcing construction through make_pimpl:

template <class P>
class pimpl_ptr
{
  // as above, P* ctor is private
private:
  pimpl_ptr(P *p) : p_(p) {}

  template <class T, class... Arg>
  friend pimpl_ptr<T> make_pimpl(Arg&&... arg);
};

template <class T, class... Arg>
pimpl_ptr<T> make_pimpl(Arg&&... arg)
{
  return pimpl_ptr<T>(new T(std::forward<Arg>(arg)...));
}

How to safely destruct class with smart pointer to incomplete object type?

I don't think your code should compile (and it doesn't in gcc)

std::unique_ptr<Cow> uses std::default_delete<Cow>, and std::default_delete<Cow>::operator() should fail to instantiate for an incomplete type Cow.

See also Is it true that a unique_ptr declaration, unlike a auto_ptr declaration, is well-defined when its template type is of an incomplete type?

So you're right: you need to ensure that default_delete<Cow>::operator() is instantiated somewhere that the Cow type is complete. Which means the destructor of Farm needs to be defined in such a place.

I've just noticed that your subject says "smart pointer", while the question specifies unique_ptr. The answer would be different for a shared_ptr, since std::shared_ptr<Cow>::reset() is a function template that captures the (static) type of the pointer passed to it and stores a deleter. So with shared_ptr all you need is that the call to reset has the complete type Cow -- the location of the destructor doesn't matter.

C++ Pimpl Idiom Incomplete Type using std::unique_ptr

You can't use defaulted constructors and assignment operators (such as SomeInt( SomeInt&& other ) = default;) declared in header file with Pimpl classes, because the default implementations are inline, and at the point of declaration SomeInt's declaration SomeInt::impl is incomplete, so unique_ptr complains. You have to declare and define out of line (that is, in implementation file) all special member functions yourself.

That is, change SomeInt and SomeComposite declarations as follows:

// SomeInt.h
SomeInt( SomeInt&& other ); // move
SomeInt& operator=( SomeInt&& other ); // move assign

// SomeInt.cpp
// after definition of SomeInt::impl
SomeInt::SomeInt( SomeInt&& other ) = default;
SomeInt& operator=( SomeInt&& other ) = default;

Another option is to create your own Pimpl pointer, as suggested in this answer.

unique_ptr, pimpl/forward declaration and complete definition

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.

Why construction of std::unique_ptr for incomplete type compiles?

This works because the point of instantiation of a template is located after the definition of Data. From the Standard:

[temp.point]

For a class template specialization, a class member template specialization, or a specialization for a class member of a class template, if the specialization is implicitly instantiated because it is referenced from within another template specialization, if the context from which the specialization is referenced depends on a template parameter, and if the specialization is not instantiated previous to the instantiation of the enclosing template, the point of instantiation is immediately before the point of instantiation of the enclosing template. Otherwise, the point of instantiation for such a specialization immediately precedes the namespace scope declaration or definition that refers to the specialization.

A specialization for a function template, a member function template, or of a member function or static data member of a class template may have multiple points of instantiations within a translation unit, and in addition to the points of instantiation described above, for any such specialization that has a point of instantiation within the translation unit, the end of the translation unit is also considered a point of instantiation. A specialization for a class template has at most one point of instantiation within a translation unit. A specialization for any template may have points of instantiation in multiple translation units. If two different points of instantiation give a template specialization different meanings according to the one-definition rule, the program is ill-formed, no diagnostic required.

Note that this is probably ill-formed (NDR) due to the last sentence in the quote. I am not confident enough to tell whether this is certainly ill-formed or not.

How do I use unique_ptr for pimpl?

I believe that your test_help.cpp actually sees the ~Help() destructor that you declared default. In that destructor, the compiler tries to generate the unique_ptr destructor, too, but it needs the Impl declaration for that.

So if you move the destructor definition to the Help.cpp, this problem should be gone.

-- EDIT --
You can define the destructor to be default in the cpp file, too:

Help::~Help() = default;

---

Related Topics