QGIM: support move-only row types

A move-only type can be used as the row if the container of the rows
can be populated via insert from another container. This is the case for
the standard containers, but not for e.g. QList.

If an insert(pos, start, end) overload is available, then use that also
in the moveRows() implementation to move the data over.

The only limitation now is that we cannot assign a complete row type as
data of an item, as we cannot get a move-only value out of a const
QVariant.

Change-Id: Icea66ed82c601fd819f7db2eca2f8e023d8aa7c2
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
This commit is contained in:
Volker Hilsheimer 2025-03-17 18:45:52 +01:00
parent 046b8523f2
commit 07804a83c4
4 changed files with 79 additions and 27 deletions

View File

@ -196,6 +196,38 @@ protected:
std::is_pointer_v<Range>,
Range, std::remove_reference_t<Range>>
>;
// A iterator type to use as the input iterator with the
// range_type::insert(pos, start, end) overload if available (it is in
// std::vector, but not in QList). Generates a prvalue when dereferenced,
// which then gets moved into the newly constructed row, which allows us to
// implement insertRows() for move-only row types.
struct EmptyRowGenerator
{
using value_type = row_type;
using reference = value_type;
using pointer = value_type *;
using iterator_category = std::input_iterator_tag;
using difference_type = int;
value_type operator*() { return impl.makeEmptyRow(parent); }
EmptyRowGenerator &operator++() { ++n; return *this; }
friend bool operator==(const EmptyRowGenerator &lhs, const EmptyRowGenerator &rhs) noexcept
{ return lhs.n == rhs.n; }
friend bool operator!=(const EmptyRowGenerator &lhs, const EmptyRowGenerator &rhs) noexcept
{ return !(lhs == rhs); }
difference_type n = 0;
Structure &impl;
const QModelIndex parent;
};
// If we have a move-only row_type and can add/remove rows, then the range
// must have an insert-from-range overload.
static_assert(static_row_count || range_features::has_insert_range
|| std::is_copy_constructible_v<row_type>,
"The range holding a move-only row-type must support insert(pos, start, end)");
public:
explicit QGenericItemModelImpl(Range &&model, QGenericItemModel *itemModel)
: QGenericItemModelImplBase(itemModel, static_cast<const Self*>(nullptr))
@ -473,8 +505,13 @@ public:
using multi_role = QGenericItemModelDetails::is_multi_role<value_type>;
if constexpr (has_metaobject<value_type>) {
if (QMetaType::fromType<value_type>() == data.metaType()) {
target = data.value<value_type>();
return true;
if constexpr (std::is_copy_assignable_v<value_type>) {
target = std::move(data).value<value_type>();
return true;
} else {
qCritical("Cannot assign %s", QMetaType::fromType<value_type>().name());
return false;
}
} else if (row_traits::fixed_size() <= 1) {
return writeRole(role, target, data);
} else if (column <= row_traits::fixed_size()
@ -739,14 +776,19 @@ public:
beginInsertRows(parent, row, row + count - 1);
const auto pos = std::next(std::begin(*children), row);
if constexpr (rows_are_pointers) {
if constexpr (range_features::has_insert_range) {
EmptyRowGenerator first{0, that(), parent};
EmptyRowGenerator last{count, that(), parent};
children->insert(pos, first, last);
} else if constexpr (rows_are_pointers) {
auto start = children->insert(pos, count, nullptr);
auto end = std::next(start, count);
for (auto it = start; it != end; ++it)
*it = that().makeEmptyRow(parent);
} else {
row_type empty_value = that().makeEmptyRow(parent);
children->insert(pos, count, empty_value);
children->insert(pos, count, that().makeEmptyRow(parent));
}
if constexpr (!rows_are_pointers) {
// fix the parent in all children of the modified row, as the
// references back to the parent might have become invalid.
that().resetParentInChildren(children);

View File

@ -20,6 +20,7 @@
#include <QtCore/qvariant.h>
#include <functional>
#include <iterator>
#include <type_traits>
#include <QtCore/q20type_traits.h>
#include <tuple>
@ -74,6 +75,20 @@ namespace QGenericItemModelDetails
: std::true_type
{};
// Can we insert from another (identical) range? Required to support
// move-only types
template <typename C, typename = void>
struct test_insert_range : std::false_type {};
template <typename C>
struct test_insert_range<C, std::void_t<decltype(std::declval<C&>().insert(
std::declval<typename C::const_iterator&>(),
std::declval<std::move_iterator<typename C::iterator>&>(),
std::declval<std::move_iterator<typename C::iterator>&>()
))>>
: std::true_type
{};
template <typename C, typename = void>
struct test_erase : std::false_type {};
@ -127,6 +142,7 @@ namespace QGenericItemModelDetails
struct range_traits : std::false_type {
static constexpr bool is_mutable = !std::is_const_v<C>;
static constexpr bool has_insert = false;
static constexpr bool has_insert_range = false;
static constexpr bool has_erase = false;
static constexpr bool has_resize = false;
};
@ -139,6 +155,7 @@ namespace QGenericItemModelDetails
using value_type = std::remove_reference_t<decltype(*std::begin(std::declval<C&>()))>;
static constexpr bool is_mutable = !std::is_const_v<C> && !std::is_const_v<value_type>;
static constexpr bool has_insert = test_insert<C>();
static constexpr bool has_insert_range = test_insert_range<C>();
static constexpr bool has_erase = test_erase<C>();
static constexpr bool has_resize = test_resize<C>();
};

View File

@ -18,7 +18,11 @@ qt_internal_add_test(tst_qgenericitemmodel
Qt::Gui
)
if (NOT INTEGRITY AND NOT VXWORKS)
if (
# INTEGRITY and VxWorks don't come with all the concepts headers
NOT INTEGRITY AND NOT VXWORKS AND
# gcc 10.2.0 chokes in a standard library header
(NOT CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 10.2.0))
if ("${CMAKE_CXX_COMPILE_FEATURES}" MATCHES "cxx_std_20")
set_property(TARGET tst_qgenericitemmodel PROPERTY CXX_STANDARD 20)
endif()

View File

@ -145,7 +145,7 @@ namespace std {
}
struct tree_row;
using value_tree = QList<tree_row>;
using value_tree = std::vector<tree_row>;
using pointer_tree = QList<tree_row *>;
struct tree_row
@ -161,22 +161,6 @@ public:
qDeleteAll(*m_childrenPointers);
}
tree_row(const tree_row &other)
: m_value(other.m_value), m_description(other.m_description)
, m_parent(other.m_parent), m_children(other.m_children)
, m_childrenPointers(other.m_childrenPointers)
{}
tree_row &operator=(const tree_row &other)
{
m_parent = other.m_parent;
m_children = other.m_children;
m_childrenPointers = other.m_childrenPointers;
m_value = other.m_value;
m_description = other.m_description;
return *this;
}
tree_row(tree_row &&other) = default;
tree_row &operator=(tree_row &&other) = default;
@ -384,11 +368,13 @@ private:
if (children) {
for (const auto &child : *children) {
const bool match = [&child, &row]{
tree_row emptyRow;
if constexpr (pointerTree) {
if (child->parentRow() != row) {
qCritical().noquote() << "Parent out of sync for:" << *child;
qCritical().noquote() << " Actual: " << child->parentRow()
<< (child->parentRow() ? *child->parentRow() : tree_row{});
<< std::move(child->parentRow() ? *child->parentRow()
: emptyRow);
qCritical().noquote() << "Expected: " << row << *row;
return false;
}
@ -396,7 +382,8 @@ private:
if (child.parentRow() != std::addressof(row)) {
qCritical().noquote() << "Parent out of sync for:" << child;
qCritical().noquote() << " Actual: " << child.parentRow()
<< (child.parentRow() ? *child.parentRow() : tree_row{});
<< std::move(child.parentRow() ? *child.parentRow()
: emptyRow);
qCritical().noquote() << "Expected: " << std::addressof(row) << row;
return false;
}
@ -1210,13 +1197,15 @@ enum class TreeProtocol { ValueImplicit, ValueReadOnly, PointerExplicit, Pointer
void tst_QGenericItemModel::createTree()
{
m_data->m_tree.reset(new value_tree{
tree_row root[] = {
{"1", "one"},
{"2", "two"},
{"3", "three"},
{"4", "four"},
{"5", "five"},
});
};
m_data->m_tree.reset(new value_tree{std::make_move_iterator(std::begin(root)),
std::make_move_iterator(std::end(root))});
(*m_data->m_tree)[1].addChild("2.1", "two.one");
(*m_data->m_tree)[1].addChild("2.2", "two.two");