From eedbcad7a403cec40d939ba3f6bc7901fdf22c24 Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Thu, 6 Jun 2024 11:27:33 +0200
Subject: [PATCH] QJniArray: allow container type for toContainer to be set
 explicitly
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add a template parameter that is defaulted to what our logic would
select as the appropritate container type. If the type is contiguous
and the element is primitive, use the optimized implementation with
the JNI helpers for copying the entire array region.

Otherwise, use an emplace_back loop.

Change-Id: I669bc3261f111bc0c7e3dd2af8fd33293c083801
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
(cherry picked from commit a6b1f80cd6c833f6eb2fbfb778254cd7fcbc000f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/kernel/qjniarray.h                | 74 ++++++++++---------
 src/corelib/kernel/qjniarray.qdoc             | 16 +++-
 .../kernel/qjniarray/tst_qjniarray.cpp        | 24 ++++++
 3 files changed, 77 insertions(+), 37 deletions(-)

diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h
index 41929c8e5fb..b1ebf3481ba 100644
--- a/src/corelib/kernel/qjniarray.h
+++ b/src/corelib/kernel/qjniarray.h
@@ -201,6 +201,13 @@ template <typename T>
 class QJniArray : public QJniArrayBase
 {
     friend struct QJniArrayIterator<T>;
+
+    template <typename E> struct ToContainerHelper { using type = QList<E>; };
+    template <> struct ToContainerHelper<jstring> { using type = QStringList; };
+    template <> struct ToContainerHelper<jbyte> { using type = QByteArray; };
+    template <typename E>
+    using ToContainerType = typename ToContainerHelper<E>::type;
+
 public:
     using Type = T;
 
@@ -306,7 +313,7 @@ public:
             else if constexpr (std::is_same_v<T, jshort>)
                 env->GetShortArrayRegion(object<jshortArray>(), i, 1, &res);
             else if constexpr (std::is_same_v<T, jint>)
-                env->GetIntArrayRegion(object<jbyteArray>(), i, 1, &res);
+                env->GetIntArrayRegion(object<jintArray>(), i, 1, &res);
             else if constexpr (std::is_same_v<T, jlong>)
                 env->GetLongArrayRegion(object<jlongArray>(), i, 1, &res);
             else if constexpr (std::is_same_v<T, jfloat>)
@@ -317,60 +324,59 @@ public:
         }
     }
 
-    auto toContainer() const
+    template <typename Container = ToContainerType<T>>
+    Container toContainer(Container &&container = {}) const
     {
+        const qsizetype sz = size();
+        if (!sz)
+            return std::forward<Container>(container);
         JNIEnv *env = jniEnv();
-        if constexpr (std::is_same_v<T, jobject>) {
-            QList<jobject> res;
-            res.reserve(size());
+
+        using ContainerType = q20::remove_cvref_t<Container>;
+
+        container.reserve(sz);
+        if constexpr (std::is_same_v<typename ContainerType::value_type, QString>) {
             for (auto element : *this)
-                res.append(element);
-            return res;
-        } else if constexpr (std::is_same_v<T, jstring>) {
-            QStringList res;
-            res.reserve(size());
+                container.emplace_back(QJniObject(element).toString());
+        } else if constexpr (std::is_base_of_v<std::remove_pointer_t<jobject>, std::remove_pointer_t<T>>) {
             for (auto element : *this)
-                res.append(QJniObject(element).toString());
-            return res;
-        } else if constexpr (std::is_same_v<T, jbyte>) {
-            const qsizetype bytecount = size();
-            QByteArray res(bytecount, Qt::Initialization::Uninitialized);
-            if (!isEmpty()) {
+                container.emplace_back(element);
+        } else if constexpr (QJniArrayBase::isContiguousContainer<ContainerType>) {
+            container.resize(sz);
+            if constexpr (std::is_same_v<T, jbyte>) {
                 env->GetByteArrayRegion(object<jbyteArray>(),
-                                        0, bytecount, reinterpret_cast<jbyte *>(res.data()));
-            }
-            return res;
-        } else {
-            QList<T> res;
-            if (isEmpty())
-                return res;
-            res.resize(size());
-            if constexpr (std::is_same_v<T, jchar>) {
+                                        0, sz,
+                                        reinterpret_cast<jbyte *>(container.data()));
+            } else if constexpr (std::is_same_v<T, jchar>) {
                 env->GetCharArrayRegion(object<jcharArray>(),
-                                        0, res.size(), res.data());
+                                        0, sz, container.data());
             } else if constexpr (std::is_same_v<T, jboolean>) {
                 env->GetBooleanArrayRegion(object<jbooleanArray>(),
-                                           0, res.size(), res.data());
+                                        0, sz, container.data());
             } else if constexpr (std::is_same_v<T, jshort>) {
                 env->GetShortArrayRegion(object<jshortArray>(),
-                                         0, res.size(), res.data());
+                                        0, sz, container.data());
             } else if constexpr (std::is_same_v<T, jint>) {
                 env->GetIntArrayRegion(object<jintArray>(),
-                                       0, res.size(), res.data());
+                                        0, sz, container.data());
             } else if constexpr (std::is_same_v<T, jlong>) {
                 env->GetLongArrayRegion(object<jlongArray>(),
-                                        0, res.size(), res.data());
+                                        0, sz, container.data());
             } else if constexpr (std::is_same_v<T, jfloat>) {
                 env->GetFloatArrayRegion(object<jfloatArray>(),
-                                         0, res.size(), res.data());
+                                        0, sz, container.data());
             } else if constexpr (std::is_same_v<T, jdouble>) {
                 env->GetDoubleArrayRegion(object<jdoubleArray>(),
-                                          0, res.size(), res.data());
+                                        0, sz, container.data());
             } else {
-                res.clear();
+                static_assert(QtPrivate::type_dependent_false<T>(),
+                              "Don't know how to copy data from a QJniArray of this type");
             }
