QToolBox: replace a QList with a std::vector<std::unique_ptr>

This user of QList depended on the stability-of-reference guarantee of QList.
Make that explicit by using a vector of unique_ptrs. Adapt to new API.

This patch does not intend to fix any pre-existing problems with the code,
such as double-lookups. It is focused on getting rid of this questionable
use of QList, so the code doesn't explode when QList temporarily becomes
QVector in wip/qt6. Not more, not less.

Change-Id: I33847f33aa9f411ad9cd6c046653b7ab22a733cb
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

src/widgets/widgets/qtoolbox.cpp

@@ -50,6 +50,8 @@
 #include <qtooltip.h>
 #include <qabstractbutton.h>
 
+#include <private/qmemory_p.h>
+
 #include "qframe_p.h"
 
 QT_BEGIN_NAMESPACE
@@ -106,7 +108,7 @@ public:
             return widget == other.widget;
         }
     };
-    typedef QList<Page> PageList;
+    typedef std::vector<std::unique_ptr<Page>> PageList;
 
     inline QToolBoxPrivate()
         : currentPage(0)
@@ -130,26 +132,27 @@ public:
 const QToolBoxPrivate::Page *QToolBoxPrivate::page(const QObject *widget) const
 {
     if (!widget)
-        return 0;
+        return nullptr;
 
-    for (PageList::ConstIterator i = pageList.constBegin(); i != pageList.constEnd(); ++i)
+    for (const auto &page : pageList) {
-        if ((*i).widget == widget)
+        if (page->widget == widget)
-            return (const Page*) &(*i);
+            return page.get();
-    return 0;
+    }
+    return nullptr;
 }
 
 QToolBoxPrivate::Page *QToolBoxPrivate::page(int index)
 {
-    if (index >= 0 && index < pageList.size())
+    if (index >= 0 && index < static_cast<int>(pageList.size()))
-        return &pageList[index];
+        return pageList[index].get();
-    return 0;
+    return nullptr;
 }
 
 const QToolBoxPrivate::Page *QToolBoxPrivate::page(int index) const
 {
-    if (index >= 0 && index < pageList.size())
+    if (index >= 0 && index < static_cast<int>(pageList.size()))
-        return &pageList.at(index);
+        return pageList[index].get();
-    return 0;
+    return nullptr;
 }
 
 void QToolBoxPrivate::updateTabs()
@@ -157,13 +160,12 @@ void QToolBoxPrivate::updateTabs()
     QToolBoxButton *lastButton = currentPage ? currentPage->button : 0;
     bool after = false;
     int index = 0;
-    for (index = 0; index < pageList.count(); ++index) {
+    for (const auto &page : pageList) {
-        const Page &page = pageList.at(index);
+        QToolBoxButton *tB = page->button;
-        QToolBoxButton *tB = page.button;
         // update indexes, since the updates are delayed, the indexes will be correct
         // when we actually paint.
         tB->setIndex(index);
-        QWidget *tW = page.widget;
+        QWidget *tW = page->widget;
         if (after) {
             QPalette p = tB->palette();
             p.setColor(tB->backgroundRole(), tW->palette().color(tW->backgroundRole()));
@@ -174,6 +176,7 @@ void QToolBoxPrivate::updateTabs()
             tB->update();
         }
         after = tB == lastButton;
+        ++index;
     }
 }
 
@@ -345,7 +348,8 @@ int QToolBox::insertItem(int index, QWidget *widget, const QIcon &icon, const QS
     Q_D(QToolBox);
     connect(widget, SIGNAL(destroyed(QObject*)), this, SLOT(_q_widgetDestroyed(QObject*)));
 
-    QToolBoxPrivate::Page c;
+    auto newPage = qt_make_unique<QToolBoxPrivate::Page>();
+    auto &c = *newPage;
     c.widget = widget;
     c.button = new QToolBoxButton(this);
     c.button->setObjectName(QLatin1String("qt_toolbox_toolboxbutton"));
@@ -360,15 +364,15 @@ int QToolBox::insertItem(int index, QWidget *widget, const QIcon &icon, const QS
     c.setText(text);
     c.setIcon(icon);
 
-    if (index < 0 || index >= (int)d->pageList.count()) {
+    if (index < 0 || index >= static_cast<int>(d->pageList.size())) {
-        index = d->pageList.count();
+        index = static_cast<int>(d->pageList.size());
-        d->pageList.append(c);
+        d->pageList.push_back(std::move(newPage));
         d->layout->addWidget(c.button);
         d->layout->addWidget(c.sv);
         if (index == 0)
             setCurrentIndex(index);
     } else {
-        d->pageList.insert(index, c);
+        d->pageList.insert(d->pageList.cbegin() + index, std::move(newPage));
         d->relayout();
         if (d->currentPage) {
             QWidget *current = d->currentPage->widget;
@@ -391,12 +395,13 @@ void QToolBoxPrivate::_q_buttonClicked()
 {
     Q_Q(QToolBox);
     QToolBoxButton *tb = qobject_cast<QToolBoxButton*>(q->sender());
-    QWidget* item = 0;
+    QWidget* item = nullptr;
-    for (QToolBoxPrivate::PageList::ConstIterator i = pageList.constBegin(); i != pageList.constEnd(); ++i)
+    for (const auto &page : pageList) {
-        if ((*i).button == tb) {
+        if (page->button == tb) {
-            item = (*i).widget;
+            item = page->widget;
             break;
         }
+    }
 
     q->setCurrentIndex(q->indexOf(item));
 }
@@ -411,7 +416,7 @@ void QToolBoxPrivate::_q_buttonClicked()
 int QToolBox::count() const
 {
     Q_D(const QToolBox);
-    return d->pageList.count();
+    return static_cast<int>(d->pageList.size());
 }
 
 void QToolBox::setCurrentIndex(int index)
@@ -438,12 +443,18 @@ void QToolBoxPrivate::relayout()
     delete layout;
     layout = new QVBoxLayout(q);
     layout->setContentsMargins(QMargins());
-    for (QToolBoxPrivate::PageList::ConstIterator i = pageList.constBegin(); i != pageList.constEnd(); ++i) {
+    for (const auto &page : pageList) {
-        layout->addWidget((*i).button);
+        layout->addWidget(page->button);
-        layout->addWidget((*i).sv);
+        layout->addWidget(page->sv);
     }
 }
 
+auto pageEquals = [](const QToolBoxPrivate::Page *page) {
+    return [page](const std::unique_ptr<QToolBoxPrivate::Page> &ptr) {
+        return ptr.get() == page;
+    };
+};
+
 void QToolBoxPrivate::_q_widgetDestroyed(QObject *object)
 {
     Q_Q(QToolBox);
@@ -458,9 +469,9 @@ void QToolBoxPrivate::_q_widgetDestroyed(QObject *object)
     delete c->button;
 
     bool removeCurrent = c == currentPage;
-    pageList.removeAll(*c);
+    pageList.erase(std::remove_if(pageList.begin(), pageList.end(), pageEquals(c)), pageList.end());
 
-    if (!pageList.count()) {
+    if (pageList.empty()) {
         currentPage = 0;
         emit q->currentChanged(-1);
     } else if (removeCurrent) {
@@ -538,9 +549,9 @@ void QToolBox::setCurrentWidget(QWidget *widget)
 QWidget *QToolBox::widget(int index) const
 {
     Q_D(const QToolBox);
-    if (index < 0 || index >= (int) d->pageList.size())
+    if (index < 0 || index >= static_cast<int>(d->pageList.size()))
         return nullptr;
-    return d->pageList.at(index).widget;
+    return d->pageList[index]->widget;
 }
 
 /*!
@@ -552,7 +563,12 @@ int QToolBox::indexOf(QWidget *widget) const
 {
     Q_D(const QToolBox);
     const QToolBoxPrivate::Page *c = (widget ? d->page(widget) : 0);
-    return c ? d->pageList.indexOf(*c) : -1;
+    if (!c)
+        return -1;
+    const auto it = std::find_if(d->pageList.cbegin(), d->pageList.cend(), pageEquals(c));
+    if (it == d->pageList.cend())
+        return -1;
+    return static_cast<int>(it - d->pageList.cbegin());
 }
 
 /*!
@@ -571,7 +587,7 @@ void QToolBox::setItemEnabled(int index, bool enabled)
     if (!enabled && c == d->currentPage) {
         int curIndexUp = index;
         int curIndexDown = curIndexUp;
-        const int count = d->pageList.count();
+        const int count = static_cast<int>(d->pageList.size());
         while (curIndexUp > 0 || curIndexDown < count-1) {
             if (curIndexDown < count-1) {
                 if (d->page(++curIndexDown)->button->isEnabled()) {