Merge two internal classes of QSharedPointer and de-virtualise them

The two classes are QtSharedPointer::ExternalRefCountData and
ExternalRefCountWithDestroyFn. The split existed because of what Qt
4.5 did before custom deleters existed: the ExternalRefCountData class
was a virtual class that contained a destroy() virtual, which was in
charge of deleting the data or returning false if it didn't.

Turns out that virtual classes was a mistake. This commit
de-virtualises them -- we couldn't do it in Qt 4 because of binary
compatibility. This saves us one pointer-size in the size of the
private, plus the fact that fewer symbols are required (there is no
virtual table to be initialised).

Additionaly, since a deleter is always stored with the reference
count, we don't need the split between the two classes anymore.

Change-Id: I1cd9400561dcee089a406a57bd856b1730f18afc
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

2 files changed, 69 insertions, 116 deletions

diff --git a/src/corelib/tools/qsharedpointer.cpp b/src/corelib/tools/qsharedpointer.cpp
index ed8f5c9c0c..8ca153f433 100644
--- a/src/corelib/tools/qsharedpointer.cpp
+++ b/src/corelib/tools/qsharedpointer.cpp
@@ -163,18 +163,18 @@
     complete object. See the \tt virtualBaseDifferentPointers autotest for
     this problem.
 
-    The d pointer is of type QtSharedPointer::ExternalRefCountData for simple
-    QSharedPointer objects, but could be of a derived type in some cases. It
-    is basically a reference-counted reference-counter.
+    The d pointer is a pointer to QtSharedPointer::ExternalRefCountData, but it
+    always points to one of the two classes derived from ExternalRefCountData.
 
     \section2 d-pointer
     \section3 QtSharedPointer::ExternalRefCountData
 
-    This class is basically a reference-counted reference-counter. It has two
-    members: \tt strongref and \tt weakref. The strong reference counter is
-    controlling the lifetime of the object tracked by QSharedPointer. a
-    positive value indicates that the object is alive. It's also the number
-    of QSharedObject instances that are attached to this Data.
+    It is basically a reference-counted reference-counter plus a pointer to the
+    function to be used to delete the pointer. It has three members: \tt
+    strongref, \tt weakref, and \tt destroyer. The strong reference counter is
+    controlling the lifetime of the object tracked by QSharedPointer. A
+    positive value indicates that the object is alive. It's also the number of
+    QSharedObject instances that are attached to this Data.
 
     When the strong reference count decreases to zero, the object is deleted
     (see below for information on custom deleters). The strong reference
@@ -189,40 +189,16 @@
     the object tracked derives from QObject, this number is increased by 1,
     since QObjectPrivate tracks it too).
 
-    ExternalRefCountData is a virtual class: it has a virtual destructor and
-    a virtual destroy() function. The destroy() function is supposed to
-    delete the object being tracked and return true if it does so. Otherwise,
-    it returns false to indicate that the caller must simply call delete.
-    This allows the normal use-case of QSharedPointer without custom deleters
-    to use only one 12- or 16-byte (depending on whether it's a 32- or 64-bit
-    architecture) external descriptor structure, without paying the price for
-    the custom deleter that it isn't using.
-
-    \section3 QtSharedPointer::ExternalRefCountDataWithDestroyFn
-
-    This class is not used directly, per se. It only exists to enable the two
-    classes that derive from it. It adds one member variable, which is a
-    pointer to a function (which returns void and takes an
-    ExternalRefCountData* as a parameter). It also overrides the destroy()
-    function: it calls that function pointer with \tt this as parameter, and
-    returns true.
-
-    That means when ExternalRefCountDataWithDestroyFn is used, the \tt
-    destroyer field must be set to a valid function that \b will delete the
-    object tracked.
-
-    This class also adds an operator delete function to ensure that it simply
-    calls the global operator delete. That should be the behaviour in all
-    compilers already, but to be on the safe side, this class ensures that no
-    funny business happens.
-
-    On a 32-bit architecture, this class is 16 bytes in size, whereas it's 24
-    bytes on 64-bit. (On Itanium where function pointers contain the global
-    pointer, it can be 32 bytes).
+    The third member is a pointer to the function that is used to delete the
+    pointer being tracked. That happens when the destroy() function is called.
+
+    The size of this class is the size of the two atomic ints plus the size of
+    a pointer. On 32-bit architectures, that's 12 bytes, whereas on 64-bit ones
+    it's 16 bytes. There is no padding.
 
     \section3 QtSharedPointer::ExternalRefCountWithCustomDeleter
 
