Remove Java iterator APIs for moving back if operator-- is not available

QSet's iterator doesn't have operator--, but the
Q_DECLARE_SEQUENTIAL_ITERATOR macro declared and implemented functions
hasPrevious/previous/peekPrevious/findPrevious for the QSetIterator
anyway, depending on that operator. The resulting code couldn't compile.

Use SFINAE to remove the various "previous" functions from Java-style
iterators if operator--() is not present.

This removes the hasPrevious() API as well, even though it could compile
(as we only check whether the iterator points at constBegin). But since
nothing useful can be done with that information, it's best to remove
that member function as well.

[ChangeLog][Potentially source-incompatible changes] QSetIterator no
longer has a hasPrevious() member function. The underlying iterator
doesn't implement operator--(), so couldn't be moved backwards anyway.

Fixes: QTBUG-113379
Change-Id: I47b0ba384d8fcd127123d8fa509cd89e10ea8c99
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/tools/qcontainertools_impl.h

@@ -302,6 +302,15 @@ template <typename Iterator>
 using IfAssociativeIteratorHasFirstAndSecond =
     std::enable_if_t<qxp::is_detected_v<FirstAndSecondTest, Iterator>, bool>;
 
+template <typename Iterator>
+using MoveBackwardsTest = decltype(
+    std::declval<Iterator &>().operator--()
+);
+
+template <typename Iterator>
+using IfIteratorCanMoveBackwards =
+    std::enable_if_t<qxp::is_detected_v<MoveBackwardsTest, Iterator>, bool>;
+
 template <typename T, typename U>
 using IfIsNotSame =
     typename std::enable_if<!std::is_same<T, U>::value, bool>::type;

src/corelib/tools/qiterator.h

@@ -10,6 +10,13 @@ QT_BEGIN_NAMESPACE
 
 #if !defined(QT_NO_JAVA_STYLE_ITERATORS)
 
+#ifdef Q_QDOC
+#define Q_DISABLE_BACKWARD_ITERATOR
+#else
+#define Q_DISABLE_BACKWARD_ITERATOR \
+        template<typename It = decltype(i), QtPrivate::IfIteratorCanMoveBackwards<It> = true>
+#endif
+
 #define Q_DECLARE_SEQUENTIAL_ITERATOR(C) \
 \
 template <class T> \
@@ -28,11 +35,15 @@ public: \
     inline bool hasNext() const { return i != c.constEnd(); } \
     inline const T &next() { return *i++; } \
     inline const T &peekNext() const { return *i; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool hasPrevious() const { return i != c.constBegin(); } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline const T &previous() { return *--i; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline const T &peekPrevious() const { const_iterator p = i; return *--p; } \
     inline bool findNext(const T &t) \
     { while (i != c.constEnd()) if (*i++ == t) return true; return false; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool findPrevious(const T &t) \
     { while (i != c.constBegin()) if (*(--i) == t) return true; \
       return false;  } \
@@ -59,8 +70,11 @@ public: \
     inline bool hasNext() const { return c->constEnd() != const_iterator(i); } \
     inline T &next() { n = i++; return *n; } \
     inline T &peekNext() const { return *i; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool hasPrevious() const { return c->constBegin() != const_iterator(i); } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline T &previous() { n = --i; return *n; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline T &peekPrevious() const { iterator p = i; return *--p; } \
     inline void remove() \
     { if (c->constEnd() != const_iterator(n)) { i = c->erase(n); n = c->end(); } } \
@@ -70,6 +84,7 @@ public: \
     inline void insert(const T &t) { n = i = c->insert(i, t); ++i; } \
     inline bool findNext(const T &t) \
     { while (c->constEnd() != const_iterator(n = i)) if (*i++ == t) return true; return false; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool findPrevious(const T &t) \
     { while (c->constBegin() != const_iterator(i)) if (*(n = --i) == t) return true; \
       n = c->end(); return false;  } \
@@ -95,13 +110,17 @@ public: \
     inline bool hasNext() const { return i != c.constEnd(); } \
     inline Item next() { n = i++; return n; } \
     inline Item peekNext() const { return i; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool hasPrevious() const { return i != c.constBegin(); } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline Item previous() { n = --i; return n; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline Item peekPrevious() const { const_iterator p = i; return --p; } \
     inline const T &value() const { Q_ASSERT(item_exists()); return *n; } \
     inline const Key &key() const { Q_ASSERT(item_exists()); return n.key(); } \
     inline bool findNext(const T &t) \
     { while ((n = i) != c.constEnd()) if (*i++ == t) return true; return false; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool findPrevious(const T &t) \
     { while (i != c.constBegin()) if (*(n = --i) == t) return true; \
       n = c.constEnd(); return false; } \
@@ -129,8 +148,11 @@ public: \
     inline bool hasNext() const { return const_iterator(i) != c->constEnd(); } \
     inline Item next() { n = i++; return n; } \
     inline Item peekNext() const { return i; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool hasPrevious() const { return const_iterator(i) != c->constBegin(); } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline Item previous() { n = --i; return n; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline Item peekPrevious() const { iterator p = i; return --p; } \
     inline void remove() \
     { if (const_iterator(n) != c->constEnd()) { i = c->erase(n); n = c->end(); } } \
@@ -140,6 +162,7 @@ public: \
     inline const Key &key() const { Q_ASSERT(item_exists()); return n.key(); } \
     inline bool findNext(const T &t) \
     { while (const_iterator(n = i) != c->constEnd()) if (*i++ == t) return true; return false; } \
+    Q_DISABLE_BACKWARD_ITERATOR \
     inline bool findPrevious(const T &t) \
     { while (const_iterator(i) != c->constBegin()) if (*(n = --i) == t) return true; \
       n = c->end(); return false; } \

src/corelib/tools/qiterator.qdoc

@@ -500,7 +500,6 @@
 */
 
 /*! \fn template <class T> bool QListIterator<T>::hasPrevious() const
-    \fn template <class T> bool QSetIterator<T>::hasPrevious() const
     \fn template <class T> bool QMutableListIterator<T>::hasPrevious() const
 
     Returns \c true if there is at least one item behind the iterator,
@@ -511,7 +510,6 @@
 */
 
 /*! \fn template <class T> const T &QListIterator<T>::previous()
-    \fn template <class T> const T &QSetIterator<T>::previous()
 
     Returns the previous item and moves the iterator back by one
     position.
@@ -534,7 +532,6 @@
 */
 
 /*! \fn template <class T> const T &QListIterator<T>::peekPrevious() const
-    \fn template <class T> const T &QSetIterator<T>::peekPrevious() const
 
     Returns the previous item without moving the iterator.
 
@@ -580,7 +577,6 @@
 */
 
 /*! \fn template <class T> bool QListIterator<T>::findPrevious(const T &value)
-    \fn template <class T> bool QSetIterator<T>::findPrevious(const T &value)
     \fn template <class T> bool QMutableListIterator<T>::findPrevious(const T &value)
 
     Searches for \a value starting from the current iterator position