range-v3 icon indicating copy to clipboard operation
range-v3 copied to clipboard

relax ranges::view_ concept

Open marehr opened this issue 2 years ago • 7 comments

See discussion at https://github.com/ericniebler/range-v3/issues/1662#issuecomment-936423357

@ericniebler I think this is a trade-off between the current concept definition in the standard and the non-ability of the range-v3 views to handle move-only views. Most range-v3 views should just be fine to copy existing views.

Do you know out of your head, what views might need default initializable?

marehr avatar Oct 06 '21 14:10 marehr

Do you know out of your head, what views might need default initializable?

https://brevzin.github.io/cpp_proposals/2325_views_default/p2325r3.html#uses-of-default-construction

And the join usage is resolved by:

  • https://github.com/ericniebler/range-v3/pull/1655

brevzin avatar Oct 19 '21 15:10 brevzin

@ericniebler Could you review this? This is a major blocker for gcc-12 interoperability with std::views and ranges-v3.

marehr avatar Jan 29 '22 16:01 marehr

Do all of the views work with non-semi regular views? How do you know? Where are the tests?

ericniebler avatar Jan 29 '22 16:01 ericniebler

Do all of the views work with non-semi regular views? How do you know?

As https://github.com/ericniebler/range-v3/issues/1662#issuecomment-938677396 commented, http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2325r3.html#uses-of-default-construction (the paper that reasoned the change in the standard) did an analysis that not much would break.

Where are the tests?

My questions are:

  • What would be an acceptable state of this PR to get it merged in?
  • Should a test case be added for each view whether a non-default constructible view can be piped with it?
  • How do you want the structure to be for such a test?

Let's take views::reverse as an example https://github.com/ericniebler/range-v3/blob/9aa032ccd0eb4bd77f58e5b181168f1038c222c6/test/view/reverse.cpp#L31-L33

Would the following suffice as a test case

    {
        std::vector<int> vec {0,1,2,3,4,5,6,7,8,9};
        auto const v = vec | debug_non_default_constructible_all_view | views::reverse;
        CHECK(v.size() == 10u);
        ::check_equal(v, {9,8,7,6,5,4,3,2,1,0});
    }

?

We would want to make a push for this, it just isn't clear to us, what needs to be done. Any direction would be helpful. (The PR as it is, would make our code already compile perfectly fine.)

marehr avatar Jan 29 '22 21:01 marehr

Yup, that would do it. Thanks for making the effort.

ericniebler avatar Jan 29 '22 23:01 ericniebler

Hello, what is the status of this PR?

mr-c avatar May 05 '22 13:05 mr-c

@mr-c With #1709 being merged I think this PR is no longer needed?

I guess the next step is to make ranges::view_ requiring moveable instead of copyable, yet I have no idea on how to proceed.

gnaggnoyil avatar Dec 16 '22 11:12 gnaggnoyil

AFAIK, we solved the problem by dropping range-v3 as dep and reimplementing some of the views ourselves.

marehr avatar Jan 25 '24 16:01 marehr