[boost] smart_ptr implementation descussion topics

From: Gennadiy Rozental (rogeeff_at_[hidden])
Date: 2002-05-06 04:41:23

Hi,

I uploaded my latest version into the vault area. See at the end for list of
changes. Sorry, this going to be long message. Here I would like to
emphasize several important topics related to smart_ptr implementation:

Topic 1: Storage and Ownership policy treatment
This is probably most important topic since it greatly affect framework
design. Let me repeat some of my point here, as I does not seems to get a
response before, while I think that Loki SmartPtr mistreat it a bit. So my
understanding of storage and ownership policies is following:
  a. Storage policy should answer the question: "HOW to store, access, copy
and release the resource".
  b. Ownership policy should answer the question: "WHEN to release the
resource".
Note that Ownership policy does not manage when to copy the resource since
it is manager by user by means of calling copy constructor or assignment
operator.
I believe that  if we follow this definitions we could build strict
orthogonal system that smoothly allow us to implement all reasonable cases
of resource management. Loki::SmartPtr does not follow this definition in
many places, and I consider this a framework flaw.
   Most important part that is affected by this definition is a design for
smart_ptr copy constructor. Loki::SmartPtr rely on Ownership policy  method
clone to perform a real copying, while I propose to move real copying into
Storage policy copy constructor. The only thing that ownership policy need
to do is to add reference after copying is performed. Without going into
details my version is looking like this now:

smart_ptr(  smart_ptr(const or not)& rhs )
: StoragePolicy( rhs ), OwnershipPolicy( rhs ),... // really here you will
see adaptor copy constructor, but for clarity ...
{
     OwnershipPolicy::add_reference( StoragePolicy::storage() );
}

Here you see clear separation of resource copying and ownership management.
Now let's take a look onto smart_ptr constructor by pointer type:

smart_ptr(  pointer_type const& rhs )
: StoragePolicy( rhs )
{
}

Here ownership policy default constructor is called. But what if ownership
policy need an access to storage to initialize itself? The clear example is
intrusive reference counting. We could of course to rely that T constructor
will do the work, but this way we separate ownership management logic into 2
different places and also it could be impossible to do for some custom
Ownership strategy. That is why I propose to add first reference method to
ownership policy interface and constructor will look like this:

smart_ptr(  pointer_type const& rhs )
: StoragePolicy( rhs )
{
    OwnershipPolicy::first_reference( StoragePolicy::storage() );
}

Another example of real usage of this method is ownership policy
by_first_who_got. Intended usage of it for iterators allocating some
resources. As we know all STL algorithm are accepting iterators by value
(including all dispatchers), so we need some smart pointer to mange the
allocated resource. We could use shared pointers of course by, it may
produce undesirable overhead on counter management. Really what we need is
to say: the one who get the resource first should destroy it. It could be
dangerous it used incorrectly, but it also could be useful in situations
like this:

void foo( string const& input_buffer )
{
     MyBufferItterator it( input_buffer )
     for_each(  input_buffer, MyBufferItterator(), do_something() );
}

It's just an example, so let not discuss it's usefulness. My point is that
it could be useful.

Now let's take a look onto release() function ( BTW I am calling it leak
following Andrei proposition - I like it much more). Original Loki::SmartPtr
did not do anything with ownership policy. Dave version reinitialize it with
default constructor. I argue that it is not general enough. Some policies
may want to do something else. Most wide usage if it is to prevent leaks at
compile time using static assert. One may say that the same could be done
using checking policy but when placing it directly into ownership policy
clarify the intended usage. In other words I think leak management also
belong to the ownership management realm and should be covered by it. Some
custom ownership policy may track all leaked objects, while other may not
leak at all.
Another issue related to release method is storage handling. Dave's version
reinitialize it with default value. I argue again that it is not generic
enough. One may want different behavior. For  example funs of version 2
auto_ptr may want to keep the value while real ownership will be managed by
appropriate ownership policy. In summary my version of release/leak method
looks like this:

    stored_type         leak()
    {
        checking_policy::on_leak( storage_policy::storage() );
        ownership_policy::leak_reference( storage_policy::storage() );
        return storage_policy::leak_storage();
    }

Let's summarize. I propose following contract for the ownership policy:
//                       OwnershipPolicy()
//                       OwnershipPolicy( OwnershipPolicy const& )
//      void            first_reference( stored_type& )
//      bool            release_reference( stored_type& )
//      void            add_reference( stored_type& )
//      void            leak_reference( stored_type& )
//      void            swap( OwnershipPolicy const& ) if not empty

