search

yet-another-user / pimpl

The C++ Pimpl Idiom Again!
36 stars 7 forks source link

Traits and inplace refactor #14

Closed muggenhor closed 7 years ago

muggenhor commented 7 years ago

This moves all the singleton related stuff to the traits class by moving construction (emplace) there.

Then it takes advantage of this to refactor the inplace policy to just use a boolean flag to indicate whether an object exists in its storage. This flag is optimized away (using boost::compressed_pair, which performs the empty base optimisation) for the always_inline policy by using an empty struct, convertible to bool as its flag.

PS I've been looking a bit at the way we handle allocators and doing some experimenting with it in my allocator-refactor branch. In short: we currently don't use all the typedefs correctly (pointers mainly), nor do we support stateful allocators (that requires an API change).

yet-another-user commented 7 years ago

Merged with a few adjustments. Thanks.

1) I keep replacing your "const auto" with "derived_type" on purpose. I want to make sure it is the "derived_type" type... and it should not be 'const' IMO. I am not even sure how it compiles. :-)

support stateful allocators

I know. I was avoiding it (until now anyway) as that'll require storing allocators with the objects... like shared_ptr does it or unique_ptr for deleters.

muggenhor commented 7 years ago

I'm using auto because the type should be allocator_traits<>::rebind_alloc<derived>::pointer. That may be a raw pointer to derived but doesn't have to be. I thought the full traits type to be much typing: hence auto. As for const that's something I generally do a lot, unless I want a variable to change. Note that the const just affects the pointer in this case, not what it points to.

Storing the allocator is pretty easy actually, just inherit from it and you get the empty base optimization along with it. See, the deleter in muggenhor/pimpl@9dc59946cd0c32d7a49a2a9d1270fe36791ab33e. It's all the parameter passing that's a lot of work, then proper handling of the allocator for copy, move and swap becomes the really tricky stuff (see the propagate_on_container_xxx stuff.

yet-another-user commented 7 years ago

'm using auto because the type should be allocator_traits<>::rebind_alloc::pointer

Yes, that's a fair comment.

Just checked that sizes of Pimpled unique and copied classes. They are of sizeof(void*) after your traits_ pointer-related changes. Damn cool. Well done. Very impressive.

yet-another-user commented 7 years ago

Storing the allocator is pretty easy actually, just inherit from it and you get the empty base optimization along with it.

It was not storing difficulty that has been stopping me. It increasing the size of the object... but you are correct, with empty-base optimization it'll be taken care of... If so, we might follow shared_ptr "lead" and API... unless you have a better idea... which from experience so far you often do. :-)

yet-another-user commented 7 years ago

BTW. I moved allocator info into detail::traits::base. You probably might like to take advantage of it in base::emplace()... because right now its signature looks pretty scary. :-)

yet-another-user commented 7 years ago

Giel, I'll be off-line for one week... potentially for two from (Aus) tomorrow morning. I opened the master branch if you decide to check anything in (or fix something I keep messing up), so that I am not in your way. Functionality you've already added is hugely impressive... it'd take me years to have it implemented. :-) If I was not sure before, then now with its no-mem-overhead, etc. features this new smart-ptr certainly needs to be in Boost. Allocators (that you mentioned and I was dragging my feet) would be a wonderful addition. No pressure. :-)