ericniebler / range-v3

Range library for C++14/17/20, basis for C++20's std::ranges
Other
4.05k stars 437 forks source link

Enumerate with arbitrary starting index #1800

Open serpent7776 opened 8 months ago

serpent7776 commented 8 months ago

Improves enumerate so that it's possible to start at any given index. Similar functionality exists in boost::adaptors::indexed

This is just a draft, it just adds an additional parameter to enumerate_fn::operator(). It works well when used with function call syntax, but:

Another way to implement this would be to transform constructed zip, but I'm not sure that's any better (also doesn't fix the first issue).

JohelEGP commented 8 months ago

doesn't work with pipe syntax without breaking existing user code (big issue)

How come?

increases size of index_view (not sure if an issue) breaks existing ABI of enumerate (probably not an issue)

You can avoid those.

For both, you can have enumerate(range, size) return drop(enumerate(range), size).

For only the first one, you can parameterize the size. Default the size to std::integral_constant<std::size_t, 0> and store it in a base or use [[no_unique_address]].

serpent7776 commented 8 months ago

How come?

When I tried modifying the last test to use enumerate with custom index start I got compilation error

enumerate.cpp:157:29: error: no match for call to ‘(const ranges::views::view_closure<ranges::views::enumerate_fn>) (int)’                                                   
  157 |           | views::enumerate(99); 

I'm assuming this is because enumerate is defined to be view_closure that doesn't accept parameters on operator(): https://github.com/ericniebler/range-v3/blob/97452bb3eb74a73fc86504421a6a27c92bce6b99/include/range/v3/view/enumerate.hpp#L116 I could change that to

RANGES_INLINE_VARIABLE(enumerate_fn, enumerate)

and add overload operator()(size_t n = 0) to enumerate_fn, but that breaks existing code, because range | enumerate no longer works and range | enumerate() is required.

For both, you can have enumerate(range, size) return drop(enumerate(range), size).

This is interesting, but I'm assuming you meant drop(detail::index_view<S, D>(), n)

For only the first one, you can parameterize the size.

I don't think I understand.

JohelEGP commented 8 months ago

For only the first one, you can parameterize the size.

I don't think I understand.

I meant something similar to how the bound of std::ranges::iota_view defaults to std::unreachable_sentinel_t. But in this case, you'd be defaulting the start index to std::integral_constant<std::ranges::range_difference_t<R>, 0>.

serpent7776 commented 7 months ago

I updated the code, so that it works with both function call syntax and piping syntax. I looked at existing code in split_when.hpp.

serpent7776 commented 7 months ago

I'd like to ask if the existing solution is OK or should I go with eliminating the size overhead of index_view

serpent7776 commented 6 months ago

ping @JohelEGP ?

JohelEGP commented 6 months ago

Yes? I'm just a passing contributor. I have no say in what gets merged, or what the correct ways are.

JohelEGP commented 6 months ago

I think you want feedback from a maintainer now.