Hey everyone, Here are my preliminary thoughts regarding the pimpl library. First, I like the idea of having it or something similar to it be part of Boost because the functionality seems like it would be useful in a lot of cases. One of the strengths of including such a library in boost in my opinion is that it will appear in the "Libraries" list, which means that more people will be exposed to the idiom and will think of it as a possible option than they would otherwise. Put another way, Boost is both a collection of libraries and of best practices, so a good pimpl implementation belongs in boost even if it is a relatively small one. Now the question is whether this particular implementation itself is good enough. Honestly, I have trouble figuring out exactly what it does except by reading through the code since the documentation itself doesn't say what it does behind the scenes except to say vaguely that it implements "pointer semantics" or "value semantics". So one thing that the documentation really needs (among the many others that have been mentioned) is a section that clearly lists the features that it offers. One objection that Artyom reasonably raised is that this implementation may try to do too much, and doesn't offer much in the way of customization. However, people like Artyom are most likely going to be rolling their own implementation of the pimpl idiom anyway. I think that it is sufficient that a implementation be "good enough" for an ordinary user's needs that they won't have to think about any of the ugly details since it does all of that work for them, and to the extent that this is true of the current implementation Now for my thoughts on the code itself. Two of the classes in the implementation should probably be reviewed separately and put in different Boost libraries rather than the Pimpl library, namely safebool and value_semantics_ptr. safebool is an interesting solution to the problem that implicit conversions to the "bool" type implies implicit conversions to hosts of other types such as integers which can result in all sorts of unexpected behaviors that will bite you when you least expect it. safebool solves this problem by providing a convenient way for you to implement an effective "bool()" by instead implementing an operator that returns a special function-pointer that is specialized to your class and thus is equal neither in type nor value to a safebool for any other class; a minor cost that is presumably incurred by this is that a small amount of code will be added to the binary for this special no-op function, since I don't see how this can be optimized away. The implementation looks straightforward and the comments contain good documentation explaining both the usage and the rationale. I would recommend that safebool be accepted into boost and added to the utilities library on its own merits. value_semantics_ptr is a class that contains a pointer to data (or possibly a null pointer) and has the semantics that upon assignment it copies the data from another pointer, allocating (or freeing) its internal pointer if necessary. I think that this is a good idea and that something like this belongs in boost, but I suggest that the current implementation should be changed at least as follows before being accepted: First, it should be taken out of the pimpl<> class and made independent. Second, the traits struct member should be changed from a derived class to a second parameter in the list of template arguments to the value_semantics_ptr type, with the deep_copy member being pulled outside and being made the default value of the new second template argument. Alternatively, a type function --- e.g., copy_traits<T> --- could be used instead. Third, if Boost::Move is going to be ready soon it would be ideal if this class could be modified to support it in order to potentially save some extraneous copies. Fourth, one of the methods confuses me and apparently also the author: bool operator<(value_semantics_ptr const& that) const { return this->impl_ < that.impl_; // Should it be "*this->impl_ < *that.impl_" instead? } so this should be finalized. Fifth, the implementation will need to be fleshed out. operators such as * (dereference) and -> will need to be added, as well as some additional typedefs such as value, pointer, reference, etc. Operator safebool() will also need to be added. In conclusion, if pimpl is accepted I think it would be better for there to be a polished value_semantics_ptr that we feel comfortable having be directly tested and exposed to users be the back-end of this class than something hidden away, so when value_semantics_ptr is ready I think that it should ultimately be accepted to the Smart Ptr library. So much for safebool and value_semantics_ptr, now on to pimpl itself. The code looks straightforward and the comments document it reasonably well --- in fact, the ratio of comments to code in the main pimpl class is roughly 2:1, and in that count I was lazily including whitespace and single { and } lines as code lines, so that gives you a sense of how heavily it is commented. I have only taken a quick glance through it since this is a pre-review, but it all looks simple and straightforward to me, and it does a good job of generating tedious boilerplate so the user doesn't have to, exactly as it claims. On a side note, one additional set of semantics that I would recommend adding is move semantics, which could be implemented using the Boost::Move library. In conclusion, I think that this library should be broken into parts with safebool being put in utilities, value_semantics_ptr (and we can bikeshed over the name here of course :-) ) being put in smart ptr, and pimpl being put in a separate library. After this is done pimpl will be quite small, but I think that it pulls enough weight that it is nonetheless useful. To repeat what I said earlier, boost is not just a repository of libraries but of C++ best practices, so even if pimpl is small if it does a good job of capturing the pimpl idiom I still think that it should be accepted into Boost. Cheers, Greg