And following contract for StoragePolicy:
//                       StoragePolicy()
//                        StoragePolicy( StoragePolicy const& )
//      void                        init( pointer_type const& )
see below for more discussion on this
//      stored_type&         storage()
//      stored_type             leak_storage()
//      pointer_type            get_pointer()
// const_pointer_type       get_pointer() const
see below for more discussion on this
//      reference_type         get_reference()
// const_reference_type    get_reference() const
//      void                         swap( StoragePolicy& ) if not empty
//      bool                         is_valid() const

Topic 2: Destructive copy semantic support.
I was cunning a little when I sad that could achieve clear orthogonality in
all reasonable case on resource management. Cause there is one example when
it is not true, namely ..... std::auto_ptr. Latest incarnation of smart_ptr
combine ownership and storage in inseparable way. We lucky that Bill Gibbons
and Greg Colvin elaborated all the solution to pitfalls for this semantic.
So if we take a look onto auto_ptr specification, we find following:
  a. auto_ptr have an explicit constructor by pointer type
  b. it copy constructors expect non const arguments
  c. it uses special class auto_ptr_ref to handle rvalue to lvalue
conversion.(I now finally got why you needed this by_ref class)
I argue that all of it is specific to destructive copy semantic. And
accordingly should not litter generic framework
interface/design/implementation ( I found that supporting/switching both
const/non-const constructors is pain in you know where). As an alternative
solution I propose to segregate everything related to destructive copy
semantic into subclass of smart_ptr. I did not work out the details yet, but
I would like to know what you think.

Topic 3. Template template parameters
While Dave trying to support template template parameters by means of
introducing macros for non-conforming compilers, I would like to argue a
value of them in general. When you specify component parameter as template
template parameter you fix expected signature of policy template class. It
could be acceptable sometimes but for smart policies I found that it is not.
It could bring some syntactic convenience when you need to specify all
parameters for smart_ptr, but it's not that important considering that most
of the users are going to use standard pointers generators anyway, some will
specify only some of them (that is  even more true now, when I introduced
named template parameters into smart_ptr), others will use typedefs to
specify it once anyway. While the drawback are obvious. For one it is too
limiting (Loki::SmartPtr itself struggle to support required signature, see
ref_linked for example where additional class introduced to copy with unused
template parameter). I may not need any template parameters, so why should I
follow the signature. I  may not want any template parameters for my policy
cause it is implemented offline. I may want more template parameter cause I
want to reuse this policy for a family of smart pointers. Not mentioning
that they are not supported by many compilers; none of boost staff will work
with it.

