From dcf76042301772b770a088526e5753ed41232d87 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Fri, 12 Feb 2021 22:35:28 +0100
Subject: [PATCH] QVariant::value/qvariant_cast: add rvalue optimization

If we have a rvalue reference to an unshared QVariant, we can avoid
potentially expensive copies, and use move semantics instead.

[ChangeLog][QtCore][QVariant] Added rvalue QVariant overloads of qvariant_cast<T>() and QVariant::value<T>().

[ChangeLog][Potentially Source-Incompatible Changes][QVariant] It is no
longer possible to take the address of a specialization of
qvariant_cast; consider using a lambda function instead.

Change-Id: Ifc74991eadcc31387b755c45484224a3200bb0ba
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
---
 .../code/src_concurrent_qtconcurrenttask.cpp  |  2 +-
 src/corelib/kernel/qvariant.h                 | 38 ++++++++++++++++++-
 .../qtconcurrenttask/tst_qtconcurrenttask.cpp |  2 +-
 .../corelib/kernel/qvariant/tst_qvariant.cpp  | 11 ++++++
 4 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/concurrent/doc/snippets/code/src_concurrent_qtconcurrenttask.cpp b/src/concurrent/doc/snippets/code/src_concurrent_qtconcurrenttask.cpp
index ac3ca7fdfb..cb1889afb6 100644
--- a/src/concurrent/doc/snippets/code/src_concurrent_qtconcurrenttask.cpp
+++ b/src/concurrent/doc/snippets/code/src_concurrent_qtconcurrenttask.cpp
@@ -30,7 +30,7 @@ std::is_invocable_v<std::decay_t<Task>, std::decay_t<Args>...>
 
 //! [5]
 QVariant value(42);
-auto result = QtConcurrent::task(&qvariant_cast<int>)
+auto result = QtConcurrent::task([](const QVariant &var){return qvariant_cast<int>(var);})
                   .withArguments(value)
                   .spawn()
                   .result(); // result == 42
diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h
index ed4978699f..2c8df591e2 100644
--- a/src/corelib/kernel/qvariant.h
+++ b/src/corelib/kernel/qvariant.h
@@ -508,7 +508,7 @@ public:
     }
 
     template<typename T>
-    inline T value() const
+    inline T value() const &
     { return qvariant_cast<T>(*this); }
 
     template<typename T>
@@ -519,6 +519,10 @@ public:
         return t;
     }
 
+    template<typename T>
+    inline T value() &&
+    { return qvariant_cast<T>(std::move(*this)); }
+
     template<typename T, if_rvalue<T> = true>
 #ifndef Q_QDOC
         /* needs is_copy_constructible for variants semantics, is_move_constructible so that moveConstruct works
@@ -654,6 +658,9 @@ private:
 
     template<typename T>
     friend inline T qvariant_cast(const QVariant &);
+    template<typename T>
+    friend inline T qvariant_cast(QVariant &&);
+
 protected:
     Private d;
     void create(int type, const void *copy);
@@ -753,6 +760,35 @@ template<typename T> inline T qvariant_cast(const QVariant &v)
     return t;
 }
 
+template<typename T> inline T qvariant_cast(QVariant &&v)
+{
+    QMetaType targetType = QMetaType::fromType<T>();
+    if (v.d.type() == targetType) {
+        if constexpr (QVariant::Private::CanUseInternalSpace<T>) {
+            return std::move(*reinterpret_cast<T *>(v.d.data.data));
+        } else {
+            if (v.d.data.shared->ref.loadRelaxed() == 1)
+                return std::move(*reinterpret_cast<T *>(v.d.data.shared->data()));
+            else
+                return v.d.get<T>();
+        }
+    }
+    if constexpr (std::is_same_v<T, QVariant>) {
+        // if the metatype doesn't match, but we want a QVariant, just return the current variant
+        return v;
+    } if constexpr (std::is_same_v<T,std::remove_const_t<std::remove_pointer_t<T>> const *>) {
+        // moving a pointer is pointless, just do the same as the const & overload
+        using nonConstT = std::remove_const_t<std::remove_pointer_t<T>> *;
+        QMetaType nonConstTargetType = QMetaType::fromType<nonConstT>();
+        if (v.d.type() == nonConstTargetType)
+            return v.d.get<nonConstT>();
+    }
+
+    T t{};
+    QMetaType::convert(v.metaType(), v.constData(), targetType, &t);
+    return t;
+}
+
 template<> inline QVariant qvariant_cast<QVariant>(const QVariant &v)
 {
     if (v.metaType().id() == QMetaType::QVariant)
diff --git a/tests/auto/concurrent/qtconcurrenttask/tst_qtconcurrenttask.cpp b/tests/auto/concurrent/qtconcurrenttask/tst_qtconcurrenttask.cpp
index 114d899e1d..652b372505 100644
--- a/tests/auto/concurrent/qtconcurrenttask/tst_qtconcurrenttask.cpp
+++ b/tests/auto/concurrent/qtconcurrenttask/tst_qtconcurrenttask.cpp
@@ -32,7 +32,7 @@ void tst_QtConcurrentTask::taskWithFreeFunction()
 {
     QVariant value(42);
 
-    auto result = task(&qvariant_cast<int>)
+    auto result = task([](const QVariant &var){ return qvariant_cast<int>(var); })
                       .withArguments(value)
                       .spawn()
                       .result();
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index 23fa969edd..d1d47b329b 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -5662,6 +5662,17 @@ void tst_QVariant::moveOperations()
         QVariant::fromValue(std::move(tester));
         QVERIFY(!tester.wasMoved); // we don't want to move from const variables
     }
+    {
+        QVariant var(std::in_place_type<MoveTester>);
+        const auto p = get_if<MoveTester>(&var);
+        QVERIFY(p);
+        auto &tester = *p;
+        QVERIFY(!tester.wasMoved);
+        [[maybe_unused]] auto copy = var.value<MoveTester>();
+        QVERIFY(!tester.wasMoved);
+        [[maybe_unused]] auto moved = std::move(var).value<MoveTester>();
+        QVERIFY(tester.wasMoved);
+    }
 }
 
 class NoMetaObject : public QObject {};