Re: [boost] [pimpl] Mini Review
I've been watching this discussion for a while (with vested interest), and want to use a few examples to shore up some of the discussion (and possibly move it off list so that things can proceed in a constructive manner). The concern is not that Pimpl is implementing an opaque pointer, it's the miscellaneous copy constructors, constructors, equality or whatever other misc "extras" that come along from using Pimpl, right?
Not quite. Copy constructors and copy assignment, for example, are appropriate. They allow the library to manage copies of the implementation object. Problems arise, for example, from the addition of null(),operator ->(), and operator *().
This is less rhetorical than I'd like, but what's the right amount of baggage that a Pimpl implementation can bring along with it? On one side of the spectrum you have an austere implementation that is literally an opaque pointer with its relevant compilation glue ("the one true/pure pimpl"). On the other side of the spectrum you've got something that overloads functions and operators and makes a pimpl implementation easier to use, ala the Pimpl library. For example: https://github.com/sean-/Boost.Examples/blob/master/pimpl/boost/pimpl/pimpl.... Not strictly necessary, but I understand why it's there and why it would be wanted in a Pimpl implementation. I also understand the puritanical argument for why you wouldn't want it.
reset(), which may be protected rather than public (I don't recall), is not as much of a problem, but my suggestion for that was that since the constructors allocate the implementation object, "resetting" the implementation should likewise not involve allocation. For that, I suggested a construct() member function template set, paralleling the constructors that implement "perfect forwarding." A constructor and a reset() that take a deleter would be appropriate for cases in which allocation is not desired. That design would be symmetrical.
Got it, ok.
class Book : public pimpl< Book >::pointer_semantics { }; // Or is the interface different because the ABI is // different? class Book { class* impl_; };
It's different because using inheritance and public member functions, pimpl<Book>::whatever has augmented Book's interface.
Correct/agreed.
With as (in?)efficient compilers sucking up gigs of RAM and frequently crashing during compilation[1], having a friendly mechanism to hide linkage and implementations behind opaque pointers is useful beyond externally exposed APIs in libraries. The tedium of doing this frequently makes the Pimpl library an easy win for many developers (though possibly not all of them and not in all situations). As a Pimpl consumer for over a year, I'm trying to concretely demonstrate where the possible pitfalls could be lurking with this implementation. Thanks in advance. -sc
I have no problem with using the Pimpl Idiom. Just this past week I was able to advance some application testing readily because we used it in a couple of classes involved in an application's disorderly shutdown due to misconfiguration. My concern is that the Pimpl Library seems to have pushed past the idiom into something else, which is surprising to me and, clearly, to others. I should also be clear that I'm not against what the Pimpl Library has done, in terms of augmenting the derivate's interface via public inheritance, but that doing so should be in a class template and a library of a different name.
That feedback makes sense, thanks Robert. I think the unease is coming from the Pimpl library implementing helpers and extending beyond only providing an opaque pointer. TBH, before using Pimpl, I'd implemented a subset of the helper functions in my own home-grown pimpl library so seeing them standardized never struck me as objectionable (the possibility of running in to name clashes, not withstanding). In fact, my search for Boost.Pimpl began because I missed adding a non-const* ->() to my implementation. After wading through pages of gcc error messages (naturally this happened in the middle of a asio/bind/wrap call) I started looking for a reference implementation that had some of this spelled out since I didn't think I should be spending my time reimplementing the wheel. I don't have an answer for what's "right" on the spectrum of austere vs "featureful", but the featureful side of Pimpl has clear merit. I'll move the rest of my comments over to the review tool. -sc http://demo.smartbear.com/boost/go?page=ReviewDisplay&reviewid=5 -- Sean Chittenden sean@chittenden.org
Sean Chittenden wrote:
This is less rhetorical than I'd like, but what's the right amount of baggage that a Pimpl implementation can bring along with it? On one side of the spectrum you have an austere implementation that is literally an opaque pointer with its relevant compilation glue ("the one true/pure pimpl").
There's good reason to provide support for copying, assignment, and swapping, of course. After that, it gets iffy.
On the other side of the spectrum you've got something that overloads functions and operators and makes a pimpl implementation easier to use, ala the Pimpl library.
There are two interfaces to discuss. There's the public interface, which is given to the derivate (Book). It is possible to derive privately or "protectedly" to reduce the public interface, but that's not the normal use case. There's also the implementation interface, which is accessible to the derivate for accessing and manipulating the implementation object. My concern is mostly for the public interface, but I have thoughts on what's included in and how to improve the implementation interface, too. It's important to discuss each interface separately. I know I've also convoluted what functionality is part of which interface, adding to the confusion I'm sure.
For example:
https://github.com/sean- /Boost.Examples/blob/master/pimpl/boost/pimpl/pimpl.hpp#L134
Not strictly necessary, but I understand why it's there and why it would be wanted in a Pimpl implementation. I also understand the puritanical argument for why you wouldn't want it.
That link points to the middle of a comment, so I don't know what you're referring to.
In fact, my search for Boost.Pimpl began because I missed adding a non-const* ->() to my implementation. After wading through pages of gcc error messages (naturally this happened in the middle of a asio/bind/wrap call) I started looking for a reference implementation that had some of this spelled out since I didn't think I should be spending my time reimplementing the wheel. I don't have an answer for what's "right" on the spectrum of austere vs "featureful", but the featureful side of Pimpl has clear merit.
I've suggested these instead of the member selection and dereferencing operators: implementation & impl(); implementation const & impl() const; Those are easy to call -- no need for (*this)->, etc. The diagnostics would probably be a bit easier to read, too. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
participants (2)
-
Sean Chittenden -
Stewart, Robert