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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this! Several minor comments.



================
Comment at: libcxx/include/__filesystem/directory_iterator.h:106
 
+#  if _LIBCPP_STD_VER >= 23
+
----------------
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.


================
Comment at: libcxx/include/filesystem:322
+    // other members as required by [input.iterators], input iterators
+  };
 
----------------
Thanks for updating the synopsis!


================
Comment at: libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp:25
+
+void test() {
+  {
----------------
Note for these small tests I don't mind having all code in main.


================
Comment at: libcxx/test/std/input.output/filesystems/class.directory_iterator/directory_iterator.members/equal.pass.cpp:28
+    fs::directory_iterator i;
+    assert(testEquality(i, std::default_sentinel, true));
+  }
----------------
Please test the following properties too
- returns a `bool` using `AssertEqualityReturnBool`
- `noexcept` using `AssertEqualityAreNoexcept`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153299/new/

https://reviews.llvm.org/D153299