Topic 4. smart_ptr pointer type constructor
There are 2 more issues with this constructor. First is it's argument type.
I argue that it should be pointer_type, but stored_type. Stored type could
be heavyweight (like another smart_pointer ), while what we really want is
plain pointer
Another issue is much more difficult.  As you will see in my implementation
I introduced another template parameter PoliciesAdaptor to support custom
policies adaptors. And now smart_ptr inherit namely this PoliciesAdaptor,
but list of storage, ownership and so on policies. Original version of
discussed constructor looked like this:
smart_ptr(  pointer_type const& rhs )
: StoragePolicy( rhs )
{
...

Now it would become like this:
smart_ptr(  pointer_type const& rhs )
: PoliciesAdaptor( rhs ) //??? PoliciesAdaptor need to know where to direct
rhs!!
{
...

Implementation of PoliciesAdaptor need to know where to forward the
argument. And what is worse it need to know it's type cause templated
version may be to greedy. With implementation of policy adaptors following
Andrei optional inheritance design it's almost impossible (though maybe it's
me).

What I propose for how is to change storage policy contract. Instead of
constructor by pointer type, let introduce init function. Then discussed
constructor will look like this:
smart_ptr(  pointer_type const& rhs )
{
    StoragePolicy::init( rhs );
...

Do you have another proposition?

Topic 5: COW semantics
I believe that Loki::SmartPtr is misleading stating that cow semantics could
not be implemented in bounds of smart_ptr. Real issue is that it does not
separate const and non const access sometime. Once we do this it became
possible to implement cow.
To separate const/non-const access we need to expand storage policy
interface to const version of get_pointer, get reference function returning
const_pointer_type and const_reference type respectively. For more details
see cow storage policy implementation.

Topic 6: Templated comparison and order operation.
Current versions of my and Dave implementations both define templated
operators where second argument is in a form "U const*". It is obviously
incorrect in general case for generic resource type. But once we try to
change this type to just "U" it become too greedy and prevent comparisons
with NULL. Given the tradeoff I choose not to support template operators for
now for generic resource.

Now let take a close look onto more specific differences between my and Dave
versions:

0. I am not using BOOST_NO_MEMBER_TEMPLATES switch since I am using member
templates extensively and rely on it. I believe it's acceptable requirement.

1. Since file became to big to efficiently manage I separated it into
several independent modules. See at the end for the list.
    Accordingly included files moved to appropriate header. Specifically
it's important for <functional> that is moved to smart_ptr_compare and would
not be included by default at all.

2. class by_ref. Even if we want to implement this logic directly in
smart_ptr we should use existent boost version of it.

3. const_pointer_type and const_reference_type should be defined by storage
policy also, in case it want to use some kind of proxy for the non const
version

4. In my current version destructive_copy is a trait class deducing value
from *storage* policy

5. In my version get_impl is the public member of the framework named get
(get_impl_ref is protected member get_reference), while internally framework
members are using protected members get_pointer, get_reference. It includes
checking on direct access. More over we need 2 versions const and non const.

6. operator== inside the smart_ptr class should compare with pointer_type
not T const*.

7. automatic_conversion_result should be non const, since if is const it
could not use non-const get_pointer, while const get pointer returns
const_pointer_type

8. I was not able to make operator[] work as a part of array_storage_policy.
So I do not provide one. Instead I added second template parameter to
default_policy ( I name it plain_pointer) Deleter that could be either
checked_deleter or checker_array_deleter

9. I did not get why do we need separate storage policy implementation to
adopt existing smart pointers. Don't they store plain pointer inside? Looks
like for some reason you are trying to combine storage and ownership policy
in one bottle. I admit that I did not tackle boost smart pointers yet but it
seems fishy. BTW you have a bug (I think) in the method unique
implementation.
I do support intrusive reference counting through traits. I believe it more
flexible. And does not require runtime polymorphism. Though I will provide
this feature also.

10. Once again: I believe that we should be touch issues related to
multithreading support for now.

11. Once again: I believe that com_ref_counted does not deserve a place in
generic framework.

12 I finally implemented ref_linked. Loki version has a serious bug (I
think). Try the following

ref_linked_smart_ptr<T> v, v1;
v = new T;
v = v1;
v = v1;

See my version to compare. It probably could be expressed more concisely,
but it works for me.

13. To support destructive copy I provide for now special wrapper
copy_destructible<base_storage_policy> that made copy destructible storage
out of supplied policy. This is not a part of ownership policy logic.

14. deep_copy in a form currently provided in Dave version does not deserve
a place in generic framework: it is too specific. I provide
deep_copy_pointer storage policy that uses copy constructor to make a stored
pointer copy.

15. Once again: compare our checking policies implementation  243 vs. 6
predefined policies plus my version provide easy support  for custom ones.

16. what is # include <poppack.h>?

Here the list of features implemented in my latest update:

1. I added named template parameters support
2. support  for destructive copy semantic in bounds of smart_ptr framework
3. I segregate framework into following modules:
smart_ptr_framework.hpp    framework itself
smart_ptr_storage.hpp         supplied storage policies implementation
smart_ptr_ownership.hpp    supplied ownership policies implementation
smart_ptr_conversion.hpp   supplied conversion policies implementation
smart_ptr_checking.hpp      supplied checking policies implementation
smart_ptr_adaptors.hpp      supplied policies adaptors implementation
smart_ptr_compare.hpp      specialization for std::less to support using
smart_ptr as a key in associated containers
smart_ptr.hpp                     composing header ( all but last of
previous headers)
4. I added First template parameter type T, to support default value
specialization
5. I added last template parameter PoliciesAdaptor to allow supply custom
policies adaptors
6. I implemented 3 policies adaptors simple, chaining and optimizing to
optimize MI EBO
7. Deleter template parameter added to plain pointer storage policy
8. I implemented deep_copy_pointer storage policy
9. I implemented ref_linked ownership policy
10. I implemented copy on write storage policy
11. I implemented copy_destructible storage policies wrapper to support copy
destructible semantic.
12. I moved every policy in specific namespace
13. I implemented standard pointer generators following to Andrei proposed
format.
14. More tests added and passed (require patch I posted yesterday)
15. Some minor changes

Things to do:

1. intrusive_ref_counted and some storage policies use new for allocation
purposes. Do we need custom allocators?
2. Move destructive copy semantic into smart_ptr subclass
3. add is_convertible checks into policies adaptors implementation to
implement convertibility checks for empty policies
4. Need to add support for existent boost smart_ptr
5. Did I miss something?

My current version is as usual here:
http://groups.yahoo.com/group/boost/files/Policy%20based%20smart_ptr/

I uploaded archived version that have all together.

I hope you did not get tired or bored reading this message.

Regards,

Gennadiy.