Re: [patch] Improving shared_ptr for C++0x

["Jonathan Wakely" <jwakely dot gcc at gmail dot com>]

On 15/11/2007, Paolo Carlini <pcarlini@suse.de> wrote:
> Hi Jonathan,
>
> I have a small comment and a much more substantive one: the first one is
> that, as a general rule, we try not to have #include in the tr1_impl

Thanks for pointing that out, I'll move them into <std/memory>

This works better, since only the C++0x version needs std::move and
new_allocator (and that could be replaced by std::allocator easily
enough if you prefer.)

> headers. The second one is that I'm rather concerned, at this stage of
> the development process, that the changes touching both the
> _GLIBCXX_INCLUDE_AS_TR1 and _GLIBCXX_INCLUDE_AS_CXX0X versions of code
> (basically the first part of your patch) can somehow, unadvertently
> damage the TR1 implementation which, by now, is rather stable. Would you
> consider splitting completely the first part of the implementation to
> /std and /tr1 (the second part in /tr1_impl, of course), similarly to
> what we are doing for type_traits? I'm sure there will be some
> redundancy but we'll be also much safer. In that case you can consider
> your changes morally pre-approved (just let me have a look ;)

gah! I was hoping you wouldn't say that  ;-)

I can do it, of course, if you'll let me dump my thoughts here and
shout if you think I'm going wrong.

I see this as the best structure if you want to freeze tr1::shared_ptr:

tr1_impl/boost_sp_counted_base.h:
_Sp_counted_base
(common to tr1 and c++0x, keep it exactly as we have it today - this
means removing the part of my patch that makes _M_destroy() pure
virtual - that's fine, it can still be overriden in C++0x as needed)

tr1/boost_sp_shared_count.h:
_Sp_counted_base_impl (PD)
_Sp_deleter (deleter in P case)
__shared_count
(these classes are exactly as we have today, but I'd rename the first
one if the ABI allows me to :)

std/boost_sp_shared_count.h: (should this really be in std? bits maybe?)
_Sp_counted_ptr (P)
_Sp_counted_deleter (PDA)
_Sp_counted_ptr_inplace (erm, sort of PA+obj)
__shared_count
(exactly as shown in my patch)

tr1_impl/boost_shared_ptr.h:
__weak_count
__shared_ptr
__weak_ptr
shared_ptr
weak_ptr
etc.
(common to tr1 and c++0x, a limited number of tests for
_GLIBCXX_INCLUDE_AS_CXX0X around move constructors and other
C++0x-only functions)

There will be some very small changes to the tr1::shared_ptr code, for
instance I will keep this part of my patch, which would be in common
code in tr1_impl:

@@ -539,7 +724,7 @@
      template<typename _Tp1>
        explicit
        __shared_ptr(_Tp1* __p)
-       : _M_ptr(__p), _M_refcount(__p, _Sp_deleter<_Tp1>())
+       : _M_ptr(__p), _M_refcount(__p)
        {
         __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>)
         // __glibcxx_function_requires(_CompleteConcept<_Tp1*>)

This will need a new unary constructor in tr1::__shared_count that
uses _Sp_deleter. This means _Sp_deleter is only needed in TR1 mode
and not in the common code. Getting rid of _Sp_deleter was a goal of
my re-work, but I'm happy if it's gone in C++0x only.

I hope that plan sounds reasonable.

Does type erasure mean the name of _Sp_counted_base_impl isn't part of
the ABI and it could be renamed? I'm not desperate to change it, but I
am curious if it would be ABI breaking.

I won't be able to work on this until monday evening at the earliest
and will need to extend the docs too, so there's no rush to let me
know if you have any better ideas.

Cheers,
Jon

P.S. I noticed a few places in the new _Sp_counted_ptr and _inplace
classes where I could use concept-checks to verify pointer conversions
and CopyConstructible requirements, so I'll add those too. Until we
get real concepts those are still useful.