[boost] Re: smart_ptr implementation descussion topics

From: David B. Held (dheld_at_[hidden])
Date: 2002-05-06 08:21:29

"Gennadiy Rozental" <rogeeff_at_[hidden]> wrote in message
news:ab5j3c$oha$1_at_main.gmane.org...
> I uploaded my latest version into the vault area.

I'll take a close look at it later, but I hope that Andrei and others will
comment
on it when they have time.

> Topic 1: Storage and Ownership policy treatment
> [...]

Your method for Storage-Ownership interaction at the SmartPtr level is
something I thought about to "properly" implement the boost emulation
policies, but I didn't want to add a hack just for one set of policies.
However,
if it is agreed that the Ownership policy might in general want to see some
of the same things that the Storage policy does, I think your solution might
be reasonable.  One problem I had with this approach: the Ownership
policy only gets StoragePolicy::pointer_type (I think).  Whereas, some of
the policies might also want StoragePolicy::stored_type.  I'm not entirely
sure.  I need to review the code again.

> Now let's take a look onto release() function ( BTW I am calling it leak
> following Andrei proposition - I like it much more).

I guess it's ok to establish a precedent, since we can always provide an
alternative name for policies that need it.

> [...]
> Most wide usage if it is to prevent leaks at compile time using static
assert.

Hmm.  While I can see how that *might* be useful, I would be more greatly
persuaded if anyone actually had a *need* for this capability in a real
application.  After all, there are uncountable features we could provide,
but
that would make for a very fat smart pointer. ;)

> Topic 2: Destructive copy semantic support.
> [...]

I don't really use auto_ptr, so I'm afraid I can't add anything useful.

> Topic 3. Template template parameters
> [...] 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.

I thought about this too, but I think there is also a danger of
over-generalization.
I guess it all depends on how the scope of smart_ptr is defined.  Perhaps
Andrei will add some commentary to this.  He did grudgingly concede that
template template parameters could be dropped, but I'd rather see him say
definitively that they ought to be dropped.

> Not mentioning that they are not supported by many compilers; none of
> boost staff will work with it.

Yes, this is an interesting point; but also unfortunate, in my opinion.  I
think
template template parameters *are* useful in many situations, and not using
them simply relieves the pressure on compiler vendors.  There are always
ways to provide equivalent functionality, if a little bit more awkward (and
I
think that's the best kind of pressure, myself).

> 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 [...]

Maybe we should have both c'tors?  And how do you know that you won't
ever want to construct from stored_type?

> Topic 5: COW semantics
> [...]
> 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. [...]

I believe my implementation already does this.

> Topic 6: Templated comparison and order operation.
> Given the tradeoff I choose not to support template operators for now for
> generic resource.

Yeah, I'm not sure we could even provide sensible comparison operators for
generic resources.  Or whether most generic resources should even have
comparison operators.  And let's not forget that this is still SmartPtr
we're
talking about. ;)

> 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 accep
table
> requirement.

Obviously, smart_ptr relies on member templates a lot.  But I do believe it
is
possible to provide reasonable, albeit limited alternatives in the absence
of
member templates.  Of course, the trickiest part is the templated c'tors.  I
think it may be possible to add helper template functions to simulate
template
c'tors, but I haven't played around with it at all yet.  I think portability
is
probably a major concern for a smart pointer, given its wide usage.  Failing
to
support compilers with every reasonable effort doesn't seem like a way to
win friends in this game.

> 1. Since file became to big to efficiently manage I separated it into
> several independent modules. See at the end for the list. [...]

Yes, I considered this as well.  Some people don't like a lot of #includes,
for whatever reason.  Maybe with a header this big, it is preferred, though.

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

Sure.  Do you know where it is, offhand?

> 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

I thought about this, but was too lazy to do anything. ;)

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

Ok.

> 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

Maybe it should have both??

> [...]
> 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?

Yup.

> Looks like for some reason you are trying to combine storage and ownership
> policy in one bottle.

Indeed.  And that is how the shared_ptr family works.  In particular, the
count (which belongs in an Ownership policy) is sometimes initialized by
the raw pointer, to which Loki::SmartPtr's Ownership interface has no
access.  Secondly, it is lacking the types defined in StoragePolicy.
Ideally,
the Ownership policy would have a lot more visibility of the Storage policy.
Not sure what Andrei thinks about that, though.  It breaks orthogonality,
but
possibly in a way whose benefits outweigh the "dirtiness" of it.

> I admit that I did not tackle boost smart pointers yet but it seems fishy.

It *is* fishy. ;)  Good luck. ;)

> BTW you have a bug (I think) in the method unique implementation.

That's possible.

> 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.

I'll take a look at your implementation and comment later.

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

Umm...I'm sorry, but are you saying we *should* or *should not* be
concerned with multithreading?

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

I myself was thinking that smart_ptr.hpp should contain the "core policies"
that provide the most basic functionality.  Right now, I have the boost
emulation policies in a header called "boost_policies.hpp".  Perhaps there
could also be an "enhanced_policies" "extended_policies" or
"auxiliary_policies" header for things that are generally useful, but not as
"pure" as the core policies.

> 12 I finally implemented ref_linked. Loki version has a serious bug (I
> think). Try the following [...]
> See my version to compare. [...]

I will.

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

I guess I'm just worried about overgeneralization.  The boost smart pointers
have one checking policy, Loki::SmartPtr has 6, and I've never heard anyone
complain that either one is lacking in checking abilities.  Do we really
*need*
the combinatorial power that gives us 243 checking policies??  It seems a
bit much, IMO.

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

The complement to #include <pshpack?.h>.  It is a Win32 packing pragma,
wrapped in a header for portability.  I needed it to get the size down to
reasonable, sane, levels.  Note that gcc doesn't have an equivalent, but
doesn't
need one.  It just "does the right thing". ;)

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

For convenience, maybe you should move your folder into the smart_pointers
folder that Beman created.  That way, people can see all the different
implementations in one, easy-to-reach location.

Dave