[libcxx-commits] [PATCH] D90999: [libc++] Implements ranges::enable_borrowed_range

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco added a comment.

I am wondering whether we should split the ranges header up into a smaller part that we use to include in other headers adn the rest.

Including all of ranges just for one trait seems a bit of an overkill.

We could always add it to concepts as that should be a transitive include for everything that includes range. Thoughts @ldionne?



================
Comment at: libcxx/include/ranges:22
+    // [range.access], range access
+    inline constexpr unspecified begin = unspecified;
+    inline constexpr unspecified end = unspecified;
----------------
I am wondering whether it is wise to copy the synopsis as it should be rather than how it is. This might trick the user to believe all of it is supported. What are the conventions here?


================
Comment at: libcxx/include/ranges:15
+
+#include <compare>              // see [compare.syn]
+#include <initializer_list>     // see [initializer.list.syn]
----------------
I am slightly surprised that concepts is not required


================
Comment at: libcxx/test/std/ranges/range.range/enable_borrowed_range.pass.cpp:24
+{
+	static_assert(!std::ranges::enable_borrowed_range<char>);
+	static_assert(!std::ranges::enable_borrowed_range<int>);
----------------
I believe you are missing coverage for all the ranges that are not borrowed, e.g. vector, list, ...


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

https://reviews.llvm.org/D90999