4
votes

I apologize for the large amount of code required to demonstrate the issue. I am having a problem using the pimpl idiom with std::unique_ptr. Specifically the problem seems to occur when one class (which has pimpl'ed implementation) is used as member data in another composite class with pimpl'ed implementation.

Most of the answers I've been able to find deal with a lack of explicit destructor declaration, but as you can see here, I have declared and defined the destructors.

What is wrong with this code, and can it be modified to compile without changing the design?

Note: the error seems to occur in the definition of SomeComposite::getValue() and that the compiler cannot see the error until compile time. The error is encountered in memory.h and the message is Invalid application of 'sizeof' to an incomplete type 'pimplproblem::SomeInt::impl'.

SomeInt.h

#pragma once
#include <iostream>
#include <memory>

namespace pimplproblem
{
    class SomeInt
    {

    public:
        explicit SomeInt( int value );
        SomeInt( const SomeInt& other ); // copy
        SomeInt( SomeInt&& other ) = default; // move
        virtual ~SomeInt();
        SomeInt& operator=( const SomeInt& other ); // assign
        SomeInt& operator=( SomeInt&& other ) = default; // move assign
        int getValue() const;

    private:
        class impl;
        std::unique_ptr<impl> myImpl;
    };
}

SomeInt.cpp

#include "SomeInt.h"

namespace pimplproblem
{
    class SomeInt::impl
    {
    public:
        impl( int value )
        :myValue( value )
        {}

        int getValue() const
        {
            return myValue;
        }
    private:
        int myValue;
    };

    SomeInt::SomeInt( int value )
    :myImpl( new impl( value ) )
    {}

    SomeInt::SomeInt( const SomeInt& other )
    :myImpl( new impl( other.getValue() ) )
    {}

    SomeInt::~SomeInt()
    {}

    SomeInt& SomeInt::operator=( const SomeInt& other )
    {
        myImpl = std::unique_ptr<impl>( new impl( other.getValue() ) );
        return *this;
    }

    int SomeInt::getValue() const
    {
        return myImpl->getValue();
    }
}

SomeComposite.h

#pragma once
#include <iostream>
#include <memory>
#include "SomeInt.h"

namespace pimplproblem
{
    class SomeComposite
    {   

    public:
        explicit SomeComposite( const SomeInt& value );
        SomeComposite( const SomeComposite& other ); // copy
        SomeComposite( SomeComposite&& other ) = default; // move
        virtual ~SomeComposite();
        SomeComposite& operator=( const SomeComposite& other ); // assign
        SomeComposite& operator=( SomeComposite&& other ) = default; // move assign
        SomeInt getValue() const;

    private:
        class impl;
        std::unique_ptr<impl> myImpl;
    };
}

SomeComposite.cpp

#include "SomeComposite.h"

namespace pimplproblem
{
    class SomeComposite::impl
    {
    public:
        impl( const SomeInt& value )
        :myValue( value )
        {}

        SomeInt getValue() const
        {
            return myValue;
        }
    private:
        SomeInt myValue;
    };

    SomeComposite::SomeComposite( const SomeInt& value )
    :myImpl( new impl( value ) )
    {}

    SomeComposite::SomeComposite( const SomeComposite& other )
    :myImpl( new impl( other.getValue() ) )
    {}

    SomeComposite::~SomeComposite()
    {}

    SomeComposite& SomeComposite::operator=( const SomeComposite& other )
    {
        myImpl = std::unique_ptr<impl>( new impl( other.getValue() ) );
        return *this;
    }

    SomeInt SomeComposite::getValue() const
    {
        return myImpl->getValue();
    }
} 
1
For cases where this enigmatic error occurs without using = default constructors, my solution was to include an explicit destructor to my class.Matt Eding

1 Answers

7
votes

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.