-    This class derives from ExternalRefCountDataWithDestroyFn and is a
+    This class derives from ExternalRefCountData and is a
     template class. As template parameters, it has the type of the pointer
     being tracked (\tt T) and a \tt Deleter, which is anything. It adds two
     fields to its parent class, matching those template parameters: a member
@@ -236,31 +212,26 @@
     the deleter in the generic case.
 
     This class is never instantiated directly: the constructors and
-    destructor are private. Only the create() function may be called to
-    return an object of this type. See below for construction details.
-
-    The size of this class depends on the size of \tt Deleter. If it's an
-    empty functor (i.e., no members), ABIs generally assign it the size of 1.
-    But given that it's followed by a pointer, up to 3 or 7 padding bytes may
-    be inserted: in that case, the size of this class is 16+4+4 = 24 bytes on
-    32-bit architectures, or 24+8+8 = 40 bytes on 64-bit architectures (48
-    bytes on Itanium with global pointers stored). If \tt Deleter is a
-    function pointer, the size should be the same as the empty structure
-    case, except for Itanium where it may be 56 bytes due to another global
-    pointer. If \tt Deleter is a pointer to a member function (PMF), the size
-    will be even bigger and will depend on the ABI. For architectures using
-    the Itanium C++ ABI, a PMF is twice the size of a normal pointer, or 24
-    bytes on Itanium itself. In that case, the size of this structure will be
-    16+8+4 = 28 bytes on 32-bit architectures, 24+16+8 = 48 bytes on 64-bit,
-    and 32+24+8 = 64 bytes on Itanium.
-
-    (Values for Itanium consider an LP64 architecture; for ILP32, pointers
-    are 32-bit in length, function pointers are 64-bit and PMF are 96-bit, so
-    the sizes are slightly less)
+    destructor are private and, in C++11, deleted. Only the create() function
+    may be called to return an object of this type. See below for construction
+    details.
+
+    The size of this class depends on the size of \tt Deleter. If it's an empty
+    functor (i.e., no members), ABIs generally assign it the size of 1. But
+    given that it's followed by a pointer, padding bytes may be inserted so
+    that the alignment of the class and of the pointer are correct. In that
+    case, the size of this class is 12+4+4 = 20 bytes on 32-bit architectures,
+    or 16+8+8 = 40 bytes on 64-bit architectures. If \tt Deleter is a function
+    pointer, the size should be the same as the empty structure case. If \tt
+    Deleter is a pointer to a member function (PMF), the size will be bigger
+    and will depend on the ABI. For architectures using the Itanium C++ ABI, a
+    PMF is twice the size of a normal pointer. In that case, the size of this
+    structure will be 12+8+4 = 24 bytes on 32-bit architectures, 16+16+8 = 40
+    bytes on 64-bit ones.
 
     \section3 QtSharedPointer::ExternalRefCountWithContiguousData
 
-    This class also derives from ExternalRefCountDataWithDestroyFn and it is
+    This class also derives from ExternalRefCountData and it is
     also a template class. The template parameter is the type \tt T of the
     class which QSharedPointer tracks. It adds only one member to its parent,
     which is of type \tt T (the actual type, not a pointer to it).
@@ -273,8 +244,8 @@
 
     Like ExternalRefCountWithCustomDeleter, this class is never instantiated
     directly. This class also provides a create() member that returns the
-    pointer, and hides its constructors and destructor. (With C++0x, we'd
-    delete them).
+    pointer, and hides its constructors and destructor. With C++11, they're
+    deleted.
 
     The size of this class depends on the size of \tt T.
 
@@ -287,12 +258,8 @@
 
     Instead of instantiating the class by the normal way, the create() method
     calls \tt{operator new} directly with the size of the class, then calls
