QQmlRefCount: de-virtualize dtor

Now that QQmlRefCounted ensures no-one uses ~QQmlRefCount() directly
anymore, we can make it non-virtual, shrinking the
sizeof(QQmlRefCount) from 8/16 to 4/4 bytes (32/64 bit platforms).

This requires moving the release() function from QQmlRefCount down
into QQmlRefCounted<T>, and static_cast'ing *this to T before calling
delete.

We need to be careful, of course, that no derived class relied on the
implied virtualness of ~QQmlRefCount() making their dtors virtual, so
require that all classes that use QQmlRefCounted are either final or
have a virtual destructor.

Update the toolsupport test and the TypeInformationVersion, as
sizeof(QQmlRefCount) was one the items checked.

Fixes: QTBUG-114817
Change-Id: I69afd36ec5b63313842c1438e0244503603ed96f
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

20 files changed, 42 insertions, 28 deletions

diff --git a/src/qml/debugger/qqmldebug.cpp b/src/qml/debugger/qqmldebug.cpp
index 19fee0c315..b9d984db1d 100644
--- a/src/qml/debugger/qqmldebug.cpp
+++ b/src/qml/debugger/qqmldebug.cpp
@@ -175,7 +175,7 @@ quintptr Q_QML_EXPORT qtDeclarativeHookData[] = {
     // TypeInformationVersion, an integral value, bumped whenever private
     // object sizes or member offsets that are used in Qt Creator's
     // data structure "pretty printing" change.
-    3,
+    4,
 
     // Version of the cache data.
     QV4_DATA_STRUCTURE_VERSION
diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 3688c00861..bf5ccc08d2 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -71,6 +71,7 @@ class Q_QML_PRIVATE_EXPORT ExecutableCompilationUnit final
 {
     Q_DISABLE_COPY_MOVE(ExecutableCompilationUnit)
 public:
+    friend class QQmlRefCounted<ExecutableCompilationUnit>;
     friend class QQmlRefPointer<ExecutableCompilationUnit>;
 
     static QQmlRefPointer<ExecutableCompilationUnit> create(
diff --git a/src/qml/qml/ftw/qqmlrefcount_p.h b/src/qml/qml/ftw/qqmlrefcount_p.h
index 2914e6279c..8cd1cbdaf1 100644
--- a/src/qml/qml/ftw/qqmlrefcount_p.h
+++ b/src/qml/qml/ftw/qqmlrefcount_p.h
@@ -30,11 +30,10 @@ class Q_QML_PRIVATE_EXPORT QQmlRefCount
 public:
     inline QQmlRefCount();
     inline void addref() const;
-    inline void release() const;
     inline int count() const;
 
 private:
-    inline virtual ~QQmlRefCount();
+    inline ~QQmlRefCount();
     template <typename T> friend class QQmlRefCounted;
 
 private:
@@ -44,8 +43,10 @@ private:
 template <typename T>
 class QQmlRefCounted : public QQmlRefCount
 {
+public:
+    inline void release() const;
 protected:
-    ~QQmlRefCounted() = default;
+    inline ~QQmlRefCounted();
 };
 
 template<class T>
@@ -130,11 +131,22 @@ void QQmlRefCount::addref() const
     refCount.ref();
 }
 
-void QQmlRefCount::release() const
+template <typename T>
+void QQmlRefCounted<T>::release() const
 {
+    static_assert(std::is_base_of_v<QQmlRefCounted, T>,
+                  "QQmlRefCounted<T> must be a base of T (CRTP)");
     Q_ASSERT(refCount.loadRelaxed() > 0);
     if (!refCount.deref())
-        delete this;
+        delete static_cast<const T *>(this);
+}
+
+template <typename T>
+QQmlRefCounted<T>::~QQmlRefCounted()
+{
+    static_assert(std::is_final_v<T> || std::has_virtual_destructor_v<T>,
+                  "T must either be marked final or have a virtual dtor, "
+                  "lest release() runs into UB.");
 }
 
 int QQmlRefCount::count() const
diff --git a/src/qml/qml/qqmlboundsignal_p.h b/src/qml/qml/qqmlboundsignal_p.h
index d33ae82e7b..ebb6c7e532 100644
--- a/src/qml/qml/qqmlboundsignal_p.h
+++ b/src/qml/qml/qqmlboundsignal_p.h
@@ -24,7 +24,7 @@
 
 QT_BEGIN_NAMESPACE
 
-class Q_QML_PRIVATE_EXPORT QQmlBoundSignalExpression
+class Q_QML_PRIVATE_EXPORT QQmlBoundSignalExpression final
     : public QQmlJavaScriptExpression,
       public QQmlRefCounted<QQmlBoundSignalExpression>
 {
diff --git a/src/qml/qml/qqmldatablob_p.h b/src/qml/qml/qqmldatablob_p.h
index 95b535eabf..fe4b81db18 100644
--- a/src/qml/qml/qqmldatablob_p.h
+++ b/src/qml/qml/qqmldatablob_p.h
@@ -56,7 +56,7 @@ public:
     };
 
     QQmlDataBlob(const QUrl &, Type, QQmlTypeLoader* manager);
-    ~QQmlDataBlob() override;
+    virtual ~QQmlDataBlob();
 
     void startLoading();
 
diff --git a/src/qml/qml/qqmlimport_p.h b/src/qml/qml/qqmlimport_p.h
index 6ee60ef8bb..f827d69b02 100644
--- a/src/qml/qml/qqmlimport_p.h
+++ b/src/qml/qml/qqmlimport_p.h
@@ -115,7 +115,7 @@ public:
     }
 };
 
