From 734c8aa9d904e81119b54d4311a2ec7161ee9b90 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= <marten.nordheim@qt.io>
Date: Thu, 26 Nov 2020 15:43:17 +0100
Subject: [PATCH] QSet: add insert(T&&)

We already have all we need in QHash to support this, so the addition
is simple enough.

Add test checking how many copies and/or moves are needed for a single
insert.

As a drive-by: remove some unneeded static_cast

Change-Id: Iaf768657644afa45f78f5c81ffcf89ba9607be96
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/tools/qset.h                      |  4 +-
 .../tst_qduplicatetracker.cpp                 |  5 --
 tests/auto/corelib/tools/qset/tst_qset.cpp    | 79 +++++++++++++++++++
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/src/corelib/tools/qset.h b/src/corelib/tools/qset.h
index 3a519888522..681ce9cbe24 100644
--- a/src/corelib/tools/qset.h
+++ b/src/corelib/tools/qset.h
@@ -184,7 +184,9 @@ public:
     typedef const_iterator ConstIterator;
     inline qsizetype count() const { return q_hash.count(); }
     inline iterator insert(const T &value)
-        { return static_cast<typename Hash::iterator>(q_hash.insert(value, QHashDummyValue())); }
+        { return q_hash.insert(value, QHashDummyValue()); }
+    inline iterator insert(T &&value)
+        { return q_hash.emplace(std::move(value), QHashDummyValue()); }
     iterator find(const T &value) { return q_hash.find(value); }
     const_iterator find(const T &value) const { return q_hash.find(value); }
     inline const_iterator constFind(const T &value) const { return find(value); }
diff --git a/tests/auto/corelib/tools/qduplicatetracker/tst_qduplicatetracker.cpp b/tests/auto/corelib/tools/qduplicatetracker/tst_qduplicatetracker.cpp
index 056934ae70f..f4f038ca940 100644
--- a/tests/auto/corelib/tools/qduplicatetracker/tst_qduplicatetracker.cpp
+++ b/tests/auto/corelib/tools/qduplicatetracker/tst_qduplicatetracker.cpp
@@ -177,13 +177,8 @@ void tst_QDuplicateTracker::appendTo_special()
     a.reserve(3);
     tracker.appendTo(a);
     for (const auto &counter : a) {
-#if QT_HAS_INCLUDE(<memory_resource>) && __cplusplus > 201402L // uses pmr::unordered_set
         QCOMPARE(counter.moves, 1);
         QCOMPARE(counter.copies, 1);
-#else // Uses QSet
-        QCOMPARE(counter.moves, 1);
-        QCOMPARE(counter.copies, 2);
-#endif
     }
 }
 
diff --git a/tests/auto/corelib/tools/qset/tst_qset.cpp b/tests/auto/corelib/tools/qset/tst_qset.cpp
index 72ad4848426..5c4cc1f1a45 100644
--- a/tests/auto/corelib/tools/qset/tst_qset.cpp
+++ b/tests/auto/corelib/tools/qset/tst_qset.cpp
@@ -59,6 +59,7 @@ private slots:
     void begin();
     void end();
     void insert();
+    void insertConstructionCounted();
     void setOperations();
     void stlIterator();
     void stlMutableIterator();
@@ -579,6 +580,84 @@ void tst_QSet::insert()
     }
 }
 
+struct ConstructionCounted
+{
+    ConstructionCounted(int i) : i(i) { }
+    ConstructionCounted(ConstructionCounted &&other) noexcept
+        : i(other.i), copies(other.copies), moves(other.moves + 1)
+    {
+        // set to some easily noticeable values
+        other.i = -64;
+        other.copies = -64;
+        other.moves = -64;
+    }
+    ConstructionCounted &operator=(ConstructionCounted &&other) noexcept
+    {
+        ConstructionCounted moved = std::move(other);
+        std::swap(*this, moved);
+        return *this;
+    }
+    ConstructionCounted(const ConstructionCounted &other) noexcept
+        : i(other.i), copies(other.copies + 1), moves(other.moves)
+    {
+    }
+    ConstructionCounted &operator=(const ConstructionCounted &other) noexcept
+    {
+        ConstructionCounted copy = other;
+        std::swap(*this, copy);
+        return *this;
+    }
+    ~ConstructionCounted() = default;
+
+    friend bool operator==(const ConstructionCounted &lhs, const ConstructionCounted &rhs)
+    {
+        return lhs.i == rhs.i;
+    }
+
+    QString toString() { return QString::number(i); }
+
+    int i;
+    int copies = 0;
+    int moves = 0;
+};
+
+size_t qHash(const ConstructionCounted &c, std::size_t seed = 0)
+{
+    return qHash(c.i, seed);
+}
+
+void tst_QSet::insertConstructionCounted()
+{
+    QSet<ConstructionCounted> set;
+
+    // copy-insert
+    ConstructionCounted toCopy(7);
+    auto inserted = set.insert(toCopy);
+    QCOMPARE(set.size(), 1);
+    auto element = set.begin();
+    QCOMPARE(inserted, element);
+    QCOMPARE(inserted->copies, 1);
+    QCOMPARE(inserted->moves, 1);
+    QCOMPARE(inserted->i, 7);
+
+    // move-insert
+    ConstructionCounted toMove(8);
+    inserted = set.insert(std::move(toMove));
+    element = set.find(8);
+    QCOMPARE(set.size(), 2);
+    QVERIFY(element != set.end());
+    QCOMPARE(inserted, element);
+    QCOMPARE(inserted->copies, 0);
+    QCOMPARE(inserted->moves, 1);
+    QCOMPARE(inserted->i, 8);
+
+    inserted = set.insert(std::move(toCopy)); // move-insert an existing value
+    QCOMPARE(set.size(), 2);
+    // The previously existing key is used as they compare equal:
+    QCOMPARE(inserted->copies, 1);
+    QCOMPARE(inserted->moves, 1);
+}
+
 void tst_QSet::setOperations()
 {
     QSet<QString> set1, set2;