Audit iterator specializations for side effects

[alexcrichton]

After some discussion about #28125 the libs subteam decided that iterator adaptors should always preserve the same semantics in terms of the convenience methods and such. This was not audited for when all the initial specializations landed, so we should take a look and make sure that everything adheres to this policy.

Additionally, documentation should be added to the Iterator trait methods indicating what form of guarantees you are given (e.g. calling last is equivalent to exhausting the iterator).

triage: I-nominated

[Stebalien]

I've checked libcore/iter.rs. core::iter::Chain::last should exhaust a before b but that's the only bug I found (and it wasn't my bug for once 🎉).

Unless, of course, you care about drop order. I haven't tried to reason about that.

[Stebalien]

The slice iterators (libcore/slice.rs) are fine because they are side-effect free.

[Stebalien]

libcollections/vec and libcore/str are fine because they are also side-effect free.

[Stebalien]

I'd appreciate it if someone else double checked but I think that's all of them.

[nagisa]

Unless, of course, you care about drop order.

This is relevant as well, because implementations of Drop may also (and usually do, in form of memory deallocation) contain side effects.

Can we make a

• list
• of iterators which were specialised
• like this?

[Stebalien]

This is relevant as well, because implementations of Drop may also (and usually do, in form of memory deallocation) contain side effects.

I strongly disagree. Drop has side effects but programmers should never rely on drop order except stack drop order because drop order is often unspecified (e.g. within structs).

---

Drop out of order

• core::iter::Chain::last
• core::iter::Peekable::last
• core::iter::Peekable::count (fixable)
• core::iter::Skip::last (fixable?)

Audited

• core::iter::Chain
• core::iter::Enumerate
• core::iter::Peekable
• core::iter::Skip
• core::iter::Take
• core::iter::Fuse
• core::str::Bytes
• core::slice::Iter
• core::slice::IterMut
• core::slice::Windows
• core::slice::Chunks
• core::slice::ChunksMut
• collections::vec::IntoIter

[nagisa]

I strongly disagree. Drop has side effects but programmers should never rely on drop order except stack drop order because drop order is often unspecified (e.g. within structs).

Ah right, this will be decided by rust-lang/rfcs#744 anyway.

[alexcrichton]

Awesome, thanks for the work here @Stebalien!

[Stebalien]

Updated list of out-of-order drops (didn't notice Peekable::count).

[alexcrichton]

triage: P-medium

[aturon]

(Need to make sure this behavior is documented/explicitly promised.)

[huonw]

Is this done? (Everything in the list above is ✅'d)

[Stebalien]

I believe this is left open as to-be-documented. Also, I don't see any decision as to whether out of order drop is ok.

[steveklabnik]

re-tagging as docs

[steveklabnik]

So I believe that all of these are then documented in Iterator's methods, at least as far as I can tell. Can someone from libs double check me here and help provide any specifics as to what's not documented?

[alexcrichton]

Looks good to me, thanks @steveklabnik!