QRM(A): Allow initializing a raw nullptr with an object

The ownership mess is not a problem if there's nothing stored at the
position yet, and it allows gradually populating a row of raw object
pointers. Hook newly inserted objects up to the autoConnectProperties
mechanism.

Add test case.

Change-Id: Ie029a2a358e6a1ed5f24869039be9c2ad542dff9
Reviewed-by: Artem Dyomin <[email protected]>

Author: vohi

src/corelib/itemmodels/qrangemodel_impl.h

@@ -1373,25 +1373,34 @@ class QRangeModelImpl
                 using multi_role = QRangeModelDetails::is_multi_role<value_type>;
 
                 auto setRangeModelDataRole = [&target, &data]{
-                    auto &targetRef = QRangeModelDetails::refTo(target);
                     constexpr auto targetMetaType = QMetaType::fromType<value_type>();
                     const auto dataMetaType = data.metaType();
                     constexpr bool isWrapped = QRangeModelDetails::is_wrapped<value_type>();
                     if constexpr (!std::is_copy_assignable_v<wrapped_value_type>) {
                         // we don't support replacing objects that are stored as raw pointers,
                         // as this makes object ownership very messy. But we can replace objects
-                        // stored in smart pointers.
-                        if constexpr (isWrapped && !std::is_pointer_v<value_type>
-                                      && std::is_copy_assignable_v<value_type>) {
-                            if (data.canConvert(targetMetaType)) {
-                                target = data.value<value_type>();
-                                return true;
+                        // stored in smart pointers, and we can initialize raw nullptr objects.
+                        if constexpr (isWrapped) {
+                            constexpr bool is_raw_pointer = std::is_pointer_v<value_type>;
+                            if constexpr (!is_raw_pointer && std::is_copy_assignable_v<value_type>) {
+                                if (data.canConvert(targetMetaType)) {
+                                    target = data.value<value_type>();
+                                    return true;
+                                }
+                            } else if constexpr (is_raw_pointer) {
+                                if (!QRangeModelDetails::isValid(target) && data.canConvert(targetMetaType)) {
+                                    target = data.value<value_type>();
+                                    return true;
+                                }
+                            } else {
+                                Q_UNUSED(target);
                             }
                         }
                         // Otherwise we have a move-only or polymorph type. fall through to
                         // error handling.
                     } else if constexpr (isWrapped) {
                         if (QRangeModelDetails::isValid(target)) {
+                            auto &targetRef = QRangeModelDetails::refTo(target);
                             // we need to get a wrapped value type out of the QVariant, which
                             // might carry a pointer. We have to try all alternatives.
                             if (const auto mt = QMetaType::fromType<wrapped_value_type>();
@@ -1405,10 +1414,10 @@ class QRangeModelImpl
                             }
                         }
                     } else if (targetMetaType == dataMetaType) {
-                        targetRef = data.value<value_type>();
+                        QRangeModelDetails::refTo(target) = data.value<value_type>();
                         return true;
                     } else if (dataMetaType.flags() & QMetaType::PointerToGadget) {
-                        targetRef = *data.value<value_type *>();
+                        QRangeModelDetails::refTo(target) = *data.value<value_type *>();
                         return true;
                     }
 #ifndef QT_NO_DEBUG
@@ -1463,6 +1472,13 @@ class QRangeModelImpl
             };
 
             success = writeAt(index, writeData);
+
+            if constexpr (itemsAreQObjects) {
+                if (success && isRangeModelRole(role) && this->autoConnectPolicy() == AutoConnectPolicy::Full) {
+                    if (QObject *item = data.value<QObject *>())
+                        Self::connectProperties(index, item, m_data.context, m_data.properties);
+                }
+            }
         }
         return success;
     }

tests/auto/corelib/itemmodels/qrangemodeladapter/tst_qrangemodeladapter.cpp

@@ -2631,8 +2631,9 @@ class ObjectTreeItem : public ObjectRow
     auto &childRows() { return m_children; }
 
 private:
-    template <std::size_t I> // read-only is enough for this
-    friend decltype(auto) get(const ObjectTreeItem &row) { return row.m_objects[I]; }
+    template <std::size_t I, typename Item,
+        std::enable_if_t<std::is_same_v<q20::remove_cvref_t<Item>, ObjectTreeItem>, bool> = true>
+    friend decltype(auto) get(Item &&row) { return q23::forward_like<Item>(row.m_objects[I]); }
 
     ObjectTreeItem *m_parentRow = nullptr;
     std::optional<ObjectTree> m_children = std::nullopt;
@@ -2684,7 +2685,7 @@ void tst_QRangeModelAdapter::insertAutoConnectObjects()
     Object *newGrandChild = new Object;
     ObjectTreeItem newBranch(newChild);
     newBranch.childRows() = ObjectTree{
-        ObjectTreeItem(), // skip the first column
+        ObjectTreeItem(), // skip the first row to verify that we continue through nullptr
         ObjectTreeItem(newGrandChild),
         ObjectTreeItem()
     };
@@ -2698,9 +2699,11 @@ void tst_QRangeModelAdapter::insertAutoConnectObjects()
     QCOMPARE(dataChangedSpy.count(), 1);
     dataChangedSpy.clear();
 
-    // newGrandChild = new Object;
-    // adapter.at({0, 2, 0}, 0) = newGrandChild;
-    // newGrandChild->setString("0.2.0");
+    newGrandChild = new Object;
+    adapter.at({0, 2, 0}, 0) = newGrandChild;
+    QCOMPARE(dataChangedSpy.count(), 1);
+    newGrandChild->setString("0.2.0");
+    QCOMPARE(dataChangedSpy.count(), 2);
 }
 
 QTEST_MAIN(tst_QRangeModelAdapter)