From 056e78f456fb050b7fc28d0dbd8ebd25d9eb291f Mon Sep 17 00:00:00 2001 From: Isak Fyksen Date: Fri, 10 Nov 2023 16:59:04 +0100 Subject: [PATCH] Add iterator, begin() and end() for QDomNodeList Implement iterator for QDomNodeList, based on iterator from QVersionNumber. Also, extract node-iteration from createList() to forEachNode(), and have both createList() and new noexceptLength() use this for appending to the QList and counting nodes without allocating them respectively. noexceptLength() is needed for noexcept [c?r?]begin/[c?r?]end methods. Add tests using begin(), end(), rbegin(), rend(), and range-based for-loop. std::reverse_iterator::operator-> does not work with ArrowProxy in C++17. This issue will be addressed in QTBUG-131933. [ChangeLog][QtXml][QDomNodeList] Added iterator support. Fixes: QTBUG-115076 Change-Id: I94570bda3bda00ab7938e9ff0472a42e38425838 Reviewed-by: Marc Mutz --- src/xml/dom/qdom.cpp | 81 +++++++++++++++++++-- src/xml/dom/qdom.h | 102 +++++++++++++++++++++++++++ src/xml/dom/qdom_p.h | 5 ++ tests/auto/xml/dom/qdom/tst_qdom.cpp | 98 +++++++++++++++++++++++++ 4 files changed, 282 insertions(+), 4 deletions(-) diff --git a/src/xml/dom/qdom.cpp b/src/xml/dom/qdom.cpp index e2faa54aab0..e1a4a36aa08 100644 --- a/src/xml/dom/qdom.cpp +++ b/src/xml/dom/qdom.cpp @@ -649,22 +649,29 @@ void QDomNodeListPrivate::createList() const if (!node_impl) return; + list.clear(); const QDomDocumentPrivate *const doc = node_impl->ownerDocument(); if (doc && timestamp != doc->nodeListTime) timestamp = doc->nodeListTime; + forEachNode([&](QDomNodePrivate *p){ list.append(p); }); +} + +void QDomNodeListPrivate::forEachNode(qxp::function_ref yield) const +{ + if (!node_impl) + return; QDomNodePrivate* p = node_impl->first; - list.clear(); if (tagname.isNull()) { while (p) { - list.append(p); + yield(p); p = p->next; } } else if (nsURI.isNull()) { while (p && p != node_impl) { if (p->isElement() && p->nodeName() == tagname) { - list.append(p); + yield(p); } if (p->first) p = p->first; @@ -681,7 +688,7 @@ void QDomNodeListPrivate::createList() const } else { while (p && p != node_impl) { if (p->isElement() && p->name==tagname && p->namespaceURI==nsURI) { - list.append(p); + yield(p); } if (p->first) p = p->first; @@ -726,6 +733,13 @@ int QDomNodeListPrivate::length() const return list.size(); } +int QDomNodeListPrivate::noexceptLength() const noexcept +{ + int count = 0; + forEachNode([&](QDomNodePrivate*){ ++count; }); + return count; +} + /************************************************************** * * QDomNodeList @@ -851,6 +865,16 @@ int QDomNodeList::length() const return impl->length(); } +/*! + Returns the number of nodes without creating the underlying QList. +*/ +int QDomNodeList::noexceptLength() const noexcept +{ + if (!impl) + return 0; + return impl->noexceptLength(); +} + /*! \fn bool QDomNodeList::isEmpty() const @@ -881,6 +905,55 @@ int QDomNodeList::length() const true). */ +/*! + \typedef QDomNodeList::const_iterator + \typedef QDomNodeList::const_reverse_iterator + \since 6.9 + + Typedefs for an opaque class that implements a (reverse) random-access + iterator over a QDomNodeList. + + \note QDomNodeList does not support modifying nodes in-place, so + there is no mutable iterator. +*/ + +/*! + \typedef QDomNodeList::value_type + \typedef QDomNodeList::difference_type + \typedef QDomNodeList::size_type + \typedef QDomNodeList::reference + \typedef QDomNodeList::const_reference + \typedef QDomNodeList::pointer + \typedef QDomNodeList::const_pointer + \since 6.9 + + Provided for STL-compatibility. + + \note QDomNodeList does not support modifying nodes in-place, so + reference and const_reference are the same type, as are pointer and + const_pointer. +*/ + +/*! + \fn QDomNodeList::begin() const + \fn QDomNodeList::end() const; + \fn QDomNodeList::rbegin() const + \fn QDomNodeList::rend() const; + \fn QDomNodeList::cbegin() const + \fn QDomNodeList::cend() const; + \fn QDomNodeList::crbegin() const + \fn QDomNodeList::crend() const; + \fn QDomNodeList::constBegin() const; + \fn QDomNodeList::constEnd() const; + \since 6.9 + + Returns a const_iterator or const_reverse_iterator, respectively, pointing + to the first or one past the last item in the list. + + \note QDomNodeList does not support modifying nodes in-place, so + there is no mutable iterator. +*/ + /************************************************************** * * QDomNodePrivate diff --git a/src/xml/dom/qdom.h b/src/xml/dom/qdom.h index 22a8f41d8ca..c7213074e25 100644 --- a/src/xml/dom/qdom.h +++ b/src/xml/dom/qdom.h @@ -7,8 +7,11 @@ #include #include +#include #include +#include + #if QT_CONFIG(dom) class tst_QDom; @@ -237,6 +240,105 @@ private: Q_XML_EXPORT friend bool comparesEqual(const QDomNodeList &lhs, const QDomNodeList &rhs) noexcept; Q_DECLARE_EQUALITY_COMPARABLE(QDomNodeList) + int noexceptLength() const noexcept; + + class It + { + const QDomNodeList *list; + int i; + + friend class QDomNodeList; + explicit constexpr It(const QDomNodeList *lp, int ii) noexcept : list(lp), i(ii) {} + + friend constexpr bool comparesEqual(const It &lhs, const It &rhs) + { Q_ASSERT(lhs.list == rhs.list); return lhs.i == rhs.i; } + friend constexpr Qt::strong_ordering compareThreeWay(const It &lhs, const It &rhs) + { Q_ASSERT(lhs.list == rhs.list); return Qt::compareThreeWay(lhs.i, rhs.i); } + // macro variant does not exist: Q_DECLARE_STRONGLY_ORDERED_LITERAL_TYPE_NON_NOEXCEPT(It) + friend constexpr bool operator==(It lhs, It rhs) { + return comparesEqual(lhs, rhs); + } +#ifdef __cpp_lib_three_way_comparison + friend constexpr std::strong_ordering operator<=>(It lhs, It rhs) { + return compareThreeWay(lhs, rhs); + } +#else + friend constexpr bool operator!=(It lhs, It rhs) { + return !comparesEqual(lhs, rhs); + } + friend constexpr bool operator<(It lhs, It rhs) { + return is_lt(compareThreeWay(lhs, rhs)); + } + friend constexpr bool operator<=(It lhs, It rhs) { + return is_lteq(compareThreeWay(lhs, rhs)); + } + friend constexpr bool operator>(It lhs, It rhs) { + return is_gt(compareThreeWay(lhs, rhs)); + } + friend constexpr bool operator>=(It lhs, It rhs) { + return is_gteq(compareThreeWay(lhs, rhs)); + } +#endif + + public: + // Rule Of Zero applies + It() = default; + + using iterator_category = std::random_access_iterator_tag; + using value_type = QDomNode; + using element_type = const QDomNode; + using difference_type = qptrdiff; // difference to [container.reqmts] + using size_type = int; // difference to [container.reqmts] + using reference = value_type; // difference to [container.reqmts] + using pointer = QtPrivate::ArrowProxy; + + reference operator*() const { return list->item(i); } + pointer operator->() const { return { **this }; } + + It &operator++() { ++i; return *this; } + It operator++(int) { auto copy = *this; ++*this; return copy; } + + It &operator--() { --i; return *this; } + It operator--(int) { auto copy = *this; --*this; return copy; } + + It &operator+=(difference_type n) { i += n; return *this; } + friend It operator+(It it, difference_type n) { it += n; return it; } + friend It operator+(difference_type n, It it) { return it + n; } + + It &operator-=(difference_type n) { i -= n; return *this; } + friend It operator-(It it, difference_type n) { it -= n; return it; } + + friend difference_type operator-(It lhs, It rhs) + { Q_ASSERT(lhs.list == rhs.list); return lhs.i - rhs.i; } + + reference operator[](difference_type n) const { return *(*this + n); } + }; + +public: + using const_iterator = It; + using const_reverse_iterator = std::reverse_iterator; + + using value_type = It::value_type; + using difference_type = It::difference_type; + using size_type = It::size_type; + using reference = It::reference; + using const_reference = reference; + using pointer = It::pointer; + using const_pointer = pointer; + + [[nodiscard]] const_iterator begin() const noexcept { return It{this, 0}; } + [[nodiscard]] const_iterator end() const noexcept { return It{this, noexceptLength()}; } + [[nodiscard]] const_iterator cbegin() const noexcept { return begin(); } + [[nodiscard]] const_iterator cend() const noexcept { return end(); } + + [[nodiscard]] const_reverse_iterator rbegin() const noexcept { return const_reverse_iterator{end()}; } + [[nodiscard]] const_reverse_iterator rend() const noexcept { return const_reverse_iterator{begin()}; } + [[nodiscard]] const_reverse_iterator crbegin() const noexcept { return rbegin(); } + [[nodiscard]] const_reverse_iterator crend() const noexcept { return rend(); } + + [[nodiscard]] const_iterator constBegin() const noexcept { return begin(); } + [[nodiscard]] const_iterator constEnd() const noexcept { return end(); } + private: QDomNodeListPrivate* impl; QDomNodeList(QDomNodeListPrivate*); diff --git a/src/xml/dom/qdom_p.h b/src/xml/dom/qdom_p.h index 55f5b2332d3..2ead7a32e2d 100644 --- a/src/xml/dom/qdom_p.h +++ b/src/xml/dom/qdom_p.h @@ -12,6 +12,9 @@ #include QT_REQUIRE_CONFIG(dom); + +#include + QT_BEGIN_NAMESPACE // @@ -145,9 +148,11 @@ public: bool operator==(const QDomNodeListPrivate &) const noexcept; void createList() const; + void forEachNode(qxp::function_ref yield) const; bool maybeCreateList() const; QDomNodePrivate *item(int index); int length() const; + int noexceptLength() const noexcept; QAtomicInt ref; /* diff --git a/tests/auto/xml/dom/qdom/tst_qdom.cpp b/tests/auto/xml/dom/qdom/tst_qdom.cpp index 2d3a15cbde3..e267f8ae9d2 100644 --- a/tests/auto/xml/dom/qdom/tst_qdom.cpp +++ b/tests/auto/xml/dom/qdom/tst_qdom.cpp @@ -68,6 +68,8 @@ private slots: void ownerElementTask45192_data(); void ownerElementTask45192(); void domNodeMapAndList(); + void domNodeListIterator(); + void domNodeListReverseIterator(); void nullDocument(); void invalidName_data(); @@ -1313,6 +1315,102 @@ void tst_QDom::domNodeMapAndList() QCOMPARE(list.item(1).nodeName(), QString()); // Make sure we don't assert } +void tst_QDom::domNodeListIterator() +{ + const auto xml = "" + "" + "" + "" + ""_L1; + QDomDocument doc; + QVERIFY(doc.setContent(xml)); + QDomNodeList list = doc.elementsByTagName("bar"); + + QCOMPARE_EQ(list.begin(), list.begin()); + QCOMPARE_EQ(list.end(), list.end()); + QCOMPARE_NE(list.begin(), list.end()); + QCOMPARE_LT(list.begin(), list.end()); + QCOMPARE_GE(list.end(), list.begin()); + + auto it = list.begin(); + it++; + ++it; + it++; + QVERIFY(it == list.end()); + it--; + --it; + it--; + QVERIFY(it == list.begin()); + it += 3; + QVERIFY(it == list.end()); + it -= 3; + QVERIFY(it == list.begin()); + it = it + 3; + QVERIFY(it == list.end()); + it = it - 3; + QVERIFY(it == list.begin()); + it = 3 + it; + QVERIFY(it == list.end()); + + QCOMPARE(list.size(), 3); + + for (int i = 0; i < list.size(); ++i) { + QCOMPARE(list.item(i).attributes().item(0).nodeValue().toInt(), i); + } + + int i = 0; + for (auto iter = list.begin(); iter != list.end(); ++iter) + QCOMPARE(iter->attributes().item(0).nodeValue().toInt(), i++); + + int j = 0; + for (const auto &item : list) + QCOMPARE(item.attributes().item(0).nodeValue().toInt(), j++); +} + +void tst_QDom::domNodeListReverseIterator() +{ + const auto xml = "" + "" + "" + "" + ""_L1; + QDomDocument doc; + QVERIFY(doc.setContent(xml)); + QDomNodeList list = doc.elementsByTagName("bar"); + + QCOMPARE_EQ(list.rbegin(), list.rbegin()); + QCOMPARE_EQ(list.rend(), list.rend()); + QCOMPARE_NE(list.rbegin(), list.rend()); + QCOMPARE_LT(list.rbegin(), list.rend()); + QCOMPARE_GE(list.rend(), list.rbegin()); + + auto it = list.rbegin(); + it++; + ++it; + it++; + QVERIFY(it == list.rend()); + it--; + --it; + it--; + QVERIFY(it == list.rbegin()); + it += 3; + QVERIFY(it == list.rend()); + it -= 3; + QVERIFY(it == list.rbegin()); + it = it + 3; + QVERIFY(it == list.rend()); + it = it - 3; + QVERIFY(it == list.rbegin()); + it = 3 + it; + QVERIFY(it == list.rend()); + +#if __cplusplus >= 202002L // QTBUG-131933 + int i = 2; + for (auto iter = list.rbegin(); iter != list.rend(); ++iter) + QCOMPARE(iter->attributes().item(0).nodeValue().toInt(), i--); +#endif +} + // Verifies that a default-constructed QDomDocument is null, and that calling // any of the factory functions causes it to be non-null. #define TEST_NULL_DOCUMENT(func) \