[libcxx-commits] [PATCH] D100255: [libcxx] adds `range` access CPOs

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I'll have a close look once it passes the build server. I expect pre-merge errors due to conflicts with D90999 <https://reviews.llvm.org/D90999>. Both modify add the `<ranges>` header.



================
Comment at: libcxx/include/CMakeLists.txt:146
+  ranges
+  __ranges/access.h
   regex
----------------
Please keep the list sorted. The recently added `__memory/*` are also sorted.


================
Comment at: libcxx/include/__ranges/access.h:9
+//===----------------------------------------------------------------------===//
+#ifndef _LIBCPP_RANGES_ACCESS_H
+#define _LIBCPP_RANGES_ACCESS_H
----------------
For consistency `_LIBCPP_RANGES_ACCESS_H` -> `_LIBCPP___RANGES_ACCESS_H`.


================
Comment at: libcxx/include/__ranges/access.h:25
+
+#if !defined(_LIBCPP_HAS_NO_RANGES)
+
----------------
I would like to keep `_LIBCPP_STD_VER > 17` to be consistent with `<concepts>`.


================
Comment at: libcxx/include/__ranges/access.h:38
+concept __can_borrow =
+    is_lvalue_reference_v<_Tp> || enable_borrowed_range<remove_cvref_t<_Tp> >;
+
----------------
Shouldn't `remove_cvref_t` be `remove_cv_t`?


================
Comment at: libcxx/include/ranges:2
+// -*- C++ -*-
+//===-------------------------- concepts ----------------------------------===//
+//
----------------
Copy-paste "concepts".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100255