[libcxx-commits] [PATCH] D102468: [libcxx][ranges] removes default_initializable from weakly_incrementable and view

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added a comment.

LGTM modulo comments.
(My previous round of comments went in just before the update to include `ostream_iterator` et al.)



================
Comment at: libcxx/docs/Cxx2aStatusPaperStatus.csv:196
 "`P2116 <https://wg21.link/P2116>`__","LWG","Remove tuple-like protocol support from fixed-extent span","Prague","|Complete|","11.0"
+"`P2325 <https://wg21.link/P2325>`__","LWG","Views should not be required to be default constructible","June Telecon","|In progress|",""
----------------
@ldionne: What is the process by which papers get added here? Can we get someone to add //all// the papers that were adopted in this week's plenary? (Then I could check whether D97742 is finally unblocked! ;))


================
Comment at: libcxx/include/iterator:919
     _Container* container;
-    typename _Container::iterator iter;
+    typename _Container::iterator iter; // FIXME: `ranges::iterator_t<Container>` in C++20 mode
 public:
----------------
ldionne wrote:
> Why a FIXME instead of `#if _LIBCPP_STD_VER > XXXX`?
> 
> If both types exist, then they should be the same, so I don't see any sort of ABI issue in case that was the concern.
I agree with Louis's assessment here.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.winc/subsumption.verify.cpp:33
 
-static_assert(check_subsumption<int*>());
+static_assert(check_subsumption<int*>()); // expected-error {{call to 'check_subsumption' is ambiguous}}
----------------
What is this test supposed to be testing?


================
Comment at: libcxx/test/std/ranges/range.req/range.view/view.subsumption.verify.cpp:55-65
 namespace subsume_default_initializable {
-  template <std::ranges::view>
-  constexpr bool test() { return true; }
-  template <std::default_initializable>
-  constexpr bool test() { return false; }
-  static_assert(test<View>());
+template <std::ranges::view>
+constexpr bool test() {
+  return true;
+}
+template <std::default_initializable>
+constexpr bool test() {
----------------
This test is now wrong. Please remove lines 55-65, and undo the file move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102468