[libc++][ranges] Addresses: LWG3719 - Directory iterators should be usable with default sentinel

[H-G-Hristov]

Summary

Addresses: LWG3719 - Directory iterators should be usable with default sentinel
https://wg21.link/LWG3719

[H-G-Hristov]

Minor tweak

[H-G-Hristov]

Try to fix CI

[H-G-Hristov]

• Fixed includes

[H-G-Hristov]

• Fixed includes
• Fix CI

[H-G-Hristov]

Try again CI

[H-G-Hristov]

Try to fix CI

[Mordante]

Thanks for working on this! Several minor comments.

libcxx/include/__filesystem/directory_iterator.h

106

This should be C++20. The LWG issues are normally bugs in the C++ Standard. We apply these to all versions of C++ back to C++03. However default_sentinel_t was introduced in C++20, so it requires C++20 or newer.

libcxx/include/filesystem

322

Thanks for updating the synopsis!

libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp

26

Note for these small tests I don't mind having all code in main.

29

Please test the following properties too

• returns a bool using AssertEqualityReturnBool
• noexcept using AssertEqualityAreNoexcept

[H-G-Hristov]

Addressed comments

[H-G-Hristov]

Tweaks

[H-G-Hristov]

@Mordante Thank you for the review!

libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp

29

Thank you for the tip!

[H-G-Hristov]

Updated sinopsises + Rebased

[Mordante]

LGTM, but please make sure the CI passes in C++20 before committing.

libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp

10

Please update the other tests too.

30

In this case I would omit the extra scope, but I don't strongly object either.

[H-G-Hristov]

Addressed comments

[H-G-Hristov]

libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp

10

Thank you! Sorry about this oversight.

libcxx/docs/Status/Cxx23Issues.csv

 "`3707 <https://wg21.link/LWG3707>`__","chunk_view::outer-iterator::value_type::size should return unsigned type","July 2022","","","|ranges|"
 "`3708 <https://wg21.link/LWG3708>`__","``take_while_view::sentinel``'s conversion constructor should move","July 2022","|Complete|","16.0","|ranges|"
 "`3709 <https://wg21.link/LWG3709>`__","LWG-3703 was underly ambitious","July 2022","",""
 "`3710 <https://wg21.link/LWG3710>`__","The ``end`` of ``chunk_view`` for input ranges can be ``const``","July 2022","","","|ranges|"
 "`3711 <https://wg21.link/LWG3711>`__","Missing preconditions for slide_view constructor","July 2022","","","|ranges|"
 "`3712 <https://wg21.link/LWG3712>`__","``chunk_view`` and ``slide_view`` should not be ``default_initializable``","July 2022","","","|ranges|"
 "`3713 <https://wg21.link/LWG3713>`__","Sorted with respect to comparator (only)","July 2022","",""
 "`3715 <https://wg21.link/LWG3715>`__","``view_interface::empty`` is overconstrained","July 2022","","","|ranges|"
-"`3719 <https://wg21.link/LWG3719>`__","Directory iterators should be usable with default sentinel","July 2022","","","|ranges|"
+"`3719 <https://wg21.link/LWG3719>`__","Directory iterators should be usable with default sentinel","July 2022","|Complete|","17.0","|ranges|"
 "`3721 <https://wg21.link/LWG3721>`__","Allow an ``arg-id`` with a value of zero for ``width`` in ``std-format-spec``","July 2022","|Complete|","16.0","|format|"
 "`3724 <https://wg21.link/LWG3724>`__","``decay-copy`` should be constrained","July 2022","|Complete|","14.0"
 "","","","","",""
 "`3028 <https://wg21.link/LWG3028>`__","Container requirements tables should distinguish ``const`` and non-``const`` variables", "November 2022","","",""
 "`3118 <https://wg21.link/LWG3118>`__","``fpos`` equality comparison unspecified", "November 2022","","",""
 "`3177 <https://wg21.link/LWG3177>`__","Limit permission to specialize variable templates to program-defined types", "November 2022","|Nothing to do|","",""
 "`3515 <https://wg21.link/LWG3515>`__","§[stacktrace.basic.nonmem]: ``operator<<`` should be less templatized", "November 2022","","",""
 "`3545 <https://wg21.link/LWG3545>`__","``std::pointer_traits`` should be SFINAE-friendly", "November 2022","","",""