-class Q_QML_PRIVATE_EXPORT QQmlImports : public QQmlRefCounted<QQmlImports>
+class Q_QML_PRIVATE_EXPORT QQmlImports final : public QQmlRefCounted<QQmlImports>
 {
     Q_DISABLE_COPY_MOVE(QQmlImports)
 public:
diff --git a/src/qml/qml/qqmlobjectcreator_p.h b/src/qml/qml/qqmlobjectcreator_p.h
index b6a071f383..0a2f1ecda9 100644
--- a/src/qml/qml/qqmlobjectcreator_p.h
+++ b/src/qml/qml/qqmlobjectcreator_p.h
@@ -84,7 +84,7 @@ struct DeferredQPropertyBinding {
     QUntypedPropertyBinding binding;
 };
 
-struct QQmlObjectCreatorSharedState : QQmlRefCounted<QQmlObjectCreatorSharedState>
+struct QQmlObjectCreatorSharedState final : QQmlRefCounted<QQmlObjectCreatorSharedState>
 {
     QQmlRefPointer<QQmlContextData> rootContext;
     QQmlRefPointer<QQmlContextData> creationContext;
diff --git a/src/qml/qml/qqmlopenmetaobject_p.h b/src/qml/qml/qqmlopenmetaobject_p.h
index eb2532b5d3..5dcb8e9a86 100644
--- a/src/qml/qml/qqmlopenmetaobject_p.h
+++ b/src/qml/qml/qqmlopenmetaobject_p.h
@@ -28,12 +28,12 @@ QT_BEGIN_NAMESPACE
 class QQmlEngine;
 class QMetaPropertyBuilder;
 class QQmlOpenMetaObjectTypePrivate;
-class Q_QML_PRIVATE_EXPORT QQmlOpenMetaObjectType
+class Q_QML_PRIVATE_EXPORT QQmlOpenMetaObjectType final
     : public QQmlRefCounted<QQmlOpenMetaObjectType>
 {
 public:
     QQmlOpenMetaObjectType(const QMetaObject *base);
-    ~QQmlOpenMetaObjectType() override;
+    ~QQmlOpenMetaObjectType();
 
     void createProperties(const QVector<QByteArray> &names);
     int createProperty(const QByteArray &name);
diff --git a/src/qml/qml/qqmlproperty_p.h b/src/qml/qml/qqmlproperty_p.h
index 2ba8304f34..da1b7693e3 100644
--- a/src/qml/qml/qqmlproperty_p.h
+++ b/src/qml/qml/qqmlproperty_p.h
@@ -35,7 +35,7 @@ class QQmlMetaObject;
 class QQmlAbstractBinding;
 class QQmlBoundSignalExpression;
 
-class Q_QML_PRIVATE_EXPORT QQmlPropertyPrivate : public QQmlRefCounted<QQmlPropertyPrivate>
+class Q_QML_PRIVATE_EXPORT QQmlPropertyPrivate final : public QQmlRefCounted<QQmlPropertyPrivate>
 {
 public:
     enum class InitFlag {
diff --git a/src/qml/qml/qqmlpropertycache_p.h b/src/qml/qml/qqmlpropertycache_p.h
index 0f44533ef7..b261d1caca 100644
--- a/src/qml/qml/qqmlpropertycache_p.h
+++ b/src/qml/qml/qqmlpropertycache_p.h
@@ -105,7 +105,7 @@ private:
         Shared = 1
     };
 
-    struct SharedHolder : public QQmlRefCounted<SharedHolder>
+    struct SharedHolder final : public QQmlRefCounted<SharedHolder>
     {
         Q_DISABLE_COPY_MOVE(SharedHolder)
         SharedHolder(QMetaObject *shared) : metaObject(shared) {}
@@ -116,7 +116,7 @@ private:
     mutable QBasicAtomicInteger<quintptr> d = 0;
 };
 
-class Q_QML_PRIVATE_EXPORT QQmlPropertyCache
+class Q_QML_PRIVATE_EXPORT QQmlPropertyCache final
     : public QQmlRefCounted<QQmlPropertyCache>
 {
 public:
@@ -136,7 +136,7 @@ public:
             const QMetaObject *, QTypeRevision metaObjectRevision = QTypeRevision::zero());
 
     QQmlPropertyCache() = default;
-    ~QQmlPropertyCache() override;
+    ~QQmlPropertyCache();
 
     void update(const QMetaObject *);
     void invalidate(const QMetaObject *);
diff --git a/src/qml/qml/qqmlscriptdata_p.h b/src/qml/qml/qqmlscriptdata_p.h
index d328e36474..4e54083395 100644
--- a/src/qml/qml/qqmlscriptdata_p.h
+++ b/src/qml/qml/qqmlscriptdata_p.h
@@ -28,7 +28,7 @@ QT_BEGIN_NAMESPACE
 class QQmlTypeNameCache;
 class QQmlContextData;
 
-class Q_AUTOTEST_EXPORT QQmlScriptData : public QQmlRefCounted<QQmlScriptData>
+class Q_AUTOTEST_EXPORT QQmlScriptData final : public QQmlRefCounted<QQmlScriptData>
 {
 private:
     friend class QQmlTypeLoader;
diff --git a/src/qml/qml/qqmltype_p_p.h b/src/qml/qml/qqmltype_p_p.h
index ab73da7a6b..73567b5a22 100644
--- a/src/qml/qml/qqmltype_p_p.h
+++ b/src/qml/qml/qqmltype_p_p.h
@@ -26,7 +26,7 @@
 
 QT_BEGIN_NAMESPACE
 
-class QQmlTypePrivate : public QQmlRefCounted<QQmlTypePrivate>
+class QQmlTypePrivate final : public QQmlRefCounted<QQmlTypePrivate>
 {
     Q_DISABLE_COPY_MOVE(QQmlTypePrivate)
 public:
@@ -145,7 +145,8 @@ public:
     void setName(const QString &uri, const QString &element);
 
 private:
-    ~QQmlTypePrivate() override;
+    ~QQmlTypePrivate();
+    friend class QQmlRefCounted<QQmlTypePrivate>;
 
     struct EnumInfo {
         QStringList path;
diff --git a/src/qml/qml/qqmltypenamecache_p.h b/src/qml/qml/qqmltypenamecache_p.h
index 23dad8a4a5..552db9f98c 100644
--- a/src/qml/qml/qqmltypenamecache_p.h
+++ b/src/qml/qml/qqmltypenamecache_p.h
@@ -45,7 +45,7 @@ struct QQmlImportRef {
 
 class QQmlType;
 class QQmlEngine;
-class Q_QML_PRIVATE_EXPORT QQmlTypeNameCache : public QQmlRefCounted<QQmlTypeNameCache>
+class Q_QML_PRIVATE_EXPORT QQmlTypeNameCache final : public QQmlRefCounted<QQmlTypeNameCache>
 {
 public:
     QQmlTypeNameCache(const QQmlRefPointer<QQmlImports> &imports) : m_imports(imports) {}
diff --git a/src/qml/qml/qqmlxmlhttprequest.cpp b/src/qml/qml/qqmlxmlhttprequest.cpp
index 60c1a7f1d3..eb4b9e4419 100644
--- a/src/qml/qml/qqmlxmlhttprequest.cpp
+++ b/src/qml/qml/qqmlxmlhttprequest.cpp
@@ -116,7 +116,7 @@ public:
     QList<NodeImpl *> attributes;
 };
 
-class DocumentImpl : public QQmlRefCounted<DocumentImpl>, public NodeImpl
+class DocumentImpl final : public QQmlRefCounted<DocumentImpl>, public NodeImpl
 {
     using Base1 = QQmlRefCounted<DocumentImpl>;
 public:
diff --git a/src/qmlmodels/qqmldelegatemodel_p_p.h b/src/qmlmodels/qqmldelegatemodel_p_p.h
index 2c63859d4c..2194224cb6 100644
--- a/src/qmlmodels/qqmldelegatemodel_p_p.h
+++ b/src/qmlmodels/qqmldelegatemodel_p_p.h
@@ -37,7 +37,7 @@ typedef QQmlListCompositor Compositor;
 class QQmlDelegateModelAttachedMetaObject;
 class QQmlAbstractDelegateComponent;
 
-class Q_QMLMODELS_PRIVATE_EXPORT QQmlDelegateModelItemMetaType
+class Q_QMLMODELS_PRIVATE_EXPORT QQmlDelegateModelItemMetaType final
     : public QQmlRefCounted<QQmlDelegateModelItemMetaType>
 {
 public:
@@ -420,7 +420,7 @@ public:
     QList<QQmlPartsModel *> models;
 };
 
-class QQmlDelegateModelAttachedMetaObject
+class QQmlDelegateModelAttachedMetaObject final
     : public QAbstractDynamicMetaObject,
       public QQmlRefCounted<QQmlDelegateModelAttachedMetaObject>
 {
diff --git a/src/qmlmodels/qqmldmabstractitemmodeldata_p.h b/src/qmlmodels/qqmldmabstractitemmodeldata_p.h
index c85c61d941..ba3763485d 100644
--- a/src/qmlmodels/qqmldmabstractitemmodeldata_p.h
+++ b/src/qmlmodels/qqmldmabstractitemmodeldata_p.h
@@ -72,7 +72,7 @@ private:
     QVector<QVariant> m_cachedData;
 };
 
-class VDMAbstractItemModelDataType
+class VDMAbstractItemModelDataType final
         : public QQmlRefCounted<VDMAbstractItemModelDataType>
         , public QQmlAdaptorModel::Accessors
         , public QAbstractDynamicMetaObject
diff --git a/src/qmlmodels/qqmldmlistaccessordata_p.h b/src/qmlmodels/qqmldmlistaccessordata_p.h
index 844668b607..440e970880 100644
--- a/src/qmlmodels/qqmldmlistaccessordata_p.h
+++ b/src/qmlmodels/qqmldmlistaccessordata_p.h
@@ -86,7 +86,7 @@ private:
 };
 
 
-class VDMListDelegateDataType
+class VDMListDelegateDataType final
     : public QQmlRefCounted<VDMListDelegateDataType>
     , public QQmlAdaptorModel::Accessors
     , public QAbstractDynamicMetaObject
diff --git a/src/qmlmodels/qqmldmobjectdata_p.h b/src/qmlmodels/qqmldmobjectdata_p.h
index edad02a073..f5ffcdc3b9 100644
--- a/src/qmlmodels/qqmldmobjectdata_p.h
+++ b/src/qmlmodels/qqmldmobjectdata_p.h
@@ -53,7 +53,7 @@ Q_SIGNALS:
     void modelDataChanged();
 };
 
-class VDMObjectDelegateDataType
+class VDMObjectDelegateDataType final
     : public QQmlRefCounted<VDMObjectDelegateDataType>,
       public QQmlAdaptorModel::Accessors
 {
diff --git a/src/quick/items/context2d/qquickcanvasitem_p.h b/src/quick/items/context2d/qquickcanvasitem_p.h
index d5bf3a83ef..9a868f03de 100644
--- a/src/quick/items/context2d/qquickcanvasitem_p.h
+++ b/src/quick/items/context2d/qquickcanvasitem_p.h
@@ -33,7 +33,7 @@ class QQuickCanvasItemPrivate;
 class QQuickPixmap;
 class QQmlV4Function;
 
-class QQuickCanvasPixmap : public QQmlRefCounted<QQuickCanvasPixmap>
+class QQuickCanvasPixmap final : public QQmlRefCounted<QQuickCanvasPixmap>
 {
 public:
     QQuickCanvasPixmap(const QImage& image);
diff --git a/tests/auto/toolsupport/tst_toolsupport.cpp b/tests/auto/toolsupport/tst_toolsupport.cpp
index 893f4b95b9..bf7a06e305 100644
--- a/tests/auto/toolsupport/tst_toolsupport.cpp
+++ b/tests/auto/toolsupport/tst_toolsupport.cpp
@@ -68,7 +68,7 @@ void tst_toolsupport::offsets_data()
     {
         QTestData &data = QTest::newRow("sizeof(QQmlRefCount)")
                 << sizeof(QQmlRefCount);
-        data << 8 << 16;
+        data << 4 << 4;
     }
 
 #if RUN_MEMBER_OFFSET_TEST