[libcxx-commits] [PATCH] D149829: [libc++][ranges] Implement the changes to container adaptors from P1206 (`ranges::to`):

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

This LGTM w/ a few comments! Thanks!



================
Comment at: libcxx/test/std/containers/container.adaptors/from_range_container_adaptors.h:9
+
+#ifndef SUPPORT_FROM_RANGE_CONTAINER_ADAPTORS_H
+#define SUPPORT_FROM_RANGE_CONTAINER_ADAPTORS_H
----------------
This include guard is wrong.


================
Comment at: libcxx/test/std/containers/container.adaptors/from_range_container_adaptors.h:90
+          class Alloc>
+constexpr void test_priority_queue_with_input(std::vector<T>&& input) {
+  auto b = Iter(input.data());
----------------
IMO these should move out of this file and be local to `priority.queue/from_range.pass.cpp`.


================
Comment at: libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/from_range.pass.cpp:17-22
+// template <container-compatible-range<T> R>
+//   priority_queue(from_range_t, R&& rg, const Compare& x = Compare()); // since C++23
+// template <container-compatible-range<T> R, class Alloc>
+//   priority_queue(from_range_t, R&& rg, const Compare&, const Alloc&); // since C++23
+// template <container-compatible-range<T> R, class Alloc>
+//   priority_queue(from_range_t, R&& rg, const Alloc&); // since C++23
----------------
Nitpick, but let's move those above includes. Please check in other test files too.


================
Comment at: libcxx/test/support/unwrap_container_adaptor.h:23-29
+  // TODO: make this work pre-C++20.
+  decltype(auto) get_comparator()
+  requires requires {
+    UnwrapAdaptor::c;
+  } {
+    return (UnwrapAdaptor::comp); // Put into parentheses to make sure the function returns a reference.
+  }
----------------
This doesn't require C++20.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149829