-            return res;
+        } else {
+            for (auto e : *this)
+                container.emplace_back(e);
         }
+        return std::forward<Container>(container);
     }
 };
 
diff --git a/src/corelib/kernel/qjniarray.qdoc b/src/corelib/kernel/qjniarray.qdoc
index 20c3fbe3ee1..c909d4650af 100644
--- a/src/corelib/kernel/qjniarray.qdoc
+++ b/src/corelib/kernel/qjniarray.qdoc
@@ -393,12 +393,22 @@
 */
 
 /*!
-    \fn template <typename T> auto QJniArray<T>::toContainer() const
+    \fn template <typename T> template <typename Container>
+        Container QJniArray<T>::toContainer(Container &&container) const
 
-    \return the data in the wrapped Java objects as a suitable C++ container,
-    which will be empty if the array is \l{QJniArrayBase::isValid()}{invalid}.
+    \return a container populated with the data in the wrapped Java array.
 
+    If no \a container is provided, then the type of the container returned
+    depends on the element type of this QJniArray.
     \include qjniarray.qdoc type-mapping
 
+    If you pass in a named container (an lvalue) for \a container, then that
+    container is filled, and a reference to it is returned. If you pass in a
+    temporary container (an rvalue, incl. the default argument), then that
+    container is filled, and returned by value.
+
+    This function returns immediately if the array is
+    \l{QJniArrayBase::isValid()}{invalid}.
+
     \sa QJniArrayBase::fromContainer()
 */
diff --git a/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp b/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp
index 2f895b7b9ed..f56b2fe2b16 100644
--- a/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp
+++ b/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp
@@ -20,6 +20,7 @@ private slots:
     void invalidArraysAreEmpty();
     void size();
     void operators();
+    void toContainer();
 };
 
 using namespace QtJniTypes;
@@ -80,6 +81,17 @@ VERIFY_RETURN_FOR_TYPE(List, List);
 VERIFY_RETURN_FOR_TYPE(List[], QJniArray<List>);
 VERIFY_RETURN_FOR_TYPE(QJniArray<List>, QJniArray<List>);
 
+#define VERIFY_CONTAINER_FOR_TYPE(In, Out) \
+    static_assert(std::is_same_v<decltype(std::declval<In>().toContainer()), Out>)
+
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<jchar>, QList<jchar>);
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<jfloat>, QList<jfloat>);
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<jbyte>, QByteArray);
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<jstring>, QStringList);
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<jclass>, QList<jclass>);
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<QJniObject>, QList<QJniObject>);
+VERIFY_CONTAINER_FOR_TYPE(QJniArray<List>, QList<List>);
+
 void tst_QJniArray::construct()
 {
     // explicit
@@ -209,6 +221,18 @@ void tst_QJniArray::operators()
     }
 }
 
+void tst_QJniArray::toContainer()
+{
+    std::vector<jchar> charVector{u'a', u'b', u'c'};
+    QJniArray<jchar> charArray(charVector);
+
+    std::vector<jchar> vector;
+    charArray.toContainer(vector);
+
+    QCOMPARE(vector, charVector);
+    QCOMPARE(charArray.toContainer<std::vector<jchar>>(), charVector);
+}
+
 QTEST_MAIN(tst_QJniArray)
 
 #include "tst_qjniarray.moc"