Implement FusedIterator for core::iter::StepBy

libs
1

StepBy currently doesn't implement FusedIterator... I cannot see a reason, not to implement

impl<I> FusedIterator for StepBy<I> where I: FusedIterator {}

Do i miss something?

1 Like
2

I don't think so. This impl seems appropriate. The only questions I'd have are:

  • Should the impl be unconditional?
  • Should the correctness of the impl be dependent independent on the correctness of the underlying Iterator's impl?

The answer to both of those is "no" in my opinion. The API docs explain the behavior of StepBy pretty clearly, so answering "yes" to either of the two questions would constitute a change to how StepBy is implemented which in turn would be a breaking change based on its API documentation. There are also already FusedIterator impls in core whose correctness is dependent on the correctness of the underlying Iterator's impl.

3

I don't understand why you'd answer that question with "no". Did you accidentally write the opposite of what you meant (dependent vs independent)?

The naive implementation of StepBy is only fused if the underlying iterator is fused. If the underlying iterator is incorrectly claiming to be fused, StepBy's claim to be fused would be just as incorrect. Which is completely fine.

Attempting to work around by fusing in StepBy would hurt performance, change existing behaviour, and require specialization. So it's clearly not a sane option.

4

Indeed. It's now fixed.

5

Sounds reasonable. Please send a libs-api ACP.

1 Like
6

Was going to open a PR as suggested, but looks like the OP already did.