-    the parent class's constructor only (ExternalRefCountDataWithDestroyFn).
-    This ensures that the inherited members are initialised properly, as well
-    as the virtual table pointer, which must point to
-    ExternalRefCountDataWithDestroyFn's virtual table. That way, we also
-    ensure that the virtual destructor being called is
-    ExternalRefCountDataWithDestroyFn's.
+    the parent class's constructor only (that is, ExternalRefCountData's constructor).
+    This ensures that the inherited members are initialised properly.
 
     After initialising the base class, the
     ExternalRefCountWithCustomDeleter::create() function initialises the new
@@ -305,7 +272,7 @@
 
     When initialising the parent class, the create() functions pass the
     address of the static deleter() member function. That is, when the
-    virtual destroy() is called by QSharedPointer, the deleter() functions
+    destroy() function is called by QSharedPointer, the deleter() functions
     are called instead. These functions static_cast the ExternalRefCountData*
     parameter to their own type and execute their deletion: for the
     ExternalRefCountWithCustomDeleter::deleter() case, it runs the user's
@@ -313,12 +280,7 @@
     ExternalRefCountWithContiguousData::deleter, it simply calls the \tt T
     destructor directly.
 
-    By not calling the constructor of the derived classes, we avoid
-    instantiating their virtual tables. Since these classes are
-    template-based, there would be one virtual table per \tt T and \tt
-    Deleter type. (This is what Qt 4.5 did.)
-
-    Instead, only one non-inline function is required per template, which is
+    Only one non-inline function is required per template, which is
     the deleter() static member. All the other functions can be inlined.
     What's more, the address of deleter() is calculated only in code, which
     can be resolved at link-time if the linker can determine that the
@@ -326,11 +288,6 @@
     classes are not exported, that is the case for Windows or for builds with
     \tt{-fvisibility=hidden}).
 
-    In contrast, a virtual table would require at least 3 relocations to be
-    resolved at module load-time, per module where these classes are used.
-    (In the Itanium C++ ABI, there would be more relocations, due to the
-    RTTI)
-
     \section3 Modifications due to pointer-tracking
 
     To ensure that pointers created with pointer-tracking enabled get
@@ -340,9 +297,9 @@
 
     When ExternalRefCountWithCustomDeleter or
     ExternalRefCountWithContiguousData are used, their create() functions
-    will set the ExternalRefCountDataWithDestroyFn::destroyer function
+    will set the ExternalRefCountData::destroyer function
     pointer to safetyCheckDeleter() instead. These static member functions
-    simply call internalSafetyCheckRemove2() before passing control to the
+    simply call internalSafetyCheckRemove() before passing control to the
     normal deleter() function.
 
     If neither custom deleter nor QSharedPointer::create() are used, then
diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index 289473514e..942bbadcce 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -171,21 +171,27 @@ namespace QtSharedPointer {
     // reference counter, and it tracks the lifetime of the pointer itself.
     // "weakref" is the outer reference counter and it tracks the lifetime of
     // the ExternalRefCountData object.
+    //
+    // The deleter is stored in the destroyer member and is always a pointer to
+    // a static function in ExternalRefCountWithCustomDeleter or in
+    // ExternalRefCountWithContiguousData
     struct ExternalRefCountData
     {
+        typedef void (*DestroyerFn)(ExternalRefCountData *);
         QBasicAtomicInt weakref;
         QBasicAtomicInt strongref;
+        DestroyerFn destroyer;
 
-        inline ExternalRefCountData()
+        inline ExternalRefCountData(DestroyerFn d)
+            : destroyer(d)
         {
             strongref.store(1);
             weakref.store(1);
         }
         inline ExternalRefCountData(Qt::Initialization) { }
-        virtual inline ~ExternalRefCountData() { Q_ASSERT(!weakref.load()); Q_ASSERT(strongref.load() <= 0); }
+        ~ExternalRefCountData() { Q_ASSERT(!weakref.load()); Q_ASSERT(strongref.load() <= 0); }
 
-        // overridden by derived classes
-        virtual inline void destroy() { }
+        void destroy() { destroyer(this); }
 
 #ifndef QT_NO_QOBJECT
         Q_CORE_EXPORT static ExternalRefCountData *getAndRef(const QObject *);
@@ -194,34 +200,21 @@ namespace QtSharedPointer {
 #endif
         inline void checkQObjectShared(...) { }
         inline void setQObjectShared(...) { }
-    };
-    // sizeof(ExternalRefCount) = 12 (32-bit) / 16 (64-bit)
-
-    // This class extends ExternalRefCountData with a pointer
-    // to a function, which is called by the destroy() function.
-    struct ExternalRefCountWithDestroyFn: public ExternalRefCountData
-    {
-        typedef void (*DestroyerFn)(ExternalRefCountData *);
-        DestroyerFn destroyer;
-
-        inline ExternalRefCountWithDestroyFn(DestroyerFn d)
-            : destroyer(d)
-        { }
 
-        inline void destroy() { destroyer(this); }
         inline void operator delete(void *ptr) { ::operator delete(ptr); }
         inline void operator delete(void *, void *) { }
     };
-    // sizeof(ExternalRefCountWithDestroyFn) = 16 (32-bit) / 24 (64-bit)
+    // sizeof(ExternalRefCountData) = 12 (32-bit) / 16 (64-bit)
 
-    // This class extends ExternalRefCountWithDestroyFn and implements
+    // This class extends ExternalRefCountData and implements
     // the static function that deletes the object. The pointer and the
-    // custom deleter are kept in the "extra" member.
+    // custom deleter are kept in the "extra" member so we can construct
+    // and destruct it independently of the full structure.
     template <class T, typename Deleter>
-    struct ExternalRefCountWithCustomDeleter: public ExternalRefCountWithDestroyFn
+    struct ExternalRefCountWithCustomDeleter: public ExternalRefCountData
     {
         typedef ExternalRefCountWithCustomDeleter Self;
-        typedef ExternalRefCountWithDestroyFn BaseClass;
+        typedef ExternalRefCountData BaseClass;
 
         struct CustomDeleter
         {
@@ -231,7 +224,8 @@ namespace QtSharedPointer {
             inline CustomDeleter(T *p, Deleter d) : deleter(d), ptr(p) {}
         };
         CustomDeleter extra;
-        // sizeof(CustomDeleter) = sizeof(Deleter) + sizeof(void*)
+        // sizeof(CustomDeleter) = sizeof(Deleter) + sizeof(void*) + padding
+        // for Deleter = stateless functor: 8 (32-bit) / 16 (64-bit) due to padding
         // for Deleter = function pointer:  8 (32-bit) / 16 (64-bit)
         // for Deleter = PMF: 12 (32-bit) / 24 (64-bit)  (GCC)
 
@@ -265,19 +259,20 @@ namespace QtSharedPointer {
             return d;
         }
     private:
-        // prevent construction and the emission of virtual symbols
-        ExternalRefCountWithCustomDeleter();
-        ~ExternalRefCountWithCustomDeleter();
+        // prevent construction
+        ExternalRefCountWithCustomDeleter() Q_DECL_EQ_DELETE;
+        ~ExternalRefCountWithCustomDeleter() Q_DECL_EQ_DELETE;
+        Q_DISABLE_COPY(ExternalRefCountWithCustomDeleter)
     };
 
-    // This class extends ExternalRefCountWithDestroyFn and adds a "T"
+    // This class extends ExternalRefCountData and adds a "T"
     // member. That way, when the create() function is called, we allocate
     // memory for both QSharedPointer's d-pointer and the actual object being
     // tracked.
     template <class T>
-    struct ExternalRefCountWithContiguousData: public ExternalRefCountWithDestroyFn
+    struct ExternalRefCountWithContiguousData: public ExternalRefCountData
     {
-        typedef ExternalRefCountWithDestroyFn Parent;
+        typedef ExternalRefCountData Parent;
         T data;
 
         static void deleter(ExternalRefCountData *self)
@@ -311,9 +306,10 @@ namespace QtSharedPointer {
         }
 
     private:
-        // prevent construction and the emission of virtual symbols
-        ExternalRefCountWithContiguousData();
-        ~ExternalRefCountWithContiguousData();
+        // prevent construction
+        ExternalRefCountWithContiguousData() Q_DECL_EQ_DELETE;
+        ~ExternalRefCountWithContiguousData() Q_DECL_EQ_DELETE;
+        Q_DISABLE_COPY(ExternalRefCountWithContiguousData)
     };
 
     // This is the main body of QSharedPointer. It implements the