ericniebler / range-v3

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

`zip` does not satisfy the semantic requirements of `bidirectional_iterator` when the ranges have different lengths #1592

Open sarah-ek opened 3 years ago

sarah-ek commented 3 years ago

forward_iterator requires equality_comparable, which says that equality must be transitive. this can be violated if we obtain the end iterator and decrement it until we reach the beginning of the range.

#include <range/v3/view/zip.hpp>
#include <ranges>

auto main() -> int {
  int a1[] = {0, 1, 2, 3, 4};
  int a3[] = {0, 1, 2, 3, 4, 5};

  auto rng = ranges::views::zip(a1, a3);
  static_assert(std::ranges::random_access_range<decltype(rng)>);
  auto it1 = rng.begin(); // 0, 0
  auto it2 = rng.end();   // 4, 5
  while (it1 != it2) {
    --it2;
  }
  // it2 is now 0, 1
  auto it3 = it1;
  ++it3; // 1, 1

  assert(it1 == it2);
  assert(it2 == it3);
  assert(it1 != it3);
}
dvirtz commented 3 years ago

The situation is worse, the values pointed to by it2 as it's decremented are not in the zipped range at all. Maybe zip shouldn't be a common range?

MikeGitb commented 3 years ago

For random access , I would expect to either have a runtime check, that both ranges have the same size or just the min of the two. Consequently rng.end() should point at (5/(one-past-4)). Harder to say what should happen for cases, where the bases are bidirectional ranges.

sarah-ek commented 3 years ago

one potential solution would be to "align" the end iterators (and maybe cache the result?) when end is computed, if the ranges are all sized.
if the ranges aren't sized, their zipped range could be a forward range instead.