QDirListing: make end() return a sentinel

This simplifies the implementation, and is more secure, because you no
longer can compare const_iterator with const_iterator and get a
non-sensical result (unequal, even though copies of each other).

Requires the STL algorithm test to be ported to C++20 ranges, because
classical STL algorithms can't deal with decltype(first) !=
decltype(last).

Found in API-review.

As a drive-by, make the reversed and inverted operators conditional on
!__cpp_impl_three_way_comparison.

Change-Id: Icd0e4dac277bd3b053f7b648f8c3f9d9f3753299
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
(cherry picked from commit 2569ca0f34fe15a2e5c828b708f1406be8d53836)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/corelib/io/qdirlisting.cpp

@@ -32,12 +32,16 @@
     files, recursively:
     \snippet code/src_corelib_io_qdirlisting.cpp 6
 
-    Iterators constructed by QDirListing (QDirListing::const_iterator) are
+    Iterators constructed by QDirListing (QDirListing::const_iterator)
+    model C++20
+    \l{https://en.cppreference.com/w/cpp/iterator/input_iterator}{std::input_iterator},
+    that is, they are
     forward-only, single-pass iterators, that don't allow random access. They
-    can be used in ranged-for loops (or with STL alogrithms that don't
+    can be used in ranged-for loops (or with C++20 range algorithms that don't
     require random access iterators). Dereferencing a valid iterator returns
-    a QDirListing::DirEntry object. The (c)end() iterator marks the end of
+    a QDirListing::DirEntry object. The (c)end() sentinel marks the end of
-    the iteration. Dereferencing the end iterator is undefined behavior.
+    the iteration. Dereferencing an iterator that is equal to \l{sentinel} is
+    undefined behavior.
 
     QDirListing::DirEntry offers a subset of QFileInfo's API (for example,
     fileName(), filePath(), exists()). Internally, DirEntry only constructs
@@ -660,8 +664,8 @@ QStringList QDirListing::nameFilters() const
 /*!
     \fn QDirListing::const_iterator QDirListing::begin() const
     \fn QDirListing::const_iterator QDirListing::cbegin() const
-    \fn QDirListing::const_iterator QDirListing::end() const
+    \fn QDirListing::sentinel QDirListing::end() const
-    \fn QDirListing::const_iterator QDirListing::cend() const
+    \fn QDirListing::sentinel QDirListing::cend() const
 
     (c)begin() returns a QDirListing::const_iterator that can be used to
     iterate over directory entries.
@@ -673,8 +677,8 @@ QStringList QDirListing::nameFilters() const
     \li Can be used in ranged-for loops; or with STL algorithms that don't
         require random access iterators
     \li Dereferencing a valid iterator returns a \c{const DirEntry &}
-    \li (c)end() returns a sentinel-like const_iterator that signals the
+    \li (c)end() returns a sentinel that signals the end of the iteration.
-        end of the iteration. Dereferencing the end() iterator is undefined
+        Dereferencing an iterator that compares equal to end() is undefined
         behavior
     \li Each time (c)begin() is called on the same QDirListing object,
         the internal state is reset and the iteration starts anew
@@ -689,6 +693,10 @@ QStringList QDirListing::nameFilters() const
     Here's how to find and read all files filtered by name, recursively:
     \snippet code/src_corelib_io_qdirlisting.cpp 1
 
+    \note The "classical" STL algorithms don't support iterator/sentinel, so
+    you need to use C++20 std::ranges algorithms fo QDirListing, or else a
+    3rd-party library that provides range-based algorithms in C++17.
+
     \sa QDirListing::DirEntry
 */
 QDirListing::const_iterator QDirListing::begin() const
@@ -719,7 +727,7 @@ QDirListing::const_iterator &QDirListing::const_iterator::operator++()
 {
     dirListPtr->advance();
     if (!dirListPtr->hasIterators())
-        *this = {}; // All done, make `this` the end() iterator
+        *this = {}; // All done, make `this` equal to the end() iterator
     return *this;
 }
 

src/corelib/io/qdirlisting.h

@@ -94,6 +94,12 @@ public:
         Q_CORE_EXPORT QDateTime fileTime(QFileDevice::FileTime type, const QTimeZone &tz) const;
     };
 
+    class sentinel
+    {
+        friend constexpr bool operator==(sentinel, sentinel) noexcept { return true; }
+        friend constexpr bool operator!=(sentinel, sentinel) noexcept { return false; }
+    };
+
     class const_iterator
     {
         friend class QDirListing;
@@ -112,23 +118,28 @@ public:
         pointer operator->() const { return &dirEntry; }
         Q_CORE_EXPORT const_iterator &operator++();
         const_iterator operator++(int) { auto tmp = *this; operator++(); return tmp; };
-        friend bool operator==(const const_iterator &lhs, const const_iterator &rhs) noexcept
+        friend bool operator==(const const_iterator &lhs, sentinel) noexcept
         {
-            // This is only used for the sentinel end iterator
+            return lhs.dirListPtr == nullptr;
-            return lhs.dirListPtr == nullptr && rhs.dirListPtr == nullptr;
         }
-        friend bool operator!=(const const_iterator &lhs, const const_iterator &rhs) noexcept
+#ifndef __cpp_impl_three_way_comparison
-        { return !(lhs == rhs); }
+        friend bool operator!=(const const_iterator &lhs, sentinel) noexcept
+        { return !operator==(lhs, sentinel{}); }
+        friend bool operator==(sentinel, const const_iterator &rhs) noexcept
+        { return operator==(rhs, sentinel{}); }
+        friend bool operator!=(sentinel, const const_iterator &rhs) noexcept
+        { return !operator==(sentinel{}, rhs); }
+#endif // __cpp_impl_three_way_comparison
     };
 
     Q_CORE_EXPORT const_iterator begin() const;
     const_iterator cbegin() const { return begin(); }
-    const_iterator end() const { return {}; }
+    sentinel end() const { return {}; }
-    const_iterator cend() const { return end(); }
+    sentinel cend() const { return end(); }
 
     // Qt compatibility
     const_iterator constBegin() const { return begin(); }
-    const_iterator constEnd() const { return end(); }
+    sentinel constEnd() const { return end(); }
 
 private:
     Q_DISABLE_COPY(QDirListing)

tests/auto/corelib/io/qdirlisting/tst_qdirlisting.cpp

@@ -23,6 +23,8 @@
 #include <QStandardPaths>
 #endif
 
+#include <algorithm>
+
 using namespace Qt::StringLiterals;
 
 Q_DECLARE_METATYPE(QDirListing::IteratorFlags)
@@ -748,18 +750,25 @@ void tst_QDirListing::hiddenDirs()
 
 void tst_QDirListing::withStdAlgorithms()
 {
+#ifndef __cpp_lib_ranges
+    QSKIP("This test requires C++20 ranges support enabled in the standard library");
+#else
+#ifdef __cpp_lib_concepts
+    static_assert(std::ranges::input_range<QDirListing&>);
+#endif
     QDirListing dirList(u"entrylist"_s, ItFlag::Recursive);
 
-    std::for_each(dirList.cbegin(), dirList.cend(), [](const auto &dirEntry) {
+    std::ranges::for_each(dirList.cbegin(), dirList.cend(), [](const auto &dirEntry) {
         QVERIFY(dirEntry.absoluteFilePath().contains("entrylist"));
     });
 
     const auto fileName = "dummy"_L1;
-    auto it = std::find_if(dirList.cbegin(), dirList.cend(), [fileName](const auto &dirEntry) {
+    auto it = std::ranges::find_if(dirList.cbegin(), dirList.cend(), [fileName](const auto &dirEntry) {
         return dirEntry.fileName() == fileName;
     });
     QVERIFY(it != dirList.cend());
     QCOMPARE(it->fileName(), fileName);
+#endif
 }
 
 QTEST_MAIN(tst_QDirListing)