[libcxx-commits] [PATCH] D111644: [libc++] LWG3480: make (recursive)_directory_iterator C++20 ranges

[Joe Loser via Phabricator via libcxx-commits]

jloser marked 3 inline comments as done.
jloser added inline comments.


================
Comment at: libcxx/include/filesystem:3024
+#if !defined(_LIBCPP_HAS_NO_RANGES)
+template<>
+inline constexpr bool _VSTD::ranges::enable_borrowed_range<_VSTD_FS::directory_iterator> = true;
----------------
Quuxplusone wrote:
> Nitpick: `template <>` for consistency. (FWIW, I personally write `template<>` consistently in my own code. But libc++ more often uses `template <>`; and more importantly, lines 3026, 3029, 3031 already use `template <>`.)
> 
> Also, do you need `_LIBCPP_TEMPLATE_VIS` throughout? I think so, but I'm not sure.
Fixed the spacing in `template <>` here, in the synopsis, and the tests.

As for the `_LIBCPP_TEMPLATE_VIS`, I don't know. I added it to the class, but can we get away with adding it just to these specializations? @ldionne 


================
Comment at: libcxx/test/std/input.output/filesystems/class.directory_iterator/range_concept_conformance.compile.pass.cpp:12-13
 
-// directory_iterator, recursive_directory_iterator
+// AppleClang 12.0.0 doesn't fully support ranges/concepts
+// XFAIL: apple-clang-12.0.0
+
----------------
Quuxplusone wrote:
> Can we do `UNSUPPORTED: libcpp-no-concepts` instead? Ditto throughout.
Let's see what BuildKite has to say -- just tried.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111644