Re: [boost] Smart Pointers documentation/questions

From: David Abrahams (dave_at_[hidden])
Date: 2002-09-16 09:44:34

Peter Dimov wrote:
> From: "David Abrahams" <dave_at_[hidden]>
>
> > - 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

That's why I've just gone over them for you.

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

It took some effort to come up with these comments/suggestions. How
can we ensure that it wasn't wasted? Shall I make edits to the docs
myself?

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

I don't understand from your remarks above whether you intend to fix
things like this or not.

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

It's easy to fix: just replace the parenthesized remark with

     for example, objects allocated with a C++ <i>new-expression</i>

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

Why not?

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

You could keep links in the main doc to the future directions doc.
It's hard to keep track of how the smart pointers actually work in the
current state.

> > - 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 long as you're gonna have that note, a little code
snippet like the one above, and a note: "where default_threading_model
is a configuration-specific typedef" would help clarify.

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

Again I can't tell if you're planning to address any of these things
I'm mentioning or if you're just deflecting. Boost smart pointers are
one of our most important libraries. We really need to make sure the
documentation stays understandable.

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

Why? What's special about being 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.)

Understood.

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

Cool.

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

I think it's easier to say that D is CopyConstructible, but you're the
author.

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

Can we have a discussion of what a good name might be before this gets
too solid? Or is it already too solid?

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

Agreed.

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

You could get the same effect by just adding an explicit "as-if".

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

Duh. You're right.

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

I know that it's /supposed/ to be. The question has to do with the
mechanism for starting threads. I have *no* idea how this stuff works
in practice, so there may be no worry. However, imagine that it uses
something like the way I remember fork() works. If I recall correctly,
when you fork() you may get a second copy of your function using the
same memory image. If the shared_ptr was passed by-value into the
function calling fork, there would be a race condition because one
copy of the function would be trying to destroy the shared_ptr on
exit, while the other one would be trying to read it.

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