[libcxx-commits] [PATCH] D116808: fix __simple_view concept in std::ranges

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

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp:85-97
+  // simple_view<V> && random_access_range<const V> && sized_range<const V>
+  {
+    std::ranges::drop_view dropView(SimpleView{}, 4);
+    dropView.begin();
+    assert(!dropView.base().non_const_begin_called);
+  }
+
----------------
```
{
  bool called = false;
  auto dv = std::ranges::drop_view<SimpleView>(SimpleView{&called}, 4);
  assert(std::as_const(dv).begin() == nullptr);
  assert(!called);
  assert(dv.begin() == nullptr);
  assert(called);
}
```
You don't want to store `bool called` inside the view object itself, because the view object itself is going to get copied around. You want to store the bool //outside// the view object, and let the view object copy around a //pointer// to the bool.

Also, as below, please move the comments into static_asserts circa lines 37 and 45.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.join.view/begin.pass.cpp:102-106
+  {
+    using WithSimpleBase = std::ranges::join_view<SimpleParentView>;
+    static_assert(std::same_as<decltype(std::declval<WithSimpleBase&>().begin()),
+                               decltype(std::declval<WithSimpleBase const&>().begin())>);
+  }
----------------
```
{
  std::ranges::join_view<ParentView<ChildView>> jv;
  ASSERT_SAME_TYPE(decltype(jv.begin()), ???);
  ASSERT_SAME_TYPE(decltype(std::as_const(jv).begin()), ???);
}
{
  std::ranges::join_view<SimpleParentView> jv;
  ASSERT_SAME_TYPE(decltype(jv.begin()), ???);
  ASSERT_SAME_TYPE(decltype(std::as_const(jv).begin()), ???);
}
```
(filling in the actual expected types)


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take/begin.pass.cpp:76-77
+  {
+    using TakeNonSimpleView = std::ranges::take_view<NonCommonSimpleView>;
+    ASSERT_SAME_TYPE(decltype(std::declval<TakeNonSimpleView&>().begin()), std::counted_iterator<int*>);
+    // the optimal result should be int* but according to the c++20 standard,
----------------
```
{
  std::ranges::take_view<NonCommonSimpleView> tv;
  ASSERT_SAME_TYPE(decltype(tv.begin()), std::counted_iterator<int*>);
  ASSERT_SAME_TYPE(decltype(std::as_const(tv).begin()), std::counted_iterator<int*>);
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116808