- add the from_range_t constructors and the related deduction guides;
- add the insert_range/assign_range/etc. member functions.
(Note: this patch is split from https://reviews.llvm.org/D142335)
Paths
| Differential D149826
[libc++][ranges] Implement the changes to vector from P1206 (`ranges::to`): ClosedPublic Authored by var-const on May 4 2023, 2:13 AM.
Details
Summary
(Note: this patch is split from https://reviews.llvm.org/D142335)
Diff Detail
Event TimelineComment Actions LGTM with passing CI! Thanks!
This revision is now accepted and ready to land.May 4 2023, 9:42 AM var-const marked 5 inline comments as done. Comment ActionsAddress feedback
Comment Actions
Closed by commit rG17bbb224f99c: [libc++][ranges] Implement the changes to vector from P1206 (`ranges::to`): (authored by varconst <varconsteq@gmail.com>, committed by var-const). · Explain WhyMay 8 2023, 11:41 PM This revision was automatically updated to reflect the committed changes. Comment Actions @rupprecht Just a heads-up: this patch refactors the internals of vector and thus carries some amount of risk of a possible breakage. The tests in this patch are quite extensive, but I'd still like to give you a warning just in case you notice any new failures that this patch could be the culprit. Adding libc++-vendors to subscribers as well. Comment Actions Before this patch, if a non-copy-constructible Iterator was passed to the vector constructor vector<_Tp, _Allocator>::vector(_InputIterator __first, _InputIterator __last) it compiled, but now it gives the error: call to deleted constructor of 'Iterator'. Is that expected ? smth like #include <iterator> #include <vector> const char* data = "abc"; class Iterator : public std::iterator<std::input_iterator_tag, const char> { public: explicit Iterator(const char c) {} Iterator(const Iterator& other) = delete; Iterator& operator=(const Iterator& other) = delete; reference operator*() const { return data[0]; } void operator++() {} bool operator==(const Iterator& other) const { return true; } bool operator!=(const Iterator& other) const { return false; } }; class Elem { public: Elem() = default; explicit Elem(const char ref) {} }; int main(int argc, char** argv) { std::vector<Elem>(Iterator(data[0]), Iterator(data[1])); } Comment Actions @rupprecht Just a heads-up: this patch refactors the internals of vector and thus carries some amount of risk of a possible breakage. The tests in this patch are quite extensive, but I'd still like to give you a warning just in case you notice any new failures that this patch could be the culprit. Adding libc++-vendors to subscribers as well.
Thank you for the report! Is this blocking you? I will look into this today. Hopefully it's an easy fix. Comment Actions
An InputIterator has to be copy-constructible: http://eel.is/c++draft/iterator.iterators#2.1, and IMO we should enforce that even if we don't make use of the functionality (e.g. static_assert(is_copy_constructible<_InputIterator>::value, "InputIterator has to be copy constructible")). Comment Actions Thanks for quick response!
yes. it's causing breakages in google code.
Great! Comment Actions I must say I agree with @philnik here. Iterators are supposed to be copyable, and implementations are free to use that in their implementation as a detail. For example, here we're just passing the iterators down to another function. We can see that GCC also does not accept this code: https://godbolt.org/z/7GhGf8Exo I don't think it would be reasonable to provide a ad-hoc guarantee like that in std::vector's constructor. In fact, I think our original sin was probably not to add a static_assert in some of these methods so we started accepting types that we shouldn't have accepted, and then Hyrum's law materialized. @asmok-g So in the end, the code will have to be fixed (and the fix is pretty trivial). However, our goal is not to make your life hard. If this breakage is very wide and challenging for you folks to fix, we can try to add a workaround on our side and give you a grace period. We're willing to work with you to make this as easy as possible, but the right thing to do in the end is to fix the code, not silently accept it. Please let us know what the scope of breakage is and whether you want us to work on providing you a grace period.
Revision Contents
Diff 520598 libcxx/docs/Status/Cxx2bPapers.csv
libcxx/include/CMakeLists.txt
libcxx/include/__ranges/container_compatible_range.h
libcxx/include/__ranges/from_range.h
libcxx/include/__split_buffer
libcxx/include/module.modulemap.in
libcxx/include/ranges
libcxx/include/vector
libcxx/test/libcxx/private_headers.verify.cpp
libcxx/test/std/containers/from_range_helpers.h
libcxx/test/std/containers/insert_range_helpers.h
libcxx/test/std/containers/sequences/from_range_sequence_containers.h
libcxx/test/std/containers/sequences/insert_range_sequence_containers.h
libcxx/test/std/containers/sequences/vector.bool/append_range.pass.cpp
libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.cons/deduct.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
libcxx/test/std/ranges/range.utility/range.utility.conv/from_range_t.compile.pass.cpp
libcxx/test/support/deduction_guides_sfinae_checks.h
|
You should also update the synopsis in <ranges> with this.