libcxx/include/__filesystem/directory_iterator.h

 #define _LIBCPP___FILESYSTEM_DIRECTORY_ITERATOR_H
 
 #include <__assert>
 #include <__availability>
 #include <__config>
 #include <__filesystem/directory_entry.h>
 #include <__filesystem/directory_options.h>
 #include <__filesystem/path.h>
+#include <__iterator/default_sentinel.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/shared_ptr.h>
 #include <__ranges/enable_borrowed_range.h>
 #include <__ranges/enable_view.h>
 #include <__system_error/error_code.h>
 #include <__utility/move.h>
 #include <cstddef>
 
__dir_element_proxy operator++(int) {
     __dir_element_proxy __p(**this);
     __increment();
     return __p;
   }
 
   _LIBCPP_HIDE_FROM_ABI
   directory_iterator& increment(error_code& __ec) { return __increment(&__ec); }
 
+#  if _LIBCPP_STD_VER >= 20
+
+  _LIBCPP_HIDE_FROM_ABI bool operator==(default_sentinel_t) const noexcept { return *this == directory_iterator(); }
+
+#  endif
+
 private:
   inline _LIBCPP_HIDE_FROM_ABI friend bool
   operator==(const directory_iterator& __lhs,
              const directory_iterator& __rhs) noexcept;
 
   // construct the dir_stream
   _LIBCPP_EXPORTED_FROM_ABI directory_iterator(const path&, error_code*, directory_options = directory_options::none);
 

libcxx/include/__filesystem/recursive_directory_iterator.h

 #ifndef _LIBCPP___FILESYSTEM_RECURSIVE_DIRECTORY_ITERATOR_H
 #define _LIBCPP___FILESYSTEM_RECURSIVE_DIRECTORY_ITERATOR_H
 
 #include <__availability>
 #include <__config>
 #include <__filesystem/directory_entry.h>
 #include <__filesystem/directory_options.h>
 #include <__filesystem/path.h>
+#include <__iterator/default_sentinel.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/shared_ptr.h>
 #include <__ranges/enable_borrowed_range.h>
 #include <__ranges/enable_view.h>
 #include <__system_error/error_code.h>
 #include <__utility/move.h>
 #include <cstddef>
 
public:
   void pop(error_code& __ec) { __pop(&__ec); }
 
   _LIBCPP_INLINE_VISIBILITY
   bool recursion_pending() const { return __rec_; }
 
   _LIBCPP_INLINE_VISIBILITY
   void disable_recursion_pending() { __rec_ = false; }
 
+#  if _LIBCPP_STD_VER >= 20
+
+  _LIBCPP_HIDE_FROM_ABI bool operator==(default_sentinel_t) const noexcept {
+    return *this == recursive_directory_iterator();
+  }
+
+#  endif
+
 private:
   _LIBCPP_EXPORTED_FROM_ABI recursive_directory_iterator(const path& __p, directory_options __opt, error_code* __ec);
   _LIBCPP_EXPORTED_FROM_ABI const directory_entry& __dereference() const;
   _LIBCPP_EXPORTED_FROM_ABI bool __try_recursion(error_code* __ec);
   _LIBCPP_EXPORTED_FROM_ABI void __advance(error_code* __ec = nullptr);
   _LIBCPP_EXPORTED_FROM_ABI recursive_directory_iterator& __increment(error_code* __ec = nullptr);
   _LIBCPP_EXPORTED_FROM_ABI void __pop(error_code* __ec = nullptr);
 

libcxx/include/filesystem

public:
         friend basic_ostream<charT, traits>&
           operator<<(basic_ostream<charT, traits>& os, const directory_entry& d);
 
     private:
       filesystem::path pathobject;        // exposition only
       friend class directory_iterator;    // exposition only
     };
 
