[libcxx-commits] [PATCH] D149826: [libc++][ranges] Implement the changes to vector 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.

LGTM with passing CI! Thanks!



================
Comment at: libcxx/include/__ranges/from_range.h:23
+
+struct from_range_t {
+  _LIBCPP_HIDE_FROM_ABI explicit from_range_t() = default;
----------------
You should also update the synopsis in `<ranges>` with this.


================
Comment at: libcxx/include/__ranges/from_range.h:27
+
+inline constexpr from_range_t from_range{};
+
----------------
I think you forgot to copy over the tests for `libcxx/test/std/ranges/range.utility/range.utility.conv/from_range_t.compile.pass.cpp`.


================
Comment at: libcxx/include/vector:1481
             __construct_at_end(__mid, __last, __new_size - size());
+        }
         else
----------------
We still need a test to catch the issue that went uncaught in D142335.


================
Comment at: libcxx/include/vector:2871
         __vallocate(__n);
-        __construct_at_end(__il.begin(), __il.end());
+        __construct_at_end(__il.begin(), __il.end(), __il.size());
     }
----------------
It feels like passing `__n` might be more appropriate, given that `__construct_at_end` takes `size_type`.


================
Comment at: libcxx/include/vector:2886
         __vallocate(__n);
-        __construct_at_end(__il.begin(), __il.end());
+        __construct_at_end(__il.begin(), __il.end(), __il.size());
     }
----------------
Same.


================
Comment at: libcxx/include/vector:2902
         __vallocate(__v.size());
-        __construct_at_end(__v.begin(), __v.end());
+        __construct_at_end(__v.begin(), __v.end(), __v.size());
     }
----------------
Here and below, I think it is correct as-is since you're using `__v.size()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149826