[boost] Smart Pointers documentation/questions

From: David Abrahams (dave_at_[hidden])
Date: 2002-09-14 09:36:23

Hi,

In preparation for giving a class, I've recently been going through the
smart pointer library documentation. I came up with these
comments/questions:

[Peter, the class is on Wed, so if you could see your way to responding to
the few *questions* (not comments) below while I'm preparing the material
this weekend, it would be very much appreciated!]

- The smart pointer common requirements documentation mentions that T must
be complete "at points of smart pointer instantiation". I think it might be
helpful to users if we briefly explain what that means (will "at the point
when the compiler requires knowledge of any of the smart pointer's members
or bases" do?). It also might be /very/ nice to mention here that
shared_ptr doesn't have these restrictions, since it's the most popular and
general-purpose of the smart pointers.

- The scoped_ptr doc says: "It cannot be used in C++ Standard Library
containers. See shared_ptr or std::auto_ptr if scoped_ptr does not meet
your needs". This is misleading, since std::auto_ptr can't be used in
stdlib containers. Worse, we might be leading users into danger: a user's
library implementation may in fact allow std::auto_ptr in containers as was
common before support for auto_ptr_ref was widespread.

- The scoped_ptr destructor doc says "Deletes the object pointed to by the
stored pointer" and then notes "that delete on a pointer with a value of 0
is harmless":
    1. If the pointer is 0 there *is* no object pointed to
    2. Why are we telling the users about deleting 0 pointers? It's an
implementation detail whether the library checks the pointer before calling
delete.
Suggestion: "Deletes the object pointed to by the stored pointer, if any",
or better yet, "Effects: delete this->get()"

- The FAQ for scoped_ptr is a little odd: signaling intent and transferring
ownership are orthogonal. You can use std::auto_ptr<> to signal the intent
to transfer ownership, after all. Maybe it should say: "When reading source
code, it is valuable to be able to draw conclusions about program behavior
based on the types being used. If scoped_ptr had a release() member, it
would become possible to transfer ownership out of its scope, weakening its
role as a way of limiting resource lifetime to a given scope. Use
std::auto_ptr<> where transfer-of-ownership is required"

- The shared_array docs say: "It cannot correctly hold a pointer to a
single object". Not strictly correct, I think:

    scoped_array<int> x(new int[1]);

Why not just say that it can only hold pointers to objects allocated with
the array form of operator new?

- scoped_array::operator[] includes the clause ", or if i is less than 0",
which is nonsense since i is std::size_t

- shared_ptr: there's the parenthesized note "(Dynamically allocated
objects are allocated with the C++ new expression.)". This is wierd.

    - In general, it's not true - there are lots of ways to
      dynamically allocate objects.

    - If the intent is to make a formal definition of the term
      "Dynamically Allocated", a parenthesized note is an odd
      way to do it

    - Anyway, doesn't restricting the meaning of D.A. that way
      conflict with the library's own ability to use custom
      deleters supplied by the user?

- "shared_ptr will not work correctly with cyclic data structures" is a
little too strong, isn't it? It works fine if you have well-expressed
ownership relationships. For example, a tree with raw or weak parent
pointers works just fine.

- The many italicized/bracketed notes in the documentation tend to distract
from readability if you're just trying to understand what the code does
now. It would probably be better to have a "future directions" section
which covers what shared_ptr will look like some day, and the rationale
behind those changes. Having them concentrated in one place will help to
see how most of these points are closely related.

- The first such note mentions that an additional template parameter could
be used to detect ODR violations. Could you explain how that works? I
vaguely remember something like this in mailing list discussion but I can't
see it offhand. As such the note is pretty much useless unless you already
have an idea what it's referring to. I'd also de-emphasize the argument
about template template parameters, since they're not all that useful
anyway ;-)

- A parenthesized note mentions "A free function shared_from_this" which I
don't see documented anywhere else. Why "from_this"? Is there something
special about a this pointer in this case? And how is that different from
shared_ptr<T>(x)?

- It also mentions intrusive_ptr, which isn't documented anywhere else. A
separate section on experimental support for intrusive pointers would be
better. BTW, how's the experiment going?

- The doc format results in the documented signatures being so
de-emphasized that they're hard to see with all that bold text around. Bill
Kempf's CSS adds a blue background under code which helps it stand out.

- template<typename Y, typename D> shared_ptr(Y * p, D d);

    Requirements: p must be convertible to T *. The copy constructor
    and destructor of D must not throw. The expression d(p) must be
    well-formed, must not invoke undefined behavior, and must not
    throw exceptions.

This isn't quite right. You should add that D is CopyConstructible.
Otherwise, a copy of d might not be equivalent to d, and D(d)(p) might
invoke undefined behavior.

BTW, is there a requirement that D's operator() be const?

- I think "use_count_is_zero" is an unintuitive name for that exception.
IMO it should be something like "no_shared_object" or
"shared_object_deleted"... or something.

- I wonder whether the by-reference conversion from std::auto_ptr is worth
it. It's hard to think of cases where the strong guarantee is useful for
this constructor. Of course, you can't take it away without breaking
backward compatibility...

- The assignment operator docs say: "Notes: The implementation is free to
meet the effects (and the implied guarantees) via different means, without
creating a temporary." How would the implementation difference be
detectable by the user? If there's no way to detect it, I strongly believe
this note and accompanying example are just a distraction.

- Also "swap is defined in the same namespace as shared_ptr as this is
currently...". You might give a reference to
http://www.gotw.ca/publications/mill08.htm to explain why the global
namespace won't work.

- Wouldn't this be a better implementation?

    template<typename Y>
    shared_ptr(shared_ptr<Y> const & r, detail::polymorphic_cast_tag)
        : px(&dynamic_cast<element_type&>(r.px)), pn(r.pn)
    {
    }

- About shared_ptr thread safety: it might be very useful to describe how
shared_ptrs can be safely passed across thread boundaries. I'm a little
unclear on how that can work, actually. Passing by-reference is clearly not
safe, since the initiating thread may destroy the pointer before the thread
being started can touch it. Suppose you pass by-value, then? It seems to me
that whether that's safe depends very much on the thread implementation. At
some point in that process, it seems to me, both threads may still be
holding references to the same shared_ptr object.

- In the weak pointer docs you refer to "the use count of all copies",
begging the question, "copies of what?". Can't you make multiple weak_ptrs
from a shared_ptr without every copying a weak_ptr?

-----------------------------------------------------------
           David Abrahams * Boost Consulting
dave_at_[hidden] * http://www.boost-consulting.com