JNI: clean up move semantics for QJniArray

Explicitly define (compiler-implemented) copy and move SMF's for
QJniArrayBase; we need to, as the destructor is declared, so without
them the compiler will implicitly convert an array to a QJniObject
and call that constructor.

Constrain the constructors and assignment operator from a QJniArray of
another type so that no narrowing conversion is allowed. Due to the
implicit conversion to QJniObject, we have to explicitly delete the
overload for narrowing conversions. Use the detector we have in
qobjectdefs_impl.h for that.

Make the detection helpers private, and add test coverage.

Change-Id: I1b2bb4435d52223567d20bb55ceb0d516e3b0b15
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit ebbf7b0fdf866190cd20e62d6b13c7c6808101da)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/corelib/kernel/qjniarray.h

@@ -159,6 +159,13 @@ class QJniArrayBase
                                                      >
                                       > : std::true_type {};
 
+protected:
+    // these are used in QJniArray
+    template <typename From, typename To>
+    using if_convertible = std::enable_if_t<QtPrivate::AreArgumentsConvertibleWithoutNarrowingBase<From, To>::value, bool>;
+    template <typename From, typename To>
+    using unless_convertible = std::enable_if_t<!QtPrivate::AreArgumentsConvertibleWithoutNarrowingBase<From, To>::value, bool>;
+
 public:
     using size_type = jsize;
     using difference_type = size_type;
@@ -196,7 +203,7 @@ public:
                                                 std::is_same<ElementType, QString>,
                                                 std::is_base_of<QtJniTypes::JObjectBase, ElementType>
                              >) {
-            return QJniArray<ElementType>(makeObjectArray(std::forward<Container>(container)));
+            return QJniArray<ElementType>(makeObjectArray(std::forward<Container>(container)).arrayObject());
         } else if constexpr (QtJniTypes::sameTypeForJni<ElementType, jfloat>) {
             return makeArray<jfloat>(std::forward<Container>(container), &JNIEnv::NewFloatArray,
                                                              &JNIEnv::SetFloatArrayRegion);
@@ -233,6 +240,11 @@ protected:
     QJniArrayBase() = default;
     ~QJniArrayBase() = default;
 
+    explicit QJniArrayBase(const QJniArrayBase &other) = default;
+    explicit QJniArrayBase(QJniArrayBase &&other) noexcept = default;
+    QJniArrayBase &operator=(const QJniArrayBase &other) = default;
+    QJniArrayBase &operator=(QJniArrayBase &&other) noexcept = default;
+
     explicit QJniArrayBase(jarray array)
         : m_object(static_cast<jobject>(array))
     {
@@ -243,6 +255,16 @@ protected:
     explicit QJniArrayBase(QJniObject &&object) noexcept
         : m_object(std::move(object))
     {}
+    QJniArrayBase &operator=(const QJniObject &object)
+    {
+        m_object = object;
+        return *this;
+    }
+    QJniArrayBase &operator=(QJniObject &&object) noexcept
+    {
+        m_object = std::move(object);
+        return *this;
+    }
 
     JNIEnv *jniEnv() const noexcept { return QJniEnvironment::getJniEnv(); }
 
@@ -286,11 +308,34 @@ public:
     explicit QJniArray(const QJniObject &object) : QJniArrayBase(object) {}
     explicit QJniArray(QJniObject &&object) noexcept : QJniArrayBase(std::move(object)) {}
 
-    // base class destructor is protected, so need to provide all SMFs
+    template <typename Other, if_convertible<Other, T> = true>
-    QJniArray(const QJniArray &other) = default;
+    QJniArray(const QJniArray<Other> &other)
-    QJniArray(QJniArray &&other) noexcept = default;
+        : QJniArrayBase(other)
-    QJniArray &operator=(const QJniArray &other) = default;
+    {
-    QJniArray &operator=(QJniArray &&other) noexcept = default;
+    }
+    template <typename Other, if_convertible<Other, T> = true>
+    QJniArray(QJniArray<Other> &&other) noexcept
+        : QJniArrayBase(std::move(other))
+    {
+    }
+    template <typename Other, if_convertible<Other, T> = true>
+    QJniArray &operator=(const QJniArray<Other> &other)
+    {
+        QJniArrayBase::operator=(QJniObject(other));
+        return *this;
+    }
+    template <typename Other, if_convertible<Other, T> = true>
+    QJniArray &operator=(QJniArray<Other> &&other) noexcept
+    {
+        QJniArray moved(std::move(other));
+        swap(moved);
+        return *this;
+    }
+    // explicitly delete to disable detour via operator QJniObject()
+    template <typename Other, unless_convertible<Other, T> = true>
+    QJniArray(const QJniArray<Other> &other) = delete;
+    template <typename Other, unless_convertible<Other, T> = true>
+    QJniArray(QJniArray<Other> &&other) noexcept = delete;
 
     template <typename Container, if_contiguous_container<Container> = true>
     explicit QJniArray(Container &&container)
@@ -303,14 +348,6 @@ public:
     {
     }
 
-    template <typename Other>
-    using if_convertible = std::enable_if_t<std::is_convertible_v<Other, T>, bool>;
-
-    template <typename Other, if_convertible<Other> = true>
-    QJniArray(QJniArray<Other> &&other)
-        : QJniArrayBase(std::forward<QJniArray<Other>>(other))
-    {
-    }
     ~QJniArray() = default;
 
     auto arrayObject() const

src/corelib/kernel/qjniarray.qdoc

@@ -234,17 +234,10 @@
 */
 
 /*!
-    \fn template <typename T> QJniArray<T>::QJniArray(const QJniArray &other)
+    \fn template <typename T> template <typename Other, QJniArrayBase::if_convertible<Other, T>> QJniArray<T>::QJniArray(const QJniArray<Other> &other)
 
-    Constructs a QJniArray that is a copy of \a other. Both QJniArray objects
+    Constructs a QJniArray by copying \a other. Both QJniArray objects will
-    will reference the same Java array object.
+    reference the same Java array object.
-*/
-
-/*!
-    \fn template <typename T> template <typename Other, if_convertible<Other>> QJniArray<T>::QJniArray(QJniArray<Other> &&other)
-
-    Constructs a QJniArray that is a copy of \a other, with both objects referencing
-    the same Java array object.
 
     This constructor only participates in overload resolution if the element
     type \c{Other} of \a other is convertible to the element type \c{T} of the
@@ -252,24 +245,36 @@
 */
 
 /*!
-    \fn template <typename T> QJniArray<T>::QJniArray(QJniArray &&other)
+    \fn template <typename T> template <typename Other, QJniArrayBase::if_convertible<Other, T>> QJniArray<T>::QJniArray(QJniArray<Other> &&other)
 
     Constructs a QJniArray by moving from \a other. The \a other array becomes
     \l{QJniArrayBase::isValid}{invalid}.
+
+    This constructor only participates in overload resolution if the element
+    type \c{Other} of \a other is convertible to the element type \c{T} of the
+    QJniArray being constructed. However, no actual conversion takes place.
 */
 
 /*!
-    \fn template <typename T> QJniArray<T>::operator=(const QJniArray &other)
+    \fn template <typename T> template <typename Other, QJniArrayBase::if_convertible<Other, T>> QJniArray &QJniArray<T>::operator=(const QJniArray<Other> &other)
 
-    Assigns \a other to this QJniArray. Both QJniArray objects will reference
+    Assigns \a other to this QJniArray, and returns a reference to this. Both
-    the same Java array object.
+    QJniArray objects will reference the same Java array object.
+
+    This operator only participates in overload resolution if the element
+    type \c{Other} of \a other is convertible to the element type \c{T} of this
+    QJniArray. However, no actual conversion takes place.
 */
 
 /*!
-    \fn template <typename T> QJniArray<T>::operator=(QJniArray &&other)
+    \fn template <typename T> template <typename Other, QJniArrayBase::if_convertible<Other, T>> QJniArray &QJniArray<T>::operator=(QJniArray<Other> &&other)
 
-    Moves \a other into this QJniArray. The \a other array becomes
+    Moves \a other into this QJniArray, and returns a reference to this. The
-    \l{QJniArrayBase::isValid}{invalid}.
+    \a other array becomes \l{QJniArrayBase::isValid}{invalid}.
+
+    This operator only participates in overload resolution if the element
+    type \c{Other} of \a other is convertible to the element type \c{T} of this
+    QJniArray. However, no actual conversion takes place.
 */
 
 /*!

tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp

@@ -17,6 +17,7 @@ public:
 
 private slots:
     void construct();
+    void copyAndMove();
     void invalidArraysAreEmpty();
     void size();
     void operators();
@@ -183,6 +184,63 @@ void tst_QJniArray::construct()
     }
 }
 
+// Verify that we can convert QJniArrays into each other as long as element types
+// are convertible without narrowing.
+template <typename From, typename To>
+using CanConstructDetector = decltype(QJniArray<To>(std::declval<QJniArray<From>>()));
+template <typename From, typename To>
+using CanAssignDetector = decltype(std::declval<QJniArray<To>>().operator=(std::declval<QJniArray<From>>()));
+
+template <typename From, typename To>
+static constexpr bool canConstruct = qxp::is_detected_v<CanConstructDetector, From, To>;
+template <typename From, typename To>
+static constexpr bool canAssign = qxp::is_detected_v<CanAssignDetector, From, To>;
+
+static_assert(canConstruct<jshort, jint> && canAssign<jshort, jint>);
+static_assert(!canConstruct<jint, jshort> && !canAssign<jint, jshort>);
+static_assert(canConstruct<jstring, jobject> && canAssign<jstring, jobject>);
+static_assert(!canConstruct<jobject, jstring> && !canAssign<jobject, jstring>);
+
+// exercise the QJniArray(QJniArray<Other> &&other) constructor
+void tst_QJniArray::copyAndMove()
+{
+    QJniArray<jshort> tempShortArray({1, 2, 3});
+
+    // copy - both arrays remain valid and reference the same object
+    {
+        QJniArray<jshort> shortArrayCopy(tempShortArray);
+        QVERIFY(tempShortArray.isValid());
+        QVERIFY(shortArrayCopy.isValid());
+        QCOMPARE(tempShortArray, shortArrayCopy);
+    }
+
+    // moving QJniArray<T> to QJniArray<T> leaves the moved-from object invalid
+    QJniArray<jshort> shortArray(std::move(tempShortArray));
+    QVERIFY(!tempShortArray.isValid());
+    QVERIFY(shortArray.isValid());
+
+    tempShortArray = shortArray;
+
+    // copying QJniArray<short> to QJniArray<int> works
+    QJniArray<jint> intArray(shortArray);
+    QVERIFY(shortArray.isValid());
+    QVERIFY(intArray.isValid());
+    QCOMPARE(intArray, shortArray);
+
+    // moving QJniArray<short> to QJniArray<int> leaves the moved-from array invalid
+    QJniArray<jlong> longArray(std::move(shortArray));
+    QVERIFY(!shortArray.isValid());
+    QVERIFY(longArray.isValid());
+    QCOMPARE(longArray, intArray);
+    QCOMPARE_NE(longArray, shortArray); // we can compare a moved-from object
+
+    longArray = intArray;
+
+    // not possible due to narrowing conversion, covered by static_asserts above
+    // QJniArray<jshort> shortArray2(longArray);
+    // intArray = longArray;
+}
+
 void tst_QJniArray::invalidArraysAreEmpty()
 {
     QJniArray<jchar> invalid;