ericniebler / stl2

LaTeX and Markdown source for the Ranges TS/STL2 and associated proposals
88 stars 8 forks source link

counted_iterator doing an unneccessary increment #634

Closed ericniebler closed 5 years ago

ericniebler commented 5 years ago

Here's a fun one. This code contains an infinite loop:

  auto rng = views::iota(0) | views::filter([](int i) {return i==0;}) | views::take(1);
  for (int i : rng)
     std::cout << i << std::endl;

It's not easy to see why. view::take::begin() returns a counted_iterator that wraps the filter iterator and the integer 1. When the counted_iterator is incremented, we do the following:

++current;
--length;

That ++current causes the filter iterator to look for another integer that is equal to zero, which (obviously) it won't find before the heat death of the universe.

Instead, counted_iterator::operator++ could be specified as:

if (1 == length)
    length = 0;
else {
    ++current;
    --length;
}

However, this difference is observable since it changes what counted_iterator::base() would return for a last-plus-one iterator.

I'm inclined to say this is not a bug, although it is deeply unfortunate. @CaseyCarter, thoughts?

CaseyCarter commented 5 years ago

I'm inclined to say this is not a bug, although it is deeply unfortunate.

This is yet another case of "we have to form the past-the-end iterator, so it needs to be valid" a la view::generate(...) | view::take(...). I don't think we can or should complicate the design to try to address individual corner cases like this as we come across them.

Notably, fixing this would requires us to touch every counted_iterator function that modifies or observes current.