14 thoughts on “GotW #101: Solution

  1. I see 2 potential problems with this:

    First of, template class pimpl::impl will need to be in the namespace as “pimpl”.

    So if widget is in its own namespace, then the implementation will need to be in a different one?

    .h

    namespace foo {

    class widget : public pimpl {…}

    }

    .cpp

    using namespace foo;

    template class pimpl::impl

    namespace foo {

    widget::widget() : pimpl(){}

    }

    Second, what about inheritance? Will that not cause ambiguity?

    class widget : public pimpl {…}
    class clock : public widget, public pimpl{…} // what pimpl to use?

  2. Would it be possible to extend the pimpl class and provide default implementation for some common methods? e.g. swap?

    class pimpl {
    protected:
    class impl;
    std::unique_ptr m;

    public:
    void swap(pimpl& other)
    {
    m.swap(other.m);
    }

    };

  3. I have tried to write a similar helper class before, but had not thought to use a perfect forwarding constructor for initialization. Cool.

    However, I really dislike the naming of pimpl.cpp, because I dislike seeing cpp files in include statements, and especially since this one is merely a template implementation file, which often have a special file extension (tpp?). Personally, I just leave them as .h files but would name them something like pimpl_fwd.h and pimpl_impl.h

  4. Shouldn’t you also include copy/move constructors?


    pimpl(std::unique_ptr&& other);
    pimpl(const std::unique_ptr& other);

    template
    pimpl::pimpl(std::unique_ptr<typename pimpl::impl>&& other) : m(std::move(other)){}

    template
    pimpl::pimpl(const typename std::unique_ptr<typename pimpl::impl>& other) : m(new pimpl::impl(*other)){}

    or maybe even?


    pimpl(T&& other);
    pimpl(const T& other);

    template
    pimpl::pimpl(T&& other) : m(std::move(other.m)){}

    template
    pimpl::pimpl(const T& other) : impl_(new pimpl::m(*other.impl_)){}

  5. Herb, I encountered a tiny problem when using pimpls like the one you describe. Perhaps you know how to solve it better. Or perhaps you could mention it your GOTW item.

    I try to apply the rule “treat warnings as errors”, and my problem is in fact about an annoying warning. (Annoying because it is a “template” which spreads across many, many lines)

    I am using VS compilers. I am exposing my class, lets call it Flight, as part of the interface of my DLL, so it is declared as:

    class EXPORT_MODE LFlight
    {
      // ...
    }; 
    

    where EXPORT_MODE is either __declspec(dllexport) or __declspec(dllimport), based on who is including the header. I think this is a standard and recommended practice?

    I do not want to expose the guts of Flight to the users of my DLL, so I use pimpl, but in fact I end up exposing the pimpl template to anyone. In the end I get warning C4251: “[class template] needs to have dll-interface to be used by clients of class LFlight”. This is a general issue of exposing template members as part of dll interface. But pimpl, on the other hand, should be just the right tool for such “dll firewall”.

    I fix this problem by adding additional declarations which introduce explicit external EXPORT_MODE specializations for pimpl:

    extern template class __declspec(dllimport) pimpl;
    or
    template class __declspec(dllexport) pimpl;

    based on who is including the header. Is this the only way?

    Regards,
    &rzej

  6. Herb, I notice you’ve changed your original solution to a simpler one, but as far as I can tell, your new solution basically amounts to writing the solution to #100 in a very round-about way when #100 is good enough as it is. At least the original inherited solution to #101 saved you writing the pimpl management code time and time again. This new solution doesn’t even do that.

    Now, instead of writing:

    class widget {
      class impl;
      std::unique_ptr<impl> d;
    };
    

    you write:

    class widget {
      class impl;
      pimpl<impl> d;
    };
    

    Seems to me like its the same result and doesn’t save us writing any code.

    I’m sure I must be missing something.

  7. @Simon: In Herb’s pimpl the difference seems to be in the variadic constructor. But you could also use different pimpl that provides value semantics: deep copy and deep assignment (unique_ptr provides shallow copy and assignment). Then your class gets copy and assignment for free (generated by compiler).

  8. I was reading this post and the previous GotW #100, and I was wondering: Is there a benefit to using this idiom to help with C++/CLI Native/Managed interoperability?

    It would be nice to have custom controls and objects be more purely managed code, but I have to do a great deal of interaction with native libraries. This can lead to Visual Studio designer instability, Visual Studio locking of DLL’s, and difficult debugging with so many managed to native transitions. Putting all of the managed/native interop code in an “Impl” managed class (and into a .cpp file and out of the header) may ease this type of development.

  9. I don’t quite see how this makes the pimpl any simpler?

    + include extras files
    – instead of pimpl_(new impl(params)), we now write pimpl(params)

    Doesn’t it actually make it more complex than using the pimpl idioms without any “helper”?

  10. @Robert: You definitely need to implement a pimpl idiom *somehow*. If you choose to do it with a raw pointer you are risking lots of memory-management-related problems. If you choose to use *some* smart pointer you need to include an extra header file anyway.

    Now, which of the hundred possible smart pointers will you choose? The one that fits your task best. What does std::unique_ptr have to offer? (1). It will release memory in the destructor, (2). It will prevent the automatic generation of invalid copy constructor and copy assignment.

    Can other smart pointers offer something else? Something better? I think yes: they can offer simpler construction syntax, and automatic generation of the correct copy constructor and copy assignment (with deep copy semantics). If you want your class to be copyable it saves you a certain amount of “boiler-plate” code. How is such pointer called? It is not called yet (at least I haven’t heard of such pointer), but you could call it pimpl.

    I guess the answer to question “is it worth it?” pretty much depends on your programming style, your taste, your environment’s constraints, etc. If you do not want your class to be copyable — such smart pointer is not any better for you than a std::unique_ptr. If you cannot afford including a header file, this solution is not for you (I can afford it, and put it into a pre-compiled header, to decrease compile-times).

    One other reason to use such pimpl helper is that by only using something with name “pimpl” makes your intentions more clear. Anyone will easily see that you wanted to implement this idiom (even if you have made some error that prevents code analysis). But again, this is just an aesthetic matter.

  11. @Andrzerj: I agree that you need to do it “somehow”. The question is what does this helper offer that is better than simply using std::unique_ptr? And yes, I would need to include anyway, but in this case I could end up “having” to include “pimpl.h” AND . For me this is a case of is the extra hassle/complexity (however small) worth it?

    I think this boils down to the case (which you describe) where you want a deep copy and would prefer automatic generation of c-const and c-assign. In any other case I cannot see any advantage for this approach over using std::unique_ptr directly.


    One other reason to use such pimpl helper is that by only using something with name “pimpl” makes your intentions more clear. Anyone will easily see that you wanted to implement this idiom (even if you have made some error that prevents code analysis). But again, this is just an aesthetic matter.

    Indeed it is an aesthetic matter, though in my personal opinion “struct implementation” is a pretty big giveaway. You migth as well do a typedef on std::unique_ptr anc call it pimpl.

  12. @Simon: I should have been clearer in the text about benefits of the helper. Using the helper, the code is simpler because if you write it yourself you also have to write out-of-line ctor, and have to remember to write the out-of-line dtor. The code is also more robust, because for example you can’t forget to write the out-of-line dtor.

  13. I believe pimpl needs to be explicitly instantiated as well. I tried using the code in GotW #101 in VC 2010, and it did not link without expliclty instantiating pimpl where I implemented widget::impl.

    MSN

  14. Should operator-> and operator* actually be const?

    T* operator->() const;
    T & operator*() const

Comments are closed.