From 4a19e97f70994bc8bb67e26533d18266b8a615bc Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Mon, 18 Dec 2023 12:28:56 +0100
Subject: JNI API review: minor cleanups of QJniArray
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

While the QJniArray type still is expected to be a QJniObject, we should
still make the QJniArrayBase type non-constructible by making the
constructors protected. Also make the destructor of that type protected
and add explicit default implementations of the SMF.

Name the SFINAE helper and predciate consistently as IfCanConvert.

Pick-to: 6.7
Change-Id: I7a7c6a320fe9acb0758b53d94832d355c99cb6bb
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/corelib/kernel/qjniarray.h  | 50 +++++++++++++++++++++++++----------------
 src/corelib/kernel/qjniobject.h |  4 ++--
 2 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h
index 743314f12b..d690137ac0 100644
--- a/src/corelib/kernel/qjniarray.h
+++ b/src/corelib/kernel/qjniarray.h
@@ -70,18 +70,6 @@ class QJniArrayBase : public QJniObject
                            > : std::true_type {};
 
 public:
-    explicit QJniArrayBase(jarray array)
-        : QJniObject(static_cast<jobject>(array))
-    {
-        static_assert(sizeof(QJniArrayBase) == sizeof(QJniObject),
-                      "QJniArrayBase must have the same size as QJniObject!");
-    }
-    explicit QJniArrayBase(const QJniObject &object)
-        : QJniObject(object)
-    {}
-    explicit QJniArrayBase(QJniObject &&object) noexcept
-        : QJniObject(std::move(object))
-    {}
 
     qsizetype size() const
     {
@@ -91,9 +79,11 @@ public:
     }
 
     template <typename Container>
-    static constexpr bool CanConvert = CanConvertHelper<Container>::value;
+    static constexpr bool canConvert = CanConvertHelper<Container>::value;
+    template <typename Container>
+    using IfCanConvert = std::enable_if_t<canConvert<Container>, bool>;
     template <typename Container
-        , std::enable_if_t<CanConvert<Container>, bool> = true
+        , IfCanConvert<Container> = true
     >
     static auto fromContainer(Container &&container)
     {
@@ -136,6 +126,20 @@ public:
 
 protected:
     QJniArrayBase() = default;
+    ~QJniArrayBase() = default;
+
+    explicit QJniArrayBase(jarray array)
+        : QJniObject(static_cast<jobject>(array))
+    {
+        static_assert(sizeof(QJniArrayBase) == sizeof(QJniObject),
+                      "QJniArrayBase must have the same size as QJniObject!");
+    }
+    explicit QJniArrayBase(const QJniObject &object)
+        : QJniObject(object)
+    {}
+    explicit QJniArrayBase(QJniObject &&object) noexcept
+        : QJniObject(std::move(object))
+    {}
 
     template <typename ElementType, typename List, typename NewFn, typename SetFn>
     static auto makeArray(List &&list, NewFn &&newArray, SetFn &&setRegion);
@@ -151,10 +155,19 @@ public:
     using Type = T;
     using const_iterator = const QJniArrayIterator<T>;
 
-    using QJniArrayBase::QJniArrayBase;
+    QJniArray() = default;
+    explicit QJniArray(jarray array) : QJniArrayBase(array) {}
+    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
+    QJniArray(const QJniArray &other) = default;
+    QJniArray(QJniArray &&other) noexcept = default;
+    QJniArray &operator=(const QJniArray &other) = default;
+    QJniArray &operator=(QJniArray &&other) noexcept = default;
 
     template <typename Container
-        , std::enable_if_t<QJniArrayBase::CanConvert<Container>, bool> = true
+        , IfCanConvert<Container> = true
     >
     explicit QJniArray(Container &&container);
 
@@ -326,12 +339,11 @@ auto QJniArrayBase::makeObjectArray(List &&list)
 
 template <typename T>
 template <typename Container
-    , std::enable_if_t<QJniArrayBase::CanConvert<Container>, bool>
+    , QJniArrayBase::IfCanConvert<Container>
 >
 QJniArray<T>::QJniArray(Container &&container)
-    : QJniArrayBase()
+    : QJniArrayBase(QJniArrayBase::fromContainer(std::forward<Container>(container)))
 {
-    *this = QJniArrayBase::fromContainer(std::forward<Container>(container));
 }
 
 namespace QtJniTypes
diff --git a/src/corelib/kernel/qjniobject.h b/src/corelib/kernel/qjniobject.h
index ee111b49cd..292596d208 100644
--- a/src/corelib/kernel/qjniobject.h
+++ b/src/corelib/kernel/qjniobject.h
@@ -815,7 +815,7 @@ auto QJniObject::LocalFrame<Args...>::convertToJni(T &&value)
         return newLocalRef<jstring>(QJniObject::fromString(value));
     } else if constexpr (QtJniTypes::IsJniArray<Type>::value) {
         return value.arrayObject();
-    } else if constexpr (QJniArrayBase::CanConvert<T>) {
+    } else if constexpr (QJniArrayBase::canConvert<T>) {
         using QJniArrayType = decltype(QJniArrayBase::fromContainer(std::forward<T>(value)));
         using ArrayType = decltype(std::declval<QJniArrayType>().arrayObject());
         return newLocalRef<ArrayType>(QJniArrayBase::fromContainer(std::forward<T>(value)).template object<jobject>());
@@ -836,7 +836,7 @@ auto QJniObject::LocalFrame<Args...>::convertFromJni(QJniObject &&object)
         return object.toString();
     } else if constexpr (QtJniTypes::IsJniArray<Type>::value) {
         return T{object};
-    } else if constexpr (QJniArrayBase::CanConvert<Type>) {
+    } else if constexpr (QJniArrayBase::canConvert<Type>) {
         // if we were to create a QJniArray from Type...
         using QJniArrayType = decltype(QJniArrayBase::fromContainer(std::declval<Type>()));
         // then that QJniArray would have elements of type
-- 
cgit v1.2.3