From 4a6cbfbe5c90158ab2f4afed3d188dae3ec0ea85 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Wed, 13 Sep 2023 10:27:04 +0200 Subject: [PATCH] QVariant: add fromMetaType The QVariant(QMetaType) constructor is a major anti-pattern: unlike *every* other QVariant's constructor, it doesn't build a QVariant holding the QMetaType object, but a QVariant of the specified type. Introduce a named constructor for this use case instead. In principle, this should lead to a deprecation of the QMetaType constructor... except that it's used everywhere, so I can't do it at this time. Drive-by, improve the documentation of the QVariant(QMetaType) constructor (since it's basically c&p for the new fromMetaType function). [ChangeLog][QtCore][QVariant] Added the QVariant::fromMetaType named constructor, that builds a QVariant of a given QMetaType. Change-Id: I4a499526bd0fe98eed0c1a3e91bcfc21efa9e352 Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qvariant.cpp | 61 +++++++++++++++---- src/corelib/kernel/qvariant.h | 4 +- .../corelib/kernel/qvariant/tst_qvariant.cpp | 18 ++++++ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 2c8e61fa349..f8a3747207f 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -516,7 +516,7 @@ void QVariant::create(int type, const void *copy) */ void QVariant::create(QMetaType type, const void *copy) { - *this = QVariant(type, copy); + *this = QVariant::fromMetaType(type, copy); } /*! @@ -910,27 +910,27 @@ void *QVariant::prepareForEmplace(QMetaType type) */ /*! - Constructs variant of type \a type, and initializes with - \a copy if \a copy is not \nullptr. + Constructs a variant of type \a type, and initializes it with + a copy of \c{*copy} if \a copy is not \nullptr (in which case, \a copy + must point to an object of type \a type). - Note that you have to pass the address of the variable you want stored. + Note that you have to pass the address of the object you want stored. Usually, you never have to use this constructor, use QVariant::fromValue() instead to construct variants from the pointer types represented by \c QMetaType::VoidStar, and \c QMetaType::QObjectStar. - If \a type does not support copy and default construction, the variant will - be invalid. + If \a type does not support copy construction and \a copy is not \nullptr, + the variant will be invalid. Similarly, if \a copy is \nullptr and + \a type does not support default construction, the variant will be + invalid. - \sa QVariant::fromValue(), QMetaType::Type + \sa QVariant::fromMetaType, QVariant::fromValue(), QMetaType::Type */ -QVariant::QVariant(QMetaType type, const void *copy) : d(type.iface()) +QVariant::QVariant(QMetaType type, const void *copy) + : d() { - type.registerType(); - if (isValidMetaTypeForVariant(type.iface(), copy)) - customConstruct(type.iface(), &d, copy); - else - d = {}; + *this = fromMetaType(type, copy); } QVariant::QVariant(int val) noexcept : d(std::piecewise_construct_t{}, val) {} @@ -2711,6 +2711,41 @@ QT_WARNING_POP \overload */ + +/*! + \since 6.7 + + Creates a variant of type \a type, and initializes it with + a copy of \c{*copy} if \a copy is not \nullptr (in which case, \a copy + must point to an object of type \a type). + + Note that you have to pass the address of the object you want stored. + + Usually, you never have to use this constructor, use QVariant::fromValue() + instead to construct variants from the pointer types represented by + \c QMetaType::VoidStar, and \c QMetaType::QObjectStar. + + If \a type does not support copy construction and \a copy is not \nullptr, + the variant will be invalid. Similarly, if \a copy is \nullptr and + \a type does not support default construction, the variant will be + invalid. + + Returns the QVariant created as described above. + + \sa QVariant::fromValue(), QMetaType::Type +*/ +QVariant QVariant::fromMetaType(QMetaType type, const void *copy) +{ + QVariant result; + type.registerType(); + const auto iface = type.iface(); + if (isValidMetaTypeForVariant(iface, copy)) { + result.d = Private(iface); + customConstruct(iface, &result.d, copy); + } + return result; +} + /*! \fn template T qvariant_cast(const QVariant &value) \relates QVariant diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h index 2c8df591e2e..c57eed97fb3 100644 --- a/src/corelib/kernel/qvariant.h +++ b/src/corelib/kernel/qvariant.h @@ -539,7 +539,7 @@ public: // handle special cases using Type = std::remove_cv_t; if constexpr (std::is_null_pointer_v) - return QVariant(QMetaType::fromType()); + return QVariant::fromMetaType(QMetaType::fromType()); else if constexpr (std::is_same_v) return std::forward(value); else if constexpr (std::is_same_v) @@ -586,6 +586,8 @@ public: return fromStdVariantImpl(std::move(value)); } + static QVariant fromMetaType(QMetaType type, const void *copy = nullptr); + template bool canConvert() const { return canConvert(QMetaType::fromType()); } diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 3a621b91d6c..a1f95310932 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -442,11 +442,21 @@ void tst_QVariant::constructor() QVERIFY(var3.isNull()); QVERIFY(var3.isValid()); + QVariant var3a = QVariant::fromMetaType(QMetaType::fromType()); + QCOMPARE(var3a.typeName(), "QString"); + QVERIFY(var3a.isNull()); + QVERIFY(var3a.isValid()); + QVariant var4 {QMetaType()}; QCOMPARE(var4.typeId(), QMetaType::UnknownType); QVERIFY(var4.isNull()); QVERIFY(!var4.isValid()); + QVariant var4a = QVariant::fromMetaType(QMetaType()); + QCOMPARE(var4a.typeId(), QMetaType::UnknownType); + QVERIFY(var4a.isNull()); + QVERIFY(!var4a.isValid()); + QVariant var5(QLatin1String("hallo")); QCOMPARE(var5.typeId(), QMetaType::QString); QCOMPARE(var5.typeName(), "QString"); @@ -487,6 +497,14 @@ void tst_QVariant::constructor_invalid() QCOMPARE(variant.typeId(), int(QMetaType::UnknownType)); QCOMPARE(variant.userType(), int(QMetaType::UnknownType)); } + { + QTest::ignoreMessage(QtWarningMsg, QRegularExpression("^Trying to construct an instance of an invalid type")); + QVariant variant = QVariant::fromMetaType(QMetaType(typeId)); + QVERIFY(!variant.isValid()); + QVERIFY(variant.isNull()); + QCOMPARE(variant.typeId(), int(QMetaType::UnknownType)); + QCOMPARE(variant.userType(), int(QMetaType::UnknownType)); + } { QTest::ignoreMessage(QtWarningMsg, QRegularExpression("^Trying to construct an instance of an invalid type")); QVariant variant(QMetaType(typeId), /* copy */ nullptr);