From e92d2f585269e74f11c3afbc23ff33885079b93f Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 16 May 2019 11:56:49 +0200 Subject: QToolBox: replace a QList with a std::vector 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 --- src/widgets/widgets/qtoolbox.cpp | 86 ++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 35 deletions(-) (limited to 'src/widgets/widgets/qtoolbox.cpp') diff --git a/src/widgets/widgets/qtoolbox.cpp b/src/widgets/widgets/qtoolbox.cpp index 1c83485bff..4d7f543a99 100644 --- a/src/widgets/widgets/qtoolbox.cpp +++ b/src/widgets/widgets/qtoolbox.cpp @@ -50,6 +50,8 @@ #include #include +#include + #include "qframe_p.h" QT_BEGIN_NAMESPACE @@ -106,7 +108,7 @@ public: return widget == other.widget; } }; - typedef QList PageList; + typedef std::vector> 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) - if ((*i).widget == widget) - return (const Page*) &(*i); - return 0; + for (const auto &page : pageList) { + if (page->widget == widget) + return page.get(); + } + return nullptr; } QToolBoxPrivate::Page *QToolBoxPrivate::page(int index) { - if (index >= 0 && index < pageList.size()) - return &pageList[index]; - return 0; + if (index >= 0 && index < static_cast(pageList.size())) + return pageList[index].get(); + return nullptr; } const QToolBoxPrivate::Page *QToolBoxPrivate::page(int index) const { - if (index >= 0 && index < pageList.size()) - return &pageList.at(index); - return 0; + if (index >= 0 && index < static_cast(pageList.size())) + return pageList[index].get(); + 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) { - const Page &page = pageList.at(index); - QToolBoxButton *tB = page.button; + for (const auto &page : pageList) { + 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(); + 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()) { - index = d->pageList.count(); - d->pageList.append(c); + if (index < 0 || index >= static_cast(d->pageList.size())) { + index = static_cast(d->pageList.size()); + 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(q->sender()); - QWidget* item = 0; - for (QToolBoxPrivate::PageList::ConstIterator i = pageList.constBegin(); i != pageList.constEnd(); ++i) - if ((*i).button == tb) { - item = (*i).widget; + QWidget* item = nullptr; + for (const auto &page : pageList) { + if (page->button == tb) { + 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(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) { - layout->addWidget((*i).button); - layout->addWidget((*i).sv); + for (const auto &page : pageList) { + layout->addWidget(page->button); + layout->addWidget(page->sv); } } +auto pageEquals = [](const QToolBoxPrivate::Page *page) { + return [page](const std::unique_ptr &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(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(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(d->pageList.size()); while (curIndexUp > 0 || curIndexDown < count-1) { if (curIndexDown < count-1) { if (d->page(++curIndexDown)->button->isEnabled()) { -- cgit v1.2.3