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

[Hui via Phabricator via libcxx-commits]

huixie90 added a comment.

@Quuxplusone

> 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.

I could do but I don't think it is necessary.
First, the test is checking the view instance stored inside the `drop_view` via `base` function, which returns by reference in the specification . It is not checking any other instances.
Second, unlike iterators, view object is not going to be copied around because 1. `view` concept only requires `movable` 2. algorithms cannot assume `view`s are cheap to copy (constant time doesn't mean cheap).  Iterators are cheap to copy.

> please move the comments into static_asserts circa lines 37 and 45.

Sure I can do. I put comments before the test points just because all other test points do the same thing. but happy to move

> (filling in the actual expected types)

Iterators are private defined inside the `view`. How can you spell it out without using `decltype`?

> `ASSERT_SAME_TYPE(decltype(std::as_const(tv).begin()), std::counted_iterator<int*>); `

The reason I don't create an instance is that I think I did create an instance at beginning and some configuration in the CI failed and complained that some of my member function is not defined. what about

  {
      using TakeNonSimpleView = std::ranges::take_view<NonCommonSimpleView>;
      ASSERT_SAME_TYPE(decltype(std::declval<TakeNonSimpleView&>().begin()), std::counted_iterator<int*>);
      ASSERT_SAME_TYPE(decltype(std::declval<TakeNonSimpleView const&>().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