QModelIndex: clean up integer size confusion in the API

QAIM::createIndex() took either int or quint32, but QMI::internalId()
returned qint64.

In the new interface, createIndex() takes, and internalId() provides,
integers of type quintptr.

This matches the storage size of the void* in the model index and
avoids truncation.

Remove the
  createIndex(int, int, quint32) and
  \obsolete createIndex(int,int,int)
overloads.

This makes a literal 0 in the third parameter ambiguous now.
The solutions have been noted in changes-5.0.0.

Change-Id: I0a0ecd8430eaf695129a4d09d14d4e30745485c4
Reviewed-by: Stephen Kelly <stephen.kelly@kdab.com>

diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0
index baa04d7..1d594b6 100644 (file)
--- a/dist/changes-5.0.0
+++ b/dist/changes-5.0.0
@@ -186,6 +186,15 @@ information about a particular change.
   * The signature of the createEditor and valuePropertyName methods
     have been changed to take arguments of type int instead of QVariant::Type.
 
+- QModelIndex/QAbstractItemModel
+
+  * The integer value that can be stored in a QModelIndex is now of type
+    quintptr to match the size of the internal storage location.
+  * The createIndex() method now only provides the void* and quintptr
+    overloads, making calls with a literal 0 (createIndex(row, col, 0))
+    ambiguous. Either cast (quintptr(0)) or omit the third argument
+    (to get the void* overload).
+
 - QWindowSystemInterface:
 
   * The signature of all handleTouchEvent() variants have changed,

diff --git a/src/corelib/itemmodels/qabstractitemmodel.cpp b/src/corelib/itemmodels/qabstractitemmodel.cpp
index e837cbf..7765415 100644 (file)
--- a/src/corelib/itemmodels/qabstractitemmodel.cpp
+++ b/src/corelib/itemmodels/qabstractitemmodel.cpp
@@ -328,15 +328,15 @@ void *QPersistentModelIndex::internalPointer() const
 }
 
 /*!
-    \fn void *QPersistentModelIndex::internalId() const
+    \fn quintptr QPersistentModelIndex::internalId() const
 
     \internal
 
-    Returns a \c{qint64} used by the model to associate the index with
+    Returns a \c{quintptr} used by the model to associate the index with
     the internal data structure.
 */
 
-qint64 QPersistentModelIndex::internalId() const
+quintptr QPersistentModelIndex::internalId() const
 {
     if (d)
         return d->index.internalId();
@@ -2345,15 +2345,7 @@ bool QAbstractItemModel::setHeaderData(int section, Qt::Orientation orientation,
 */
 
 /*!
-    \fn QModelIndex QAbstractItemModel::createIndex(int row, int column, int id) const
-    \obsolete
-
-    Use QModelIndex
-    QAbstractItemModel::createIndex(int row, int column, quint32 id) instead.
-*/
-
-/*!
-    \fn QModelIndex QAbstractItemModel::createIndex(int row, int column, quint32 id) const
+    \fn QModelIndex QAbstractItemModel::createIndex(int row, int column, quintptr id) const
 
     Creates a model index for the given \a row and \a column with the internal
     identifier, \a id.
@@ -3237,7 +3229,7 @@ QAbstractTableModel::~QAbstractTableModel()
 
 QModelIndex QAbstractTableModel::index(int row, int column, const QModelIndex &parent) const
 {
-    return hasIndex(row, column, parent) ? createIndex(row, column, 0) : QModelIndex();
+    return hasIndex(row, column, parent) ? createIndex(row, column) : QModelIndex();
 }
 
 /*!
@@ -3366,7 +3358,7 @@ QAbstractListModel::~QAbstractListModel()
 
 QModelIndex QAbstractListModel::index(int row, int column, const QModelIndex &parent) const
 {
-    return hasIndex(row, column, parent) ? createIndex(row, column, 0) : QModelIndex();
+    return hasIndex(row, column, parent) ? createIndex(row, column) : QModelIndex();
 }
 
 /*!

diff --git a/src/corelib/itemmodels/qabstractitemmodel.h b/src/corelib/itemmodels/qabstractitemmodel.h
index 78176c3..fdc4d6a 100644 (file)
--- a/src/corelib/itemmodels/qabstractitemmodel.h
+++ b/src/corelib/itemmodels/qabstractitemmodel.h
@@ -64,7 +64,7 @@ public:
     inline int row() const { return r; }
     inline int column() const { return c; }
     inline void *internalPointer() const { return p; }
-    inline qint64 internalId() const { return reinterpret_cast<qint64>(p); }
+    inline quintptr internalId() const { return quintptr(p); }
     inline QModelIndex parent() const;
     inline QModelIndex sibling(int row, int column) const;
     inline QModelIndex child(int row, int column) const;
@@ -88,7 +88,10 @@ public:
           }
           return false; }
 private:
-    inline QModelIndex(int row, int column, void *ptr, const QAbstractItemModel *model);
+    inline QModelIndex(int arow, int acolumn, void *ptr, const QAbstractItemModel *amodel)
+        : r(arow), c(acolumn), p(ptr), m(amodel) {}
+    inline QModelIndex(int arow, int acolumn, quintptr id, const QAbstractItemModel *amodel)
+        : r(arow), c(acolumn), p(reinterpret_cast<void*>(id)), m(amodel) {}
     int r, c;
     void *p;
     const QAbstractItemModel *m;
@@ -121,7 +124,7 @@ public:
     int row() const;
     int column() const;
     void *internalPointer() const;
-    qint64 internalId() const;
+    quintptr internalId() const;
     QModelIndex parent() const;
     QModelIndex sibling(int row, int column) const;
     QModelIndex child(int row, int column) const;
@@ -329,8 +332,7 @@ protected:
     QAbstractItemModel(QAbstractItemModelPrivate &dd, QObject *parent = 0);
 
     inline QModelIndex createIndex(int row, int column, void *data = 0) const;
-    inline QModelIndex createIndex(int row, int column, int id) const;
-    inline QModelIndex createIndex(int row, int column, quint32 id) const;
+    inline QModelIndex createIndex(int row, int column, quintptr id) const;
 
     void encodeData(const QModelIndexList &indexes, QDataStream &stream) const;
     bool decodeData(int row, int column, const QModelIndex &parent, QDataStream &stream);
@@ -400,25 +402,8 @@ inline bool QAbstractItemModel::moveColumn(const QModelIndex &sourceParent, int
 { return moveRows(sourceParent, sourceColumn, 1, destinationParent, destinationChild); }
 inline QModelIndex QAbstractItemModel::createIndex(int arow, int acolumn, void *adata) const
 { return QModelIndex(arow, acolumn, adata, this); }
-inline QModelIndex QAbstractItemModel::createIndex(int arow, int acolumn, int aid) const
-#if defined(Q_CC_MSVC)
-#pragma warning( push )
-#pragma warning( disable : 4312 ) // avoid conversion warning on 64-bit
-#endif
-{ return QModelIndex(arow, acolumn, reinterpret_cast<void*>(aid), this); }
-#if defined(Q_CC_MSVC)
-#pragma warning( pop )
-#endif
-inline QModelIndex QAbstractItemModel::createIndex(int arow, int acolumn, quint32 aid) const
-#if defined(Q_CC_MSVC)
-#pragma warning( push )
-#pragma warning( disable : 4312 ) // avoid conversion warning on 64-bit
-#endif
-{ return QModelIndex(arow, acolumn, reinterpret_cast<void*>(aid), this); }
-#if defined(Q_CC_MSVC)
-#pragma warning( pop )
-#endif
-
+inline QModelIndex QAbstractItemModel::createIndex(int arow, int acolumn, quintptr aid) const
+{ return QModelIndex(arow, acolumn, aid, this); }
 
 class Q_CORE_EXPORT QAbstractTableModel : public QAbstractItemModel
 {
@@ -463,10 +448,6 @@ private:
 
 // inline implementations
 
-inline QModelIndex::QModelIndex(int arow, int acolumn, void *adata,
-                                const QAbstractItemModel *amodel)
-    : r(arow), c(acolumn), p(adata), m(amodel) {}
-
 inline QModelIndex QModelIndex::parent() const
 { return m ? m->parent(*this) : QModelIndex(); }
 

diff --git a/src/widgets/itemviews/qtablewidget.cpp b/src/widgets/itemviews/qtablewidget.cpp
index 14d1491..b74d119 100644 (file)
--- a/src/widgets/itemviews/qtablewidget.cpp
+++ b/src/widgets/itemviews/qtablewidget.cpp
@@ -521,8 +521,8 @@ void QTableModel::sort(int column, Qt::SortOrder order)
                  : unsortable.at(i - sortable.count()));
         for (int c = 0; c < columnCount(); ++c) {
             sorted_table[tableIndex(i, c)] = item(r, c);
-            from.append(createIndex(r, c, 0));
-            to.append(createIndex(i, c, 0));
+            from.append(createIndex(r, c));
+            to.append(createIndex(i, c));
         }
     }
 

diff --git a/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp b/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp
index 88751b1..e0c024e 100644 (file)
--- a/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp
+++ b/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp
@@ -179,7 +179,7 @@ QtTestModel::QtTestModel(const QVector<QVector<QString> > tbl, QObject *parent)
 
 QModelIndex QtTestModel::index(int row, int column, const QModelIndex &parent) const
 {
-    return hasIndex(row, column, parent) ? createIndex(row, column, 0) : QModelIndex();
+    return hasIndex(row, column, parent) ? createIndex(row, column) : QModelIndex();
 }
 
 QModelIndex QtTestModel::parent(const QModelIndex &) const { return QModelIndex(); }

diff --git a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
index f7e9021..c86eed1 100644 (file)
--- a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
+++ b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp
@@ -1534,7 +1534,7 @@ public:
     }
     QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const
     {
-        return hasIndex(row, column, parent) ? createIndex(row, column, 0) : QModelIndex();
+        return hasIndex(row, column, parent) ? createIndex(row, column) : QModelIndex();
     }
     int rowCount(const QModelIndex & /* parent */) const
     {

diff --git a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
index 352349f..b2a8716 100644 (file)
--- a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
+++ b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
@@ -3972,7 +3972,7 @@ public:
             qint64 parentRowPlusOne = index.internalId();
             if (parentRowPlusOne > 0) {
                 int row = static_cast<int>(parentRowPlusOne - 1);
-                return createIndex(row, 0, (quint32)0);
+                return createIndex(row, 0);
             }
         }
         return QModelIndex();
@@ -3992,7 +3992,7 @@ public:
 
     QModelIndex index(int row, int column, const QModelIndex &parent) const
     {
-        return createIndex(row, column, parent.isValid() ? (quint32)(parent.row() + 1) : (quint32)0);
+        return createIndex(row, column, parent.isValid() ? (quintptr)(parent.row() + 1) : (quintptr)0);
     }
 
 public slots:
@@ -4003,16 +4003,16 @@ public slots:
 
         if (current.isValid()) {
             int selectedRow = current.row();
-            quint32 parentRowPlusOne = static_cast<quint32>(current.internalId());
+            const quintptr parentRowPlusOne = current.internalId();
 
             for (int i = 0; i < 2; ++i) {
                 // announce the removal of all non top level items
-                beginRemoveRows(createIndex(i, 0, 0), 0, 3);
+                beginRemoveRows(createIndex(i, 0), 0, 3);
                 // nothing to actually do for the removal
                 endRemoveRows();
 
                 // put them back in again
-                beginInsertRows(createIndex(i, 0, 0), 0, 3);
+                beginInsertRows(createIndex(i, 0), 0, 3);
                 // nothing to actually do for the insertion
                 endInsertRows();
             }