Re: [boost] Smart Pointers documentation/questions

From: Peter Dimov (pdimov_at_[hidden])
Date: 2002-09-16 09:17:57

From: "David Abrahams" <dave_at_[hidden]>
> 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 common requirements don't really apply to shared_ptr. Unfortunately I
don't have the time necessary to go over them in the detail they deserve;
since I'm left with the impression that only shared_ptr has the remote
chance to go into the C++ standard, I've been concentrating on
shared_ptr.htm. It's possible that I got the priorities wrong. ;-)

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

Documentation bug. i is std::ptrdiff_t to allow the implementation to detect
negative indices.

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

The "Introduction" section does not give definitions or authoritative
answers on how shared_ptr behaves. It's a "plain English" description of the
class. I'm not very good at the "plain English" descriptions so I've mainly
left is as-is. ;-)

> - 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 italicized/bracketed notes are mostly for my own use, although they
might be of interest to others as well. A dedicated "future directions"
section simply will not work. The only other option is for me to maintain a
separate copy of shared_ptr.htm for my own use, and this won't really be a
step forward for others that may be interested.

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

template<class T, class ThreadingModel = default_threading_model> class
shared_ptr;

for example.

> As such the note is pretty much useless unless you already
> have an idea what it's referring to.

Sorry. :-)

> - 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)?

shared_from_this is not yet official. It is supposed to be called only from
within a member function to obtain a shared_ptr from the this pointer, the
object must already be managed by a shared_ptr, and there may be other (not
yet determined) constraints (currently the constraint is that the object
must derive from counted_base.)

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

intrusive_ptr.html will hopefully be in the release after 1.29.

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

Yes, you are right. "The expression d2(p), where d2 is a copy of d, ..."
perhaps.

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

Not in the current implementation.

> - 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've never been good at intuitive naming.

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

Better safe than sorry.

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

I strongly believe that every time the standard uses C++ code to describe
effects, someone somewhere misinterprets this as required implementation. I
_think_ that reference count updates are not observable behavior even
without the note so the implementation is free to optimize, but I think that
the note should stay.

> - 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)
>     {
>     }

You mean *r.px? This is undefined behavior when r.px == 0, is it not?

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

Pass by value should be safe. shared_ptr is supposed to be as thread-safe as
an int.

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

You are right, the "all copies" language is imprecise, this applies to
shared_ptr.htm too. The meaning is "all pointers that share ownership" but
this is still not defined. This is something that I intend to fix. All
shared/weak pointers that are created from each other using a copy
constructor, a conversion constructor, make_shared, or shared_* casts, are
considered "copies".

Thank you for the comments.