Implementing ExactSizeIterator

[dcormier]

Also implemented FusedIterator

[parasyte]

Thank you for the contribution! Did you have any additional context for the utility of exact size iterator for error source iteration?

For comparison, core::error::Source implements FusedIterator, but not ExactSizeIterator. This crate is intended to be compatible with the nightly-only error_iter feature, not to extend its capabilities.

I also haven't seen any discussion on whether ExactSizeIterator should or will be implemented for core::error::Source. Are you aware of any?

[parasyte]

Since size_hint is now iterating the remainder of the chain, I don't know if that could be called an optimization (using terminology from the size_hint documentation).

What you have here is the correct thing to do for ExactSizeIterator, however.

[dcormier]

Did you have any additional context for the utility of exact size iterator for error source iteration?

The intention was to able to optimize .collect() calls from this iterator, since things like the FromIterator implementation for Vec can take advantage of that to pre-allocate.

It may not be worth it, but I figured that calculation would be cheap enough that it's still a net benefit.

I also haven't seen any discussion on whether ExactSizeIterator should or will be implemented for core::error::Source. Are you aware of any?

I am not.

[parasyte]

I don't really know what to do here. If we make the iterator implement ExactSizeIterator and the standard library does not, that breaks an implied compatibility guarantee that this crates sets for itself. Quoting the readme:

Use Error::sources on stable Rust.

If the goal was to use Error::sources and other bells and whistles, then I would quickly accept this.

On the other hand, having the FusedIterator impl is very attractive because that is something that the standard library does provide.

Sorry I don't have a decision, here. But I wanted to offer my thoughts so that it doesn't look like this was forgotten.

[dcormier]

Sorry I don't have a decision, here. But I wanted to offer my thoughts so that it doesn't look like this was forgotten.

I appreciate that.

Some thoughts, in case it's helpful:

If the goal of this crate is to mirror the current behavior of the unstable feature, then I can certainly understand only adding FusedIterator.

If you'd like to expand this crate beyond the functionality of the feature, then maybe some or all of this PR is worth considering?

[parasyte]

I'm trying to think of a way to incorporate this, and an optional feature flag might be the best way to make progress.

But before that, I think it would be prudent to benchmark the before and after cases with collect() to ensure it does actually improve performance. It's hard to say if it's the allocation costs or the iteration costs in the len() impl that dominate without benchmarks. If performance is just a wash with varying iterator sizes, I would not accept the ExactSizeIterator impl at all.