From 43fa292ff621550d0977908b499fb54096251825 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Sat, 6 Jun 2020 00:27:44 +0200 Subject: [PATCH] QVariant::setValue(): enable move semantics Given we optimize for the case where the new value is of the same type of the one already stored in the variant, enable move assignment for that case. As a drive-by, avoid a path to detach() for data() if we know we're detached. Change-Id: I9abbdc10637ce77ebb747b49d83e1ef914d997bb Reviewed-by: Thiago Macieira --- src/corelib/kernel/qvariant.cpp | 14 ++++++- src/corelib/kernel/qvariant.h | 42 ++++++++++--------- src/gui/text/qtextformat.cpp | 2 +- .../corelib/kernel/qvariant/tst_qvariant.cpp | 2 +- .../qgraphicsitem/tst_qgraphicsitem.cpp | 8 ++-- 5 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index f4d4dafe6b..8a34806c60 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -4019,7 +4019,7 @@ QDebug operator<<(QDebug dbg, const QVariant::Type p) #endif -/*! \fn template void QVariant::setValue(const T &value) +/*! \fn template void QVariant::setValue(T &&value) Stores a copy of \a value. If \c{T} is a type that QVariant doesn't support, QMetaType is used to store the value. A compile @@ -4032,6 +4032,18 @@ QDebug operator<<(QDebug dbg, const QVariant::Type p) \sa value(), fromValue(), canConvert() */ +/*! \fn template void QVariant::setValue(const QVariant &value) + + Copies \a value over this QVariant. It is equivalent to simply + assigning \a value to this QVariant. +*/ + +/*! \fn template void QVariant::setValue(QVariant &&value) + + Moves \a value over this QVariant. It is equivalent to simply + move assigning \a value to this QVariant. +*/ + /*! \fn template T QVariant::value() const Returns the stored value converted to the template type \c{T}. diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h index b5e9deedf0..87fb16485f 100644 --- a/src/corelib/kernel/qvariant.h +++ b/src/corelib/kernel/qvariant.h @@ -356,8 +356,28 @@ class Q_CORE_EXPORT QVariant const void *constData() const; inline const void *data() const { return constData(); } - template - inline void setValue(const T &value); + template, QVariant>>> + void setValue(T &&avalue) + { + using VT = std::decay_t; + QMetaType metaType = QMetaType::fromType(); + // If possible we reuse the current QVariant private. + if (isDetached() && d.type() == metaType) { + *reinterpret_cast(const_cast(constData())) = std::forward(avalue); + } else { + *this = QVariant::fromValue(std::forward(avalue)); + } + } + + void setValue(const QVariant &avalue) + { + *this = avalue; + } + + void setValue(QVariant &&avalue) + { + *this = std::move(avalue); + } template inline T value() const @@ -533,24 +553,6 @@ inline bool QVariant::isValid() const return d.type().isValid(); } -template -inline void QVariant::setValue(const T &avalue) -{ - QMetaType metaType = QMetaType::fromType(); - // If possible we reuse the current QVariant private. - if (isDetached() && d.type() == metaType) { - *reinterpret_cast(data()) = avalue; - } else { - *this = QVariant::fromValue(avalue); - } -} - -template<> -inline void QVariant::setValue(const QVariant &avalue) -{ - *this = avalue; -} - #ifndef QT_NO_DATASTREAM Q_CORE_EXPORT QDataStream& operator>> (QDataStream& s, QVariant& p); Q_CORE_EXPORT QDataStream& operator<< (QDataStream& s, const QVariant& p); diff --git a/src/gui/text/qtextformat.cpp b/src/gui/text/qtextformat.cpp index 58c318bcbf..d12f7e152d 100644 --- a/src/gui/text/qtextformat.cpp +++ b/src/gui/text/qtextformat.cpp @@ -2122,7 +2122,7 @@ void QTextBlockFormat::setTabPositions(const QList &tabs) QList::ConstIterator iter = tabs.constBegin(); while (iter != tabs.constEnd()) { QVariant v; - v.setValue(*iter); + v.setValue(*iter); list.append(v); ++iter; } diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 9920c8812d..344b020956 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -327,7 +327,7 @@ void tst_QVariant::constructor() QVERIFY(var7.isValid()); QVERIFY(!var7.isNull()); QVariant var8; - var8.setValue(5); + var8.setValue(5); QVERIFY(var8.isValid()); QVERIFY(!var8.isNull()); } diff --git a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp index 1c311a27ea..aa08f1c44d 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp @@ -4499,7 +4499,7 @@ protected: break; case QGraphicsItem::ItemTransformChange: { QVariant variant; - variant.setValue(transform()); + variant.setValue(transform()); oldValues << variant; } break; @@ -4616,7 +4616,7 @@ void tst_QGraphicsItem::itemChange() } { // ItemTransformChange / ItemTransformHasChanged - tester.itemChangeReturnValue.setValue(QTransform().rotate(90)); + tester.itemChangeReturnValue.setValue(QTransform().rotate(90)); tester.setTransform(QTransform::fromTranslate(50, 0), true); ++changeCount; // notification sent too ++changeCount; @@ -4628,7 +4628,7 @@ void tst_QGraphicsItem::itemChange() QCOMPARE(qvariant_cast(tester.values.at(tester.values.size() - 1)), QTransform().rotate(90)); QVariant variant; - variant.setValue(QTransform()); + variant.setValue(QTransform()); QCOMPARE(tester.oldValues.constLast(), variant); QCOMPARE(tester.transform(), QTransform().rotate(90)); } @@ -4763,7 +4763,7 @@ void tst_QGraphicsItem::itemChange() } { // ItemParentChange - tester.itemChangeReturnValue.setValue(nullptr); + tester.itemChangeReturnValue.setValue(static_cast(nullptr)); tester.setParentItem(&testerHelper); QCOMPARE(tester.changes.size(), ++changeCount); QCOMPARE(tester.changes.constLast(), QGraphicsItem::ItemParentChange);