Introduce QAbstractItemModel::checkIndex()
When implementing a custom model there's the habit, in each and every function that takes a QModelIndex, to carefully checking the index passed by the caller. This index is checked for "legality" (*): does the index belong to this model, is the index pointing to an existing row and column, and so on. These checks are hand-rolled and, as such, slightly different and possibly incomplete (i.e. wrong) every time. What's worse, these checks are implemented via "ordinary" code (if statements). However, passing an illegal index to a QAIM function is a precondition violation, and as such does not (and must not) be checked in ordinary conditions, as it triggers undefined behavior. On the other hand, while debugging a custom model or a custom hierarchy of (proxy) models, having such checks in place can be a significant aid. Enter checkIndex(): a debugging helper for QAbstractItemModel and its subclasses. checkIndex() centralizes the checks for legality of a given index. User code is free to assert on it, or have some other fallback mechanism in case a check fails. (*) Using "legality" here instead of "validity" in order to avoid confusion between QModelIndex::isValid() and what checkIndex() really does. [ChangeLog][QtCore][QAbstractItemModel] Added QAbstractItemModel::checkIndex(), a debugging function for QAbstractItemModel subclasses. Change-Id: I1eea0586b1ac3ededdbfbf46759145022dc5ad86 Reviewed-by: Thorbjørn Lund Martsum <tmartsum@gmail.com> Reviewed-by: David Faure <david.faure@kdab.com>
This commit is contained in:
Giuseppe D'Angelo
parent
9c53f4d33a
commit
69496d4e22
@ -48,11 +48,14 @@
|
||||
#include <qstack.h>
|
||||
#include <qbitarray.h>
|
||||
#include <qdatetime.h>
|
||||
#include <qloggingcategory.h>
|
||||
|
||||
#include <limits.h>
|
||||
|
||||
QT_BEGIN_NAMESPACE
|
||||
|
||||
Q_LOGGING_CATEGORY(lcCheckIndex, "qt.core.qabstractitemmodel.checkindex")
|
||||
|
||||
QPersistentModelIndexData *QPersistentModelIndexData::create(const QModelIndex &index)
|
||||
{
|
||||
Q_ASSERT(index.isValid()); // we will _never_ insert an invalid index in the list
|
||||
@ -3320,6 +3323,140 @@ QModelIndexList QAbstractItemModel::persistentIndexList() const
|
||||
return result;
|
||||
}
|
||||
|
||||
/*!
|
||||
\enum QAbstractItemModel::CheckIndexOption
|
||||
\since 5.11
|
||||
|
||||
This enum can be used to control the checks performed by
|
||||
QAbstractItemModel::checkIndex().
|
||||
|
||||
\value CheckIndexOption::NoOption No check options are specified.
|
||||
|
||||
\value CheckIndexOption::IndexIsValid The model index passed to
|
||||
QAbstractItemModel::checkIndex() is checked to be a valid model index.
|
||||
|
||||
\value CheckIndexOption::DoNotUseParent Does not perform any check
|
||||
involving the usage of the parent of the index passed to
|
||||
QAbstractItemModel::checkIndex().
|
||||
|
||||
\value CheckIndexOption::ParentIsInvalid The parent of the model index
|
||||
passed to QAbstractItemModel::checkIndex() is checked to be an invalid
|
||||
model index. If both this option and CheckIndexOption::DoNotUseParent
|
||||
are specified, then this option is ignored.
|
||||
*/
|
||||
|
||||
/*!
|
||||
\since 5.11
|
||||
|
||||
This function checks whether \a index is a legal model index for
|
||||
this model. A legal model index is either an invalid model index, or a
|
||||
valid model index for which all the following holds:
|
||||
|
||||
\list
|
||||
|
||||
\li the index' model is \c{this};
|
||||
\li the index' row is greater or equal than zero;
|
||||
\li the index' row is less than the row count for the index' parent;
|
||||
\li the index' column is greater or equal than zero;
|
||||
\li the index' column is less than the column count for the index' parent.
|
||||
|
||||
\endlist
|
||||
|
||||
The \a options argument may change some of these checks. If \a options
|
||||
contains \c{CheckIndexOption::IndexIsValid}, then \a index must be a valid
|
||||
index; this is useful when reimplementing functions such as \l{data()} or
|
||||
\l{setData()}, which expect valid indexes.
|
||||
|
||||
If \a options contains \c{CheckIndexOption::DoNotUseParent}, then the
|
||||
checks that would call \l{parent()} are omitted; this allows calling this
|
||||
function from a \l{parent()} reimplementation (otherwise, this would result
|
||||
in endless recursion and a crash).
|
||||
|
||||
If \a options does not contain \c{CheckIndexOption::DoNotUseParent}, and it
|
||||
contains \c{CheckIndexOption::ParentIsInvalid}, then an additional check is
|
||||
performed: the parent index is checked for not being valid. This is useful
|
||||
when implementing flat models such as lists or tables, where no model index
|
||||
should have a valid parent index.
|
||||
|
||||
This function returns true if all the checks succeeded, and false otherwise.
|
||||
This allows to use the function in \l{Q_ASSERT} and similar other debugging
|
||||
mechanisms. If some check failed, a warning message will be printed in the
|
||||
\c{qt.core.qabstractitemmodel.checkindex} logging category, containing
|
||||
some information that may be useful for debugging the failure.
|
||||
|
||||
\note This function is a debugging helper for implementing your own item
|
||||
models. When developing complex models, as well as when building
|
||||
complicated model hierarchies (e.g. using proxy models), it is useful to
|
||||
call this function in order to catch bugs relative to illegal model indices
|
||||
(as defined above) accidentally passed to some QAbstractItemModel API.
|
||||
|
||||
\warning Note that it's undefined behavior to pass illegal indices to item
|
||||
models, so applications must refrain from doing so, and not rely on any
|
||||
"defensive" programming that item models could employ to handle illegal
|
||||
indexes gracefully.
|
||||
|
||||
\sa QModelIndex
|
||||
*/
|
||||
bool QAbstractItemModel::checkIndex(const QModelIndex &index, CheckIndexOptions options) const
|
||||
{
|
||||
if (!index.isValid()) {
|
||||
if (options & CheckIndexOption::IndexIsValid) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index << "is not valid (expected valid)";
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
if (index.model() != this) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index
|
||||
<< "is for model" << index.model()
|
||||
<< "which is different from this model" << this;
|
||||
return false;
|
||||
}
|
||||
|
||||
if (index.row() < 0) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index
|
||||
<< "has negative row" << index.row();
|
||||
return false;
|
||||
}
|
||||
|
||||
if (index.column() < 0) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index
|
||||
<< "has negative column" << index.column();
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!(options & CheckIndexOption::DoNotUseParent)) {
|
||||
const QModelIndex parentIndex = index.parent();
|
||||
if (options & CheckIndexOption::ParentIsInvalid) {
|
||||
if (parentIndex.isValid()) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index
|
||||
<< "has valid parent" << parentIndex
|
||||
<< "(expected an invalid parent)";
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const int rc = rowCount(parentIndex);
|
||||
if (index.row() >= rc) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index
|
||||
<< "has out of range row" << index.row()
|
||||
<< "rowCount() is" << rc;
|
||||
return false;
|
||||
}
|
||||
|
||||
const int cc = columnCount(parentIndex);
|
||||
if (index.column() >= cc) {
|
||||
qCWarning(lcCheckIndex) << "Index" << index
|
||||
<< "has out of range column" << index.column()
|
||||
<< "columnCount() is" << cc;
|
||||
return false;
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/*!
|
||||
\class QAbstractTableModel
|
||||
|
||||
@ -250,6 +250,17 @@ public:
|
||||
};
|
||||
Q_ENUM(LayoutChangeHint)
|
||||
|
||||
enum class CheckIndexOption {
|
||||
NoOption = 0x0000,
|
||||
IndexIsValid = 0x0001,
|
||||
DoNotUseParent = 0x0002,
|
||||
ParentIsInvalid = 0x0004,
|
||||
};
|
||||
Q_ENUM(CheckIndexOption)
|
||||
Q_DECLARE_FLAGS(CheckIndexOptions, CheckIndexOption)
|
||||
|
||||
Q_REQUIRED_RESULT bool checkIndex(const QModelIndex &index, CheckIndexOptions options = CheckIndexOption::NoOption) const;
|
||||
|
||||
Q_SIGNALS:
|
||||
void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles = QVector<int>());
|
||||
void headerDataChanged(Qt::Orientation orientation, int first, int last);
|
||||
@ -343,6 +354,8 @@ private:
|
||||
Q_DISABLE_COPY(QAbstractItemModel)
|
||||
};
|
||||
|
||||
Q_DECLARE_OPERATORS_FOR_FLAGS(QAbstractItemModel::CheckIndexOptions)
|
||||
|
||||
inline bool QAbstractItemModel::insertRow(int arow, const QModelIndex &aparent)
|
||||
{ return insertRows(arow, 1, aparent); }
|
||||
inline bool QAbstractItemModel::insertColumn(int acolumn, const QModelIndex &aparent)
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
CONFIG += testcase
|
||||
TARGET = tst_qabstractitemmodel
|
||||
QT = core testlib
|
||||
QT = core testlib gui
|
||||
|
||||
mtdir = ../../../other/modeltest
|
||||
INCLUDEPATH += $$PWD/$${mtdir}
|
||||
|
||||
@ -31,6 +31,7 @@
|
||||
|
||||
#include <QtCore/QSortFilterProxyModel>
|
||||
#include <QtCore/QStringListModel>
|
||||
#include <QtGui/QStandardItemModel>
|
||||
|
||||
#include "dynamictreemodel.h"
|
||||
|
||||
@ -105,6 +106,8 @@ private slots:
|
||||
|
||||
void testFunctionPointerSignalConnection();
|
||||
|
||||
void checkIndex();
|
||||
|
||||
private:
|
||||
DynamicTreeModel *m_model;
|
||||
};
|
||||
@ -2283,6 +2286,75 @@ void tst_QAbstractItemModel::testFunctionPointerSignalConnection()
|
||||
// model.rowsInserted(QModelIndex(), 0, 0);
|
||||
}
|
||||
|
||||
void tst_QAbstractItemModel::checkIndex()
|
||||
{
|
||||
const QRegularExpression ignorePattern("^Index QModelIndex");
|
||||
|
||||
// checkIndex is QAbstractItemModel API; using QStandardItem as an easy
|
||||
// way to build a tree model
|
||||
QStandardItemModel model;
|
||||
QStandardItem *topLevel = new QStandardItem("topLevel");
|
||||
model.appendRow(topLevel);
|
||||
|
||||
topLevel->appendRow(new QStandardItem("child1"));
|
||||
topLevel->appendRow(new QStandardItem("child2"));
|
||||
|
||||
QVERIFY(model.checkIndex(QModelIndex()));
|
||||
QVERIFY(model.checkIndex(QModelIndex(), QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QVERIFY(model.checkIndex(QModelIndex(), QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(QModelIndex(), QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
|
||||
QModelIndex topLevelIndex = model.index(0, 0);
|
||||
QVERIFY(topLevelIndex.isValid());
|
||||
QVERIFY(model.checkIndex(topLevelIndex));
|
||||
QVERIFY(model.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QVERIFY(model.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QVERIFY(model.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
|
||||
QModelIndex childIndex = model.index(0, 0, topLevelIndex);
|
||||
QVERIFY(childIndex.isValid());
|
||||
QVERIFY(model.checkIndex(childIndex));
|
||||
QVERIFY(model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QVERIFY(model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
|
||||
childIndex = model.index(1, 0, topLevelIndex);
|
||||
QVERIFY(childIndex.isValid());
|
||||
QVERIFY(model.checkIndex(childIndex));
|
||||
QVERIFY(model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QVERIFY(model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
|
||||
topLevel->removeRow(1);
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(childIndex));
|
||||
QVERIFY(model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(childIndex, QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
|
||||
QStandardItemModel model2;
|
||||
model2.appendRow(new QStandardItem("otherTopLevel"));
|
||||
topLevelIndex = model2.index(0, 0);
|
||||
QVERIFY(topLevelIndex.isValid());
|
||||
QVERIFY(model2.checkIndex(topLevelIndex));
|
||||
QVERIFY(model2.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QVERIFY(model2.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QVERIFY(model2.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(topLevelIndex));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::DoNotUseParent));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::ParentIsInvalid));
|
||||
QTest::ignoreMessage(QtWarningMsg, ignorePattern);
|
||||
QVERIFY(!model.checkIndex(topLevelIndex, QAbstractItemModel::CheckIndexOption::IndexIsValid));
|
||||
}
|
||||
|
||||
QTEST_MAIN(tst_QAbstractItemModel)
|
||||
#include "tst_qabstractitemmodel.moc"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user