From 3ef6f35cf6d68fad241d744b7cfa2e3adec496f2 Mon Sep 17 00:00:00 2001
From: Tobias Hunger <tobias.hunger@qt.io>
Date: Thu, 15 Mar 2018 13:00:05 +0100
Subject: [PATCH] PointerAlgorithm: Fix take and takeDefault

Fix take unit tests to actually test the correct template instance,
add more unit tests for takeDefault.

Change-Id: I51f5b17b6478a8c1388a91840edfb9c702697b28
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
---
 src/libs/utils/pointeralgorithm.h             | 10 ++--
 .../pointeralgorithm/tst_pointeralgorithm.cpp | 46 ++++++++++++-------
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/src/libs/utils/pointeralgorithm.h b/src/libs/utils/pointeralgorithm.h
index 64550fdee6c..1ab336f5101 100644
--- a/src/libs/utils/pointeralgorithm.h
+++ b/src/libs/utils/pointeralgorithm.h
@@ -275,7 +275,7 @@ auto toRawPointer(const SourceContainer &sources)
 // take:
 /////////////////
 template<typename C>
-Q_REQUIRED_RESULT Utils::optional<ValueType<C>> take(C &container, ValueType<C> *p)
+Q_REQUIRED_RESULT Utils::optional<ValueType<C>> take(C &container, PointerType<C> p)
 {
     return take(container, [p](const ValueType<C> &v) { return v.get() == p; });
 }
@@ -283,23 +283,23 @@ Q_REQUIRED_RESULT Utils::optional<ValueType<C>> take(C &container, ValueType<C>
 template <typename C>
 Q_REQUIRED_RESULT Utils::optional<ValueType<C>> take(C &container, std::nullptr_t)
 {
-    return take(container, static_cast<ValueType<C> *>(nullptr));
+    return take(container, static_cast<PointerType<C>>(nullptr));
 }
 
 //////////////////
 // takeOrDefault:
 /////////////////
 template<typename C>
-Q_REQUIRED_RESULT ValueType<C> takeOrDefault(C &container, ValueType<C> *p)
+Q_REQUIRED_RESULT ValueType<C> takeOrDefault(C &container, PointerType<C> p)
 {
     auto result = take(container, [p](const ValueType<C> &v) { return v.get() == p; });
-    return bool(result) ? result.value() : Utils::make_optional<ValueType<C>>(nullptr);
+    return bool(result) ? std::move(result.value()) : std::move(ValueType<C>());
 }
 
 template <typename C>
 Q_REQUIRED_RESULT ValueType<C> takeOrDefault(C &container, std::nullptr_t)
 {
-    return takeOrDefault(container, static_cast<ValueType<C> *>(nullptr));
+    return takeOrDefault(container, static_cast<PointerType<C>>(nullptr));
 }
 
 } // namespace Utils
diff --git a/tests/auto/pointeralgorithm/tst_pointeralgorithm.cpp b/tests/auto/pointeralgorithm/tst_pointeralgorithm.cpp
index 807d5986550..d160768c5fc 100644
--- a/tests/auto/pointeralgorithm/tst_pointeralgorithm.cpp
+++ b/tests/auto/pointeralgorithm/tst_pointeralgorithm.cpp
@@ -50,6 +50,7 @@ private slots:
     void toRawPointer();
     void toReferences();
     void take();
+    void takeOrDefault();
 };
 
 
@@ -283,25 +284,38 @@ void tst_PointerAlgorithm::toReferences()
 void tst_PointerAlgorithm::take()
 {
     {
-        QList<Struct> v {1, 3, 5, 6, 7, 8, 9, 11, 13, 15, 13, 16, 17};
-        Utils::optional<Struct> r1 = Utils::take(v, [](const Struct &s) { return s.member == 13; });
-        QVERIFY(static_cast<bool>(r1));
-        QCOMPARE(r1.value().member, 13);
-        Utils::optional<Struct> r2 = Utils::take(v, [](const Struct &s) { return s.member == 13; });
-        QVERIFY(static_cast<bool>(r2));
-        QCOMPARE(r2.value().member, 13);
-        Utils::optional<Struct> r3 = Utils::take(v, [](const Struct &s) { return s.member == 13; });
-        QVERIFY(!static_cast<bool>(r3));
+        std::vector<std::unique_ptr<int>> vector;
+        vector.emplace_back(std::make_unique<int>(5));
+        vector.emplace_back(std::make_unique<int>(2));
+        vector.emplace_back(std::make_unique<int>(6));
+        vector.emplace_back(std::make_unique<int>(7));
+        vector.emplace_back(std::unique_ptr<int>());
+        std::vector<int *> ptrVector = Utils::toRawPointer(vector);
+        int foo = 42;
 
-        Utils::optional<Struct> r4 = Utils::take(v, &Struct::isEven);
-        QVERIFY(static_cast<bool>(r4));
-        QCOMPARE(r4.value().member, 6);
+        QVERIFY(Utils::take(vector, ptrVector.at(0)).value().get() == ptrVector.at(0));
+        QVERIFY(Utils::take(vector, ptrVector.at(0)) == Utils::nullopt);
+        QVERIFY(Utils::take(vector, &foo) == Utils::nullopt);
+        QVERIFY(Utils::take(vector, nullptr).value().get() == nullptr);
     }
+}
+
+void tst_PointerAlgorithm::takeOrDefault()
+{
     {
-        QList<Struct> v {0, 0, 0, 0, 0, 0, 1, 2, 3};
-        Utils::optional<Struct> r1 = Utils::take(v, &Struct::member);
-        QVERIFY(static_cast<bool>(r1));
-        QCOMPARE(r1.value().member, 1);
+        std::vector<std::unique_ptr<int>> vector;
+        vector.emplace_back(std::make_unique<int>(5));
+        vector.emplace_back(std::make_unique<int>(2));
+        vector.emplace_back(std::make_unique<int>(6));
+        vector.emplace_back(std::make_unique<int>(7));
+        vector.emplace_back(std::unique_ptr<int>());
+        std::vector<int *> ptrVector = Utils::toRawPointer(vector);
+        int foo = 42;
+
+        QVERIFY(Utils::takeOrDefault(vector, ptrVector.at(0)).get() == ptrVector.at(0));
+        QVERIFY(Utils::takeOrDefault(vector, ptrVector.at(0)).get() == nullptr);
+        QVERIFY(Utils::takeOrDefault(vector, &foo).get() == nullptr);
+        QVERIFY(Utils::takeOrDefault(vector, nullptr).get() == nullptr);
     }
 }