1

As described in the MSDN library here I wanted to experiment a bit with the pimpl idiom. Right now I have a Foo.hpp with

template<typename T>
class Foo {
public:
    typedef std::shared_ptr<Foo<T>> Ptr;

    Foo();
private:
    class Impl;
    std::unique_ptr<Impl> pImpl;
};

where the T parameter isn't used yet. The implementation is stored in Foo.cpp

template<typename T>
class Foo<T>::Impl {
public:
    int m_TestVar;
};

template<typename T>
Foo<T>::Foo() : pImpl(new Impl) {
    this->pImpl->m_TestVar = 0x3713;
}

Currently the compiler has two errors and one warning:

  • use of undefined type 'Foo<T>::Impl'; ... vc\include\memory in line 1150
  • can't delete an incomplete type; ... vc\include\memory in line 1151
  • deletion of pointer to incomplete type 'Foo<T>::Impl'; no destructor called; ... vc\include\memory in line 1152

What is the concflict here and how could I resolve it?

Edit. Removed the call to std::make_shared - copy&paste fail based on one old version.

Christian Ivicevic
  • 10,071
  • 7
  • 39
  • 74
  • 1. The definitions **must** be in the header file (because of the template). 2. I don't see `Foo::Foo` defined – Kiril Kirov Mar 20 '13 at 12:38
  • @KirilKirov: By moving all definitions in the header file I would forfeit any speed improvements during compilation and all callers of the header would have to be recompiled when changing the Impl just because it is for a templated class (in comparison to the description from the MSDN Library). Should I still use the pimpl idiom to decouple the parts? – Christian Ivicevic Mar 20 '13 at 12:46
  • have a look at this: http://herbsutter.com/gotw/_101/ – André Mar 20 '13 at 13:04
  • @André What aspect of that article are you referring to? – AlwaysLearning Sep 07 '15 at 15:03

2 Answers2

2

I have had a similar issue - we've a base class in our system called NamedComponent and I wanted to create a template which takes an existing named component and converts it into a pimpl facade.

What I did was separate the template into a header and an inline file, and create a function to cause the template to be instantiated. This allows the implementation to be in a library, with the template instantiations of the facade with that implementation, and for the client to be able to use the facade based on the template and a forward declaration of the implementation.

header 'Foo.h':

template<class T> class Foo
{
public:
    Foo ();
    virtual ~Foo();

private:
    T *impl_;

public:
    // forwarding functions
    void DoIt();
};

inline functions 'Foo.inl':

#include "Foo.h"

template<class T> Foo<T>::Foo() :
    impl_ ( new T )
{
}

template<class T> Foo<T>::~Foo()
{
    delete impl_;
}

// forwarding functions
template<class T> void Foo<T>::DoIt()
{
    impl_ -> DoIt();
}    

// force instantiation
template<typename T>
void InstantiateFoo()
{
    Foo<T> foo;
    foo.DoIt();
}

implementation cpp file - include the template inline functions, define the implementation, reference the instantiation function:

#include "Foo.inl"

class ParticularImpl {
public:
    void DoIt() {
        std::cout << __FUNCTION__ << std::endl;
    }
};

void InstantiateParticularFoo() {
    InstantiateFoo<ParticularImpl>();
}

client cpp file - include the template header, forward declare the implementation and use the pimpl facade:

#include "Foo.h"
class ParticularImpl;

int main () {
    Foo<ParticularImpl> bar;

    bar.DoIt();
}

You may have to fiddle with the InstantiateFoo function's contents to force the compiler to instantiate all functions - in my case, the base called all the pimpl's functions in template methods so once one was referenced, they all were. You don't need to call the Instantiate functions, just link to them.

Pete Kirkham
  • 48,893
  • 5
  • 92
  • 171
  • So thank you for your input. I had issues with your instantiations, so I decided to stick to Michael Wilds suggestion to explicitly instantiate this for each of my types like `template class Foo; template class Foo;` in the inline file and it works without your helper methods. – Christian Ivicevic Mar 21 '13 at 14:57
1

IMHO PIMPL doesn't make much sense with templates, unless you know all possible template parameters and that set is fairly small. The problem is, that you will either have the Impl implementation in the header file otherwise, as has been noted in the comments. If the number of possible T parameters is small, you still can go with the separation, but you'll need to declare the specialisations in the header and then explicitly instantiate them in the source file.

Now to the compiler error: unique_ptr<Impl> requires the definition of Impl to be available. You'll need to directly use new and delete in the ctor Foo::Foo and dtor Foo::~Foo, respectively instead and drop the convenience/safety of smart pointers.

Michael Wild
  • 24,977
  • 3
  • 43
  • 43
  • Luckily I would end up having only half a dozen supported specializations where I would like to stick to PIMPL which would make the separation feasible. I will probably need to try a few more approaches until I get a good result. Thanks for reminding me of the explicit specializations. – Christian Ivicevic Mar 20 '13 at 13:03