Is pimpl compatible with anonymous namespaces?

Question

I am trying to use the pimpl pattern and define the implementation class in an anonymous namespace. Is this possible in C++? My failed attempt is described below.

Is it possible to fix this without moving the implementation into a namespace with a name (or the global one)?

class MyCalculatorImplementation;

class MyCalculator
{
public:
    MyCalculator();
    int CalculateStuff(int);

private:
    MyCalculatorImplementation* pimpl;
};

namespace // If i omit the namespace, everything is OK
{
    class MyCalculatorImplementation
    {
    public:
        int Calculate(int input)
        {
            // Insert some complicated calculation here
        }

    private:
        int state[100];
    };
}

// error C2872: 'MyCalculatorImplementation' : ambiguous symbol
MyCalculator::MyCalculator(): pimpl(new MyCalculatorImplementation)
{
}

int MyCalculator::CalculateStuff(int x)
{
    return pimpl->Calculate(x);
}

score 7 · Accepted Answer · answered Apr 21 '11 at 14:50

7

No, the type must be at least declared before the pointer type can be used, and putting anonymous namespace in the header won't really work. But why would you want to do that, anyway? If you really really want to hide the implementation class, make it a private inner class, i.e.

// .hpp
struct Foo {
    Foo();
    // ...
private:
    struct FooImpl;
    boost::scoped_ptr<FooImpl> pimpl;
};

// .cpp
struct Foo::FooImpl {
    FooImpl();
    // ...
};

Foo::Foo() : pimpl(new FooImpl) { }

answered Apr 21 '11 at 14:50

Cat Plus Plus

113,388
26
185
215

1

This is what I used for the longest time, too, until someone pointed out to me that if you export class `Foo`, it also exports class `Foo::FooImpl`, and that's usually not what you want... – Marc Mutz - mmutz Apr 21 '11 at 14:56
@mmutz Does _export_ mean the MS-related `__declspec(dllexport)`? If yes, i probably don't need to worry. – anatolyg Apr 21 '11 at 15:01
@anatolyg: yes, or `__attribute__((visibility=default))` on GCC/ELF systems. – Marc Mutz - mmutz Apr 21 '11 at 15:04

markshiz · Answer 2 · 2016-07-20T03:17:11.203

3

Yes. There is a work around for this. Declare the pointer in the header file as void*, then use a reinterpret cast inside your implementation file.

Note: Whether this is a desirable work-around is another question altogether. As is often said, I will leave that as an exercise for the reader.

See a sample implementation below:

class MyCalculator 
{
public:
    MyCalculator();
    int CalculateStuff(int);

private:
    void* pimpl;
};

namespace // If i omit the namespace, everything is OK
{
    class MyCalculatorImplementation
    {
    public:
        int Calculate(int input)
        {
            // Insert some complicated calculation here
        }

    private:
        int state[100];
    };
}

MyCalculator::MyCalculator(): pimpl(new MyCalculatorImplementation)
{
}

MyCalaculator::~MyCalaculator() 
{
    // don't forget to cast back for destruction!
    delete reinterpret_cast<MyCalculatorImplementation*>(pimpl);
}

int MyCalculator::CalculateStuff(int x)
{
    return reinterpret_cast<MyCalculatorImplementation*>(pimpl)->Calculate(x);
}

edited Jul 20 '16 at 03:17

answered Jul 11 '11 at 14:54

markshiz

2,273
18
24

I think classical pimpl wouldn't be so popular, if `reinterpret_cast` actually was necessary. This clearly is not the way to go... – m8mble Jul 18 '16 at 19:26
@m8mble To be clear, the question posed wasn't whether it was _advisable_; the question was whether it was _possible_. As posed above, it is definitely possible, despite the other answers to the contrary. – markshiz Jul 20 '16 at 03:04
1

@m8mbl Your question of whether it is necessary is another question altogether. And so a down-vote here seems a little much. The post is still informative and offers a workaround that might be useful for somebody. – markshiz Jul 20 '16 at 03:10

score 1 · Answer 3 · answered Apr 21 '11 at 14:55

No, you can't do that. You have to forward-declare the Pimpl class:

class MyCalculatorImplementation;

and that declares the class. If you then put the definition into the unnamed namespace, you are creating another class (anonymous namespace)::MyCalculatorImplementation, which has nothing to do with ::MyCalculatorImplementation.

If this was any other namespace NS, you could amend the forward-declaration to include the namespace:

namespace NS {
    class MyCalculatorImplementation;
}

but the unnamed namespace, being as magic as it is, will resolve to something else when that header is included into other translation units (you'd be declaring a new class whenever you include that header into another translation unit).

But use of the anonymous namespace is not needed here: the class declaration may be public, but the definition, being in the implementation file, is only visible to code in the implementation file.

score 1 · Answer 4 · answered Apr 21 '11 at 15:26

If you actually want a forward declared class name in your header file and the implementation in an anonymous namespace in the module file, then make the declared class an interface:

// header
class MyCalculatorInterface;

class MyCalculator{
   ...
   MyCalculatorInterface* pimpl;
};



//module
class MyCalculatorInterface{
public:
    virtual int Calculate(int) = 0;
};

int MyCalculator::CalculateStuff(int x)
{
    return pimpl->Calculate(x);
}

namespace {
    class MyCalculatorImplementation: public MyCalculatorInterface {
        ...
    };
}

// Only the ctor needs to know about MyCalculatorImplementation
// in order to make a new one.
MyCalculator::MyCalculator(): pimpl(new MyCalculatorImplementation)
{
}

You'll still pollute the namespace of the public class with the `Interface` class -> no gain. — Marc Mutz - mmutz, Apr 21 '11 at 17:44

Jeroen Lammertink · Answer 5 · 2021-03-24T10:55:37.817

markshiz and quamrana provided the inspiration for the solution below.

class Implementation, is intended to be declared in a global header file and serves as a void* for any pimpl application in your code base. It is not in an anonymous/unnamed namespace, but since it only has a destructor the namespace pollution remains acceptably limited.

class MyCalculatorImplementation derives from class Implementation. Because pimpl is declared as std::unique_ptr<Implementation> there is no need to mention MyCalculatorImplementation in any header file. So now MyCalculatorImplementation can be implemented in an anonymous/unnamed namespace.

The gain is that all member definitions in MyCalculatorImplementation are in the anonymous/unnamed namespace. The price you have to pay, is that you must convert Implementation to MyCalculatorImplementation. For that purpose a conversion function toImpl() is provided. I was doubting whether to use a dynamic_cast or a static_cast for the conversion. I guess the dynamic_cast is the typical prescribed solution; but static_cast will work here as well and is possibly a little more performant.

#include <memory>

class Implementation
{
public:
  virtual ~Implementation() = 0;
};
inline Implementation::~Implementation() = default;

class MyCalculator
{
public:
  MyCalculator();
  int CalculateStuff(int);

private:
  std::unique_ptr<Implementation> pimpl;
};

namespace // Anonymous
{
class MyCalculatorImplementation
  : public Implementation
{
public:
  int Calculate(int input)
  {
    // Insert some complicated calculation here
  }

private:
  int state[100];
};

MyCalculatorImplementation& toImpl(Implementation& impl)
{
  return dynamic_cast<MyCalculatorImplementation&>(impl);
}
}

// no error C2872 anymore
MyCalculator::MyCalculator() : pimpl(std::make_unique<MyCalculatorImplementation>() )
{
}

int MyCalculator::CalculateStuff(int x)
{
  return toImpl(*pimpl).Calculate(x);
}

Is pimpl compatible with anonymous namespaces?

5 Answers5