From e38d1f06bd81adc1b7c077ad164e24f722456c6f Mon Sep 17 00:00:00 2001
From: Eike Ziller <eike.ziller@qt.io>
Date: Thu, 22 Dec 2022 09:40:34 +0100
Subject: Fix that Utils::sorted could modify input container

Utils::sorted had overloads for "const Container &", "Container &&", and
"const Container &&", but for _templated_ types "Container &&" does
_not_ mean "rvalue reference", it means "rvalue or lvalue reference"
(e.g. "universal" reference).

That means that for non-const lvalue references that "Container &&"
overload was used, which modifies the input container. Which is a fine
optimization for rvalue references, but is wrong for lvalue references.

Add another overload explicitly for "Container &" before the
"Container &&" overload, and add some tests.

Also fix the compiler warning that triggered the investigation:

warning: local variable 'container' will be copied despite being
returned by name [-Wreturn-std-move]
note: call 'std::move' explicitly to avoid copying

Amends 13f40f5471e55757a2cf9bba8d052750a2f2a753

Change-Id: I14461fde5fc51a8bb679fd72b886e13b18c47e7b
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: hjk <hjk@qt.io>
---
 tests/auto/algorithm/tst_algorithm.cpp | 101 +++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

(limited to 'tests/auto/algorithm/tst_algorithm.cpp')

diff --git a/tests/auto/algorithm/tst_algorithm.cpp b/tests/auto/algorithm/tst_algorithm.cpp
index 5f2eaf23e2..dbe3d732a4 100644
--- a/tests/auto/algorithm/tst_algorithm.cpp
+++ b/tests/auto/algorithm/tst_algorithm.cpp
@@ -28,6 +28,7 @@ private slots:
     void findOrDefault();
     void toReferences();
     void take();
+    void sorted();
 };
 
 
@@ -550,6 +551,106 @@ void tst_Algorithm::take()
     }
 }
 
+void tst_Algorithm::sorted()
+{
+    const QList<int> vOrig{4, 3, 6, 5, 8};
+    const QList<int> vExpected{3, 4, 5, 6, 8};
+
+    // plain
+    {
+        // non-const lvalue
+        QList<int> vncl = vOrig;
+        const QList<int> rncl = Utils::sorted(vncl);
+        QCOMPARE(rncl, vExpected);
+        QCOMPARE(vncl, vOrig); // was not modified
+
+        // const lvalue
+        const QList<int> rcl = Utils::sorted(vOrig);
+        QCOMPARE(rcl, vExpected);
+
+        // non-const rvalue
+        const auto vncr = [vOrig]() -> QList<int> { return vOrig; };
+        const QList<int> rncr = Utils::sorted(vncr());
+        QCOMPARE(rncr, vExpected);
+
+        // const rvalue
+        const auto vcr = [vOrig]() -> const QList<int> { return vOrig; };
+        const QList<int> rcr = Utils::sorted(vcr());
+        QCOMPARE(rcr, vExpected);
+    }
+
+    // predicate
+    {
+        // non-const lvalue
+        QList<int> vncl = vOrig;
+        const QList<int> rncl = Utils::sorted(vncl, [](int a, int b) { return a < b; });
+        QCOMPARE(rncl, vExpected);
+        QCOMPARE(vncl, vOrig); // was not modified
+
+        // const lvalue
+        const QList<int> rcl = Utils::sorted(vOrig, [](int a, int b) { return a < b; });
+        QCOMPARE(rcl, vExpected);
+
+        // non-const rvalue
+        const auto vncr = [vOrig]() -> QList<int> { return vOrig; };
+        const QList<int> rncr = Utils::sorted(vncr(), [](int a, int b) { return a < b; });
+        QCOMPARE(rncr, vExpected);
+
+        // const rvalue
+        const auto vcr = [vOrig]() -> const QList<int> { return vOrig; };
+        const QList<int> rcr = Utils::sorted(vcr(), [](int a, int b) { return a < b; });
+        QCOMPARE(rcr, vExpected);
+    }
+
+    const QList<Struct> mvOrig({4, 3, 2, 1});
+    const QList<Struct> mvExpected({1, 2, 3, 4});
+    // member
+    {
+        // non-const lvalue
+        QList<Struct> mvncl = mvOrig;
+        const QList<Struct> rncl = Utils::sorted(mvncl, &Struct::member);
+        QCOMPARE(rncl, mvExpected);
+        QCOMPARE(mvncl, mvOrig); // was not modified
+
+        // const lvalue
+        const QList<Struct> rcl = Utils::sorted(mvOrig, &Struct::member);
+        QCOMPARE(rcl, mvExpected);
+
+        // non-const rvalue
+        const auto vncr = [mvOrig]() -> QList<Struct> { return mvOrig; };
+        const QList<Struct> rncr = Utils::sorted(vncr(), &Struct::member);
+        QCOMPARE(rncr, mvExpected);
+
+        // const rvalue
+        const auto vcr = [mvOrig]() -> const QList<Struct> { return mvOrig; };
+        const QList<Struct> rcr = Utils::sorted(vcr(), &Struct::member);
+        QCOMPARE(rcr, mvExpected);
+    }
+
+    // member function
+    {
+        // non-const lvalue
+        QList<Struct> mvncl = mvOrig;
+        const QList<Struct> rncl = Utils::sorted(mvncl, &Struct::getMember);
+        QCOMPARE(rncl, mvExpected);
+        QCOMPARE(mvncl, mvOrig); // was not modified
+
+        // const lvalue
+        const QList<Struct> rcl = Utils::sorted(mvOrig, &Struct::getMember);
+        QCOMPARE(rcl, mvExpected);
+
+        // non-const rvalue
+        const auto vncr = [mvOrig]() -> QList<Struct> { return mvOrig; };
+        const QList<Struct> rncr = Utils::sorted(vncr(), &Struct::getMember);
+        QCOMPARE(rncr, mvExpected);
+
+        // const rvalue
+        const auto vcr = [mvOrig]() -> const QList<Struct> { return mvOrig; };
+        const QList<Struct> rcr = Utils::sorted(vcr(), &Struct::getMember);
+        QCOMPARE(rcr, mvExpected);
+    }
+}
+
 QTEST_GUILESS_MAIN(tst_Algorithm)
 
 #include "tst_algorithm.moc"
-- 
cgit v1.2.1