[libcxx-commits] [PATCH] D142335: [libc++][ranges] Partially implement `ranges::to`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/deque:609
+  template <_ContainerCompatibleRange<_Tp> _Range>
+  _LIBCPP_HIDE_FROM_ABI deque(from_range_t, _Range&& __range,
+      const allocator_type& __a = allocator_type())
----------------
ldionne wrote:
> Testing: We should make sure that we do the right thing if we throw halfway through the construction. Do we destroy the elements constructed so far? Do we do that in reverse construction order?
> 
> I think the answer right now is "no" to both of these questions. If so, and if that's a pre-existing issue for other constructors of `deque`, and if the standard requires us to do that, let's at least file a bug report so we can remember to fix that.
Created https://github.com/llvm/llvm-project/issues/62056. Looks like it's a regression in LLVM 16, perhaps due to removing the base class from `deque`?


================
Comment at: libcxx/include/ranges:353
+
+  struct from_range_t { explicit from_range_t() = default; };  // Since C++23
+  inline constexpr from_range_t from_range{};                  // Since C++23
----------------
ldionne wrote:
> This is nitpicky but let's make sure we have a test that the constructor is explicit.
Done. Perhaps there's an easier way to check this than using a concept?


================
Comment at: libcxx/test/std/ranges/range.utility/range.utility.conv/to_std_containers.pass.cpp:69-89
+
+template <class T>
+using all_containers = types::type_list<
+    std::vector<T>,
+    std::deque<T>,
+    std::list<T>,
+    std::forward_list<T>,
----------------
ldionne wrote:
> Here's a suggestion that would avoid having to define `is_equal` with a bit of magic:
> 
> ```
> template <class T>
> using sequence_containers = types::type_list<std::vector<T>, std::deque<T>, std::list<T>, std::forward_list<T>>;
> 
> template <class T>
> using set_like_associative_containers = types::type_list<std::set<T>, std::multiset<T>, std::unordered_set<T>, std::unordered_multiset<T>>;
> 
> template <class Key, class Value>
> using map_like_associative_containers = types::type_list<std::map<Key, Value>, std::multimap<Key, Value>, std::unordered_map<Key, Value>, std::unordered_multimap<Key, Value>>;
> 
> void test() {
>   // Sequence = Sequence
>   types::for_each(sequence_containers<int>{}, []<class To>() {
>     types::for_each(sequence_containers<int>{}, []<class From>() {
>       test_conversion<From, To>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::equal(a, b); });
>     });
>   });
>   // Set-like = Set-like
>   types::for_each(set_like_containers<int>{}, []<class To>() {
>     types::for_each(set_like_containers<int>{}, []<class From>() {
>       test_conversion<From, To>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>     });
>   });
>   // Map-like = Map-like
>   types::for_each(map_like_containers <int, int>{}, []<class To>() {
>     types::for_each(map_like_containers<int, int>{}, []<class From>() {
>       test_conversion<From, To>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>     });
>   });
> 
>   // Sequence = Set-like and Set-like = Sequence
>   types::for_each(sequence_containers<int>{}, []<class To>() {
>     types::for_each(set_like_associative_containers<int>{}, []<class From>() {
>       test_conversion<From, To>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>       test_conversion<To, From>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>     });
>   });
>   // Sequence = Map-like and Map-like = Sequence
>   types::for_each(sequence_containers<std::pair<int, int>>{}, []<class To>() {
>     types::for_each(map_like_associative_containers<int, int>{}, []<class From>() {
>       test_conversion<From, To>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>       test_conversion<To, From>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>     });
>   });
>   // Set-like = Map-like and Map-like = Set-like
>   types::for_each(set_like_containers<std::pair<int, int>>{}, []<class To>() {
>     types::for_each(map_like_containers<int, int>{}, []<class From>() {
>       test_conversion<From, To>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>       test_conversion<To, From>(/* equal = */[](auto const& a, auto const& b) { return std::ranges::is_permutation(a, b); });
>     });
>   });
> }
> ```
Thanks! It changed a bit, but the overall idea is the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142335