[libcxx-commits] [PATCH] D144822: [libc++][ranges] P2711R1 Making Multi-Param Constructors Of views explicit

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added a comment.
This revision now requires changes to proceed.

With added tests this should be good to go.



================
Comment at: libcxx/docs/Status/Cxx2bPapers.csv:111
 "`P2164R9 <https://wg21.link/P2164R9>`__","LWG", "``views::enumerate``","February 2023","","","|ranges|"
-"`P2711R1 <https://wg21.link/P2711R1>`__","LWG", "Making multi-param constructors of ``views`` ``explicit``","February 2023","","","|ranges|"
+"`P2711R1 <https://wg21.link/P2711R1>`__","LWG", "Making multi-param constructors of ``views`` ``explicit``","February 2023","Partial","","|ranges|"
 "`P2609R3 <https://wg21.link/P2609R3>`__","LWG", "Relaxing Ranges Just A Smidge","February 2023","","","|ranges|"
----------------
Please add a note that `join_with_view` isn't yet implemented, so we know what's missing to complete it.


================
Comment at: libcxx/include/__ranges/drop_view.h:72
     _LIBCPP_HIDE_FROM_ABI
-    constexpr drop_view(_View __base, range_difference_t<_View> __count)
+    constexpr explicit drop_view(_View __base, range_difference_t<_View> __count)
       : __count_(__count)
----------------
You are missing a test for this constructor.


================
Comment at: libcxx/include/__ranges/iota_view.h:314
     _LIBCPP_HIDE_FROM_ABI
-    constexpr iota_view(type_identity_t<_Start> __value, type_identity_t<_BoundSentinel> __bound_sentinel)
+    constexpr explicit iota_view(type_identity_t<_Start> __value, type_identity_t<_BoundSentinel> __bound_sentinel)
         : __value_(std::move(__value)), __bound_sentinel_(std::move(__bound_sentinel)) {
----------------
You are missing tests for these constructors.


================
Comment at: libcxx/include/__ranges/lazy_split_view.h:81
   _LIBCPP_HIDE_FROM_ABI
-  constexpr lazy_split_view(_View __base, _Pattern __pattern)
+  constexpr explicit lazy_split_view(_View __base, _Pattern __pattern)
     : __base_(std::move(__base)), __pattern_(std::move(__pattern)) {}
----------------
You are missing tests for these constructors.


================
Comment at: libcxx/include/__ranges/split_view.h:78
 
-  _LIBCPP_HIDE_FROM_ABI constexpr split_view(_View __base, _Pattern __pattern)
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit split_view(_View __base, _Pattern __pattern)
       : __base_(std::move(__base)), __pattern_(std::move((__pattern))) {}
----------------
You are missing tests for these constructors.


================
Comment at: libcxx/include/__ranges/take_view.h:67
 
-  _LIBCPP_HIDE_FROM_ABI constexpr take_view(_View __base, range_difference_t<_View> __count)
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit take_view(_View __base, range_difference_t<_View> __count)
       : __base_(std::move(__base)), __count_(__count) {
----------------
You are missing a test for this constructor.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.filter/ctor.view_pred.pass.cpp:46
 
+using RFV = std::ranges::filter_view<Range, Pred>;
+
----------------
We normally don't use aliases to make it easier to see what is tested. For a single instance it's also not really worth it.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.filter/ctor.view_pred.pass.cpp:90
   static_assert(test());
-
+  static_assert(!test_convertible<RFV>(), "This constructor must be explicit");
   return 0;
----------------
We generally have SFINAE tests at the top of the file, so let's move them there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144822