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

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Mar 23 12:46:58 PDT 2021


cjdb added a comment.

Overall, LGTM, but I'm wondering if we should hold off on merging till we've got consensus on how detail headers are going to be handled.
This is only because I'm seeing `__ranges_enable_helpers`.



================
Comment at: libcxx/include/ranges:15
+
+#include <compare>              // see [compare.syn]
+#include <initializer_list>     // see [initializer.list.syn]
----------------
Mordante wrote:
> miscco wrote:
> > I am slightly surprised that concepts is not required
> Agreed. For this patch I don't need concepts, but `std::ranges::begin()` will require them.
It's transitively included by `<iterator>`.


================
Comment at: libcxx/test/std/ranges/range.range/enable_borrowed_range.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Please change to `enable_borrowed_range.pass.cpp` (we've agreed in other patches to move away from `.compile`). Same with other files (this is just the first I noticed).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90999



More information about the libcxx-commits mailing list