On Thu, Jun 27, 2019 at 12:23 AM Gavin Lambert via Boost < boost@lists.boost.org> wrote:
Granted this does allow for a zero-base-size optimization, which is useful as most of the time it would be an empty tuple, but it still feels wrong (and would allow inappropriate slicing if you try to pass an out_ptr as a tuple parameter, since it's public inheritance).
That's a mistake from mixing class and struct: it should be all "class" and the tuple base should be marked protected. I've fixed it on the feature/boost.review branch.
There's one thing I really dislike about the implementation, though. It's the clever aliasing trick. While it is clever, it's unsafe and it adds complexity to the library. In my opinion, this kind of thing should live merely in a separate experimental branch, or always be enabled in the cases where we know for sure it's safe (I think that's never but I'm not sure).
I tend to agree. While strict-aliasing rules are a bit of a pain in the butt, sneaking around behind them and poking private members of unique_ptr and friends doesn't seem like a good idea, even if it does improve benchmarks.
And "UB-based optimization" does not inspire confidence.
I will pull request a friend class of a forward declaration for boost::movelib::unique_ptr to make it not have to go through UB. There is nothing I can do for standard pointers other than to give up the optimization or to use the UB. (This is one of the reasons it was proposed to the standard, asides from being an existing practice with wide implementation experience.) Sincerely, JeanHeyd