Pimpl not working

Question

This is a very noobish mistake, but I dont know whats happening here.

There are loads of pimpl examples but I dont understand why this isn't working (this was one of the examples more or less but I dont see the difference).

I have a very simple Pimpl example, but it wont work.

// Foo.hpp
#include <boost/scoped_ptr.hpp>

class Foo
{
 struct Bar;
 //boost::scoped_ptr<Bar> pImpl;
 Bar* pImpl;

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

 int returnValue();

private:

};

and

// Foo.cpp
#include "foo.hpp"

struct Foo::Bar
{ 
 Bar() {}
 ~Bar() {}
 int value;
};

Foo::Foo() : pImpl(new Bar())
{
 pImpl->value = 7;
}

int Foo::returnValue() {
 return *pImpl->value;
}

Compiling this gives me the error. C2100: illegal indirection.

Thanks.

Answer 1

int returnValue() should be a member function:

//  vvvvv
int Foo::returnValue() {
 return pImpl->value; // no need to dereference, value isn't a pointer
}

---

You need to define your constructor, copy-constructor, copy assignment operator, and destructor after the implementation class has been defined. (Otherwise the implicit destructor is dangerous, and scoped_ptr won't let you do that):

// Foo.hpp
#include <boost/scoped_ptr.hpp>

class Foo
{
    struct Bar;
    boost::scoped_ptr<Bar> pImpl;

public:
    Foo();
    ~Foo();

    int returnValue(); // could be const (so should be)

private:
    // just disable copying, like scoped_ptr
    Foo(const Foo&); // not defined
    Foo& operator=(const Foo&); // not defined
};

And:

// Foo.cpp
#include "foo.hpp"

struct Foo::Bar
{ 
    int value;
};

Foo::Foo() :
pImpl(new Bar())
{
    pImpl->value = 7;
}

Foo::~Foo()
{
    // okay, Bar defined at this point; scoped_ptr can work
}

int Foo::returnValue()
{
    return pImpl->value;
}

Answer 2

As an aside, you may have a problem using boost::scoped_ptr for a pImpl because your pImpl is forwardly declared and you may find that the class needs to be fully visible in order to call scoped_ptr's destructor (which deletes the underlying).

Some compilers will allow you to work around this by putting the body of your destructor in the compilation unit (the .cpp file) where the class is visible.

The simplest solution is that if your destructor has to be implemented anyway you may as well just use a raw pointer and have your destructor delete it. And if you want to use something from boost to help you, derive your outer class from boost::noncopyable. Otherwise ensure you handle copy-construction and assignment properly.

You can use shared_ptr to your pImpl. You can then copy your outer class around happily although they share the same underlying unless you overload the copy-constructor and assignment operator to do otherwise.