[boost] Re: Extensible polymorphic pointer

From: David B. Held (dheld_at_[hidden])
Date: 2002-08-07 23:49:30

"Philippe A. Bouchard" <philippeb_at_[hidden]> wrote in message
news:aisoeh$hau$1_at_main.gmane.org...
> [...]
> Well this issue is based purely on the fact that an integer is always the
> size of a machine word; they will always be aligned with memory.
> Integers are defined this way in the standards.

Let's take a close look at your code.  Here is an excerpt from ptr_base:

 private:
  element_type * m_ptr;

  int & get_count() const
  {
   return * reinterpret_cast<int *>(m_ptr - len<int>() - len<int>());
  }

  int & get_type_id() const
  {
   return * reinterpret_cast<int *>(m_ptr - len<int>());
  }

Now, there appear to be numerous problems with this code.  First,
let's look at:

    m_ptr - len<int>()

Suppose that sizeof(int) == 4, sizeof(element_type) == 5, and that
m_ptr % 4 == 1, due to this size.  That would make the offset -20,
but the alignment would not be correct for int!

Furthermore, this code assumes that

  static int s_id;

or

struct __ptr_ctti
{
    static int s_size;
};

is located at (char*)m_ptr - 4.  But both of those objects are static, which
means you should only get one instance per class.  But if your program
contains more than one ptr_base<>, why on earth would we expect a
copy of a static object to exist in every instance??  This is very
confusing!

Let's take a look at your conversion/copy c'tor:

    template <typename _U>
    __ptr_base(_U * a_p = 0) : m_ptr(reinterpret_cast<_T *>(a_p))
    {
        s_rectify.reserve(__ptr_base<_U, __false_type>::s_id + 1);
        s_rectify[__ptr_base<_U, __false_type>::s_id] = int(static_cast<_T
*>((_U *) 1) - offset(1));

        get_count() = 1;
        get_type_id() = __ptr_base<_U, __false_type>::s_id;
    }

I notice that you try your hardest to stick to C++-style casts, except in
the
peculiar construction:

    int(static_cast<_T *>((_U *) 1) - offset(1));

Here you cast the constant value 1 to a U*.  Very creative.  Then you
subtract
one byte?  And cast back to a T*?  What on earth is this code supposed to
be doing?!?  Let's look at the one place it is called, and see if we can
bring
any sanity to this code.

   element_type * get() const
   {
       return m_ptr + s_rectify[get_type_id()];
   }

Suppose that sizeof(T) = 4 and sizeof(U) = 6.  That means our offset is
computed as 5?  Well, first of all, we have a problem.  Conversions
between pointers are only defined when the conversion results in a
properly aligned pointer.  In this case, it looks like we end up with:

    static_cast<T*>(5)

which, with any luck, wouldn't compile, because, among other reasons,
"5" is not a suitable alignment for T.  However, that is not the end of
our worries.  Suppose rectify returned 5 anyway, and we tried to get
the element_type*.  It looks to me that m_ptr is already what we want,
but what do I know?  Instead, we return m_ptr + 5.  What is that supposed
to point to?  I have no idea.

You say that this pointer is supposed to be faster than shared_ptr<>.
But your get() implementation, which is called by operator* (which I
assume is a fairly frequent operation), contains at least two adds
and two dereferences more than shared_ptr<>.  In fact, you couldn't
possibly get any faster than shared_ptr<>::operator*.  Also,
shared_ptr<>'s copy c'tor appears to have much less overhead than
your pointer.  I'm really having a hard time seeing where the speed
difference is, and the direction of that difference.

This line also bothers me:

        get_type_id() = __ptr_base<_U, __false_type>::s_id;

It's one thing to poke around where you don't belong, but here you
brazenly assign to whatever random address is returned by
get_type_id().  Have you tried compiling this code on a few different
compilers?  Have you tried making some test cases that make sure
this code really works properly?  The test code you uploaded does
not inspire a lot of confidence.

Dave