-    class directory_iterator;
+  class directory_iterator {
+  public:
+    using iterator_category = input_iterator_tag;
+    using value_type        = directory_entry;
+    using difference_type   = ptrdiff_t;
+    using pointer           = const directory_entry*;
+    using reference         = const directory_entry&;
+
+    // [fs.dir.itr.members], member functions
+    directory_iterator() noexcept;
+    explicit directory_iterator(const path& p);
+    directory_iterator(const path& p, directory_options options);
+    directory_iterator(const path& p, error_code& ec);
+    directory_iterator(const path& p, directory_options options,
+                       error_code& ec);
+    directory_iterator(const directory_iterator& rhs);
+    directory_iterator(directory_iterator&& rhs) noexcept;
+    ~directory_iterator();
+
+    directory_iterator& operator=(const directory_iterator& rhs);
+    directory_iterator& operator=(directory_iterator&& rhs) noexcept;
+
+    const directory_entry& operator*() const;
+    const directory_entry* operator->() const;
+    directory_iterator&    operator++();
+    directory_iterator&    increment(error_code& ec);
+
+    bool operator==(default_sentinel_t) const noexcept {          // since C++20
+      return *this == directory_iterator();
+    }
+
+    // other members as required by [input.iterators], input iterators
+  };
 
     // enable directory_iterator range-based for statements
     directory_iterator begin(directory_iterator iter) noexcept;
     directory_iterator end(directory_iterator) noexcept;
 
-    class recursive_directory_iterator;
+class recursive_directory_iterator {
+  public:
+    using iterator_category = input_iterator_tag;
+    using value_type        = directory_entry;
+    using difference_type   = ptrdiff_t;
+    using pointer           = const directory_entry*;
+    using reference         = const directory_entry&;
+
+    // [fs.rec.dir.itr.members], constructors and destructor
+    recursive_directory_iterator() noexcept;
+    explicit recursive_directory_iterator(const path& p);
+    recursive_directory_iterator(const path& p, directory_options options);
+    recursive_directory_iterator(const path& p, directory_options options,
+                                 error_code& ec);
+    recursive_directory_iterator(const path& p, error_code& ec);
+    recursive_directory_iterator(const recursive_directory_iterator& rhs);
+    recursive_directory_iterator(recursive_directory_iterator&& rhs) noexcept;
+    ~recursive_directory_iterator();
+
+    // [fs.rec.dir.itr.members], observers
+    directory_options  options() const;
+    int                depth() const;
+    bool               recursion_pending() const;
+
+    const directory_entry& operator*() const;
+    const directory_entry* operator->() const;
+
+    // [fs.rec.dir.itr.members], modifiers
+    recursive_directory_iterator&
+      operator=(const recursive_directory_iterator& rhs);
+    recursive_directory_iterator&
+      operator=(recursive_directory_iterator&& rhs) noexcept;
+
+    recursive_directory_iterator& operator++();
+    recursive_directory_iterator& increment(error_code& ec);
+
+    void pop();
+    void pop(error_code& ec);
+    void disable_recursion_pending();
+
+    bool operator==(default_sentinel_t) const noexcept {          // since C++20
+      return *this == recursive_directory_iterator();
+    }
+
+    // other members as required by [input.iterators], input iterators
+  };
 
     // enable recursive_directory_iterator range-based for statements
     recursive_directory_iterator begin(recursive_directory_iterator iter) noexcept;
     recursive_directory_iterator end(recursive_directory_iterator) noexcept;
 
   class file_status {
   public:
     // [fs.file.status.cons], constructors and destructor

libcxx/include/regex

public:
     regex_iterator(BidirectionalIterator a, BidirectionalIterator b,
                    const regex_type&& re,
                    regex_constants::match_flag_type m
                                      = regex_constants::match_default) = delete; // C++14
     regex_iterator(const regex_iterator&);
     regex_iterator& operator=(const regex_iterator&);
 
     bool operator==(const regex_iterator&) const;
-    bool operator!=(const regex_iterator&) const;                                // Removed in C++20
+    bool operator==(default_sentinel_t) const { return *this == regex_iterator(); } // since C++20
+    bool operator!=(const regex_iterator&) const;                                   // Removed in C++20
 
     const value_type& operator*() const;
     const value_type* operator->() const;
 
     regex_iterator& operator++();
     regex_iterator operator++(int);
 };
 
public:
     template <size_t N>
         regex_token_iterator(BidirectionalIterator a, BidirectionalIterator b,
                              const regex_type&& re, const int (&submatches)[N],
                              regex_constants::match_flag_type m = regex_constants::match_default) = delete; // C++14
     regex_token_iterator(const regex_token_iterator&);
     regex_token_iterator& operator=(const regex_token_iterator&);
 
     bool operator==(const regex_token_iterator&) const;
-    bool operator!=(const regex_token_iterator&) const;                  // Removed in C++20
+    bool operator==(default_sentinel_t) const { return *this == regex_token_iterator(); } // since C++20
+    bool operator!=(const regex_token_iterator&) const;                                   // Removed in C++20
 
     const value_type& operator*() const;
     const value_type* operator->() const;
 
     regex_token_iterator& operator++();
     regex_token_iterator operator++(int);
 };
 
 typedef regex_token_iterator<const char*>             cregex_token_iterator;
 typedef regex_token_iterator<const wchar_t*>          wcregex_token_iterator;
 typedef regex_token_iterator<string::const_iterator>  sregex_token_iterator;
 typedef regex_token_iterator<wstring::const_iterator> wsregex_token_iterator;
 
 } // std
 */
 
 #include <__algorithm/find.h>
 #include <__algorithm/search.h>
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__availability>
 #include <__config>
 #include <__iterator/back_insert_iterator.h>
+#include <__iterator/default_sentinel.h>
 #include <__iterator/wrap_iter.h>
 #include <__locale>
 #include <__memory_resource/polymorphic_allocator.h>
 #include <__type_traits/is_swappable.h>
 #include <__utility/move.h>
 #include <__utility/pair.h>
 #include <__utility/swap.h>
 #include <__verbose_abort>
regex_iterator(_BidirectionalIterator __a, _BidirectionalIterator __b,
                                               = regex_constants::match_default);
 #if _LIBCPP_STD_VER >= 14
     regex_iterator(_BidirectionalIterator __a, _BidirectionalIterator __b,
                    const regex_type&& __re,
                    regex_constants::match_flag_type __m
                                      = regex_constants::match_default) = delete;
 #endif
 
-    bool operator==(const regex_iterator& __x) const;
+    _LIBCPP_HIDE_FROM_ABI bool operator==(const regex_iterator& __x) const;
+#if _LIBCPP_STD_VER >= 20
+    _LIBCPP_HIDE_FROM_ABI bool operator==(default_sentinel_t) const { return *this == regex_iterator(); }
+#endif
 #if _LIBCPP_STD_VER < 20
     _LIBCPP_INLINE_VISIBILITY
     bool operator!=(const regex_iterator& __x) const {return !(*this == __x);}
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     reference operator*() const {return  __match_;}
     _LIBCPP_INLINE_VISIBILITY
template <size_t _Np>
                              const int (&__submatches)[_Np],
                              regex_constants::match_flag_type __m =
                                       regex_constants::match_default) = delete;
 #endif
 
     regex_token_iterator(const regex_token_iterator&);
     regex_token_iterator& operator=(const regex_token_iterator&);
 
-    bool operator==(const regex_token_iterator& __x) const;
-    _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_HIDE_FROM_ABI bool operator==(const regex_token_iterator& __x) const;
+#if _LIBCPP_STD_VER >= 20
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDE_FROM_ABI bool operator==(default_sentinel_t) const {
+        return *this == regex_token_iterator();
+    }
+#endif
 #if _LIBCPP_STD_VER < 20
+    _LIBCPP_INLINE_VISIBILITY
     bool operator!=(const regex_token_iterator& __x) const {return !(*this == __x);}
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     const value_type& operator*() const {return *__result_;}
     _LIBCPP_INLINE_VISIBILITY
     const value_type* operator->() const {return __result_;}
 

libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <filesystem>
+
+// class directory_iterator
+
+// bool operator==(default_sentinel_t) const noexcept {           // since C++20
+//   return *this == directory_iterator();
+// }
+
+#include <cassert>
+#include <iterator>
+
+#include "filesystem_include.h"
+#include "test_comparisons.h"
+
+int main(int, char**) {
+  AssertEqualityAreNoexcept<fs::directory_iterator>();
+  AssertEqualityReturnBool<fs::directory_iterator>();
+
+  fs::directory_iterator i;
+  assert(testEquality(i, std::default_sentinel, true));
+
+  return 0;
+}

libcxx/test/std/input.output/filesystems/class.rec.dir.itr/rec.dir.itr.members/equal.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <filesystem>
+
+// class recursive_directory_iterator
+
+// bool operator==(default_sentinel_t) const noexcept {           // since C++20
+//   return *this == recursive_directory_iterator();
+// }
+
+#include <cassert>
+#include <iterator>
+
+#include "filesystem_include.h"
+#include "test_comparisons.h"
+
+int main(int, char**) {
+  AssertEqualityAreNoexcept<fs::recursive_directory_iterator>();
+  AssertEqualityReturnBool<fs::recursive_directory_iterator>();
+
+  fs::recursive_directory_iterator i;
+  assert(testEquality(i, std::default_sentinel, true));
+
+  return 0;
+}

libcxx/test/std/re/re.iter/re.regiter/re.regiter.comp/equal.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <regex>
+
+// class regex_token_iterator<BidirectionalIterator, charT, traits>
+
+// bool operator==(default_sentinel_t) const { return *this == regex_iterator(); } // since C++20
+
+#include <cassert>
+#include <iterator>
+#include <regex>
+
+#include "test_comparisons.h"
+
+int main(int, char**) {
+  AssertEqualityReturnBool<std::cregex_iterator>();
+
+  {
+    std::cregex_iterator i;
+    assert(testEquality(i, std::default_sentinel, true));
+  }
+
+  AssertEqualityReturnBool<std::sregex_token_iterator>();
+
+  {
+    std::sregex_token_iterator i;
+    assert(testEquality(i, std::default_sentinel, true));
+  }
+
+  return 0;
+}

libcxx/test/std/re/re.iter/re.tokiter/re.tokiter.comp/equal.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <regex>
 
 // class regex_token_iterator<BidirectionalIterator, charT, traits>
 
 // bool operator==(const regex_token_iterator& right) const;
+// bool operator==(default_sentinel_t) const { return *this == regex_token_iterator(); } // since C++20
 // bool operator!=(const regex_token_iterator& right) const; // generated by the compiler in C++20
 
-#include <regex>
 #include <cassert>
-#include "test_macros.h"
+#include <iterator>
+#include <regex>
+
+#include "test_comparisons.h"
+
+int main(int, char**) {
+#if _LIBCPP_STD_VER >= 20
+  AssertEqualityReturnBool<std::cregex_token_iterator>();
+
+  {
+    std::cregex_token_iterator i;
+    assert(testEquality(i, std::default_sentinel, true));
+  }
+
+  AssertEqualityReturnBool<std::sregex_token_iterator>();
 
-int main(int, char**)
   {
-    {
-        std::regex phone_numbers("\\d{3}-\\d{4}");
-        const char phone_book[] = "start 555-1234, 555-2345, 555-3456 end";
-        std::cregex_token_iterator i(std::begin(phone_book), std::end(phone_book)-1,
-                                     phone_numbers, -1);
-        assert(i != std::cregex_token_iterator());
-        assert(!(i == std::cregex_token_iterator()));
-        std::cregex_token_iterator i2 = i;
-        assert(i2 == i);
-        assert(!(i2 != i));
-        ++i;
-        assert(!(i2 == i));
-        assert(i2 != i);
-    }
+    std::sregex_token_iterator i;
+    assert(testEquality(i, std::default_sentinel, true));
+  }
+#endif
+
+    std::cregex_token_iterator i(std::begin(phone_book), std::end(phone_book) - 1, phone_numbers, -1);
 
   return 0;
 }