std: do not modify iterator state in TokenIterator.peek()

[gpanders]

A common property of "peek()" functions is that they provide the next element in the iterator without modifying the internal iterator state. This is true of many other peek() functions in the Zig stdlib but is not true of TokenIterator.peek(), which advances the iterator's index to the beginning of the next token.

This can result in surprising behavior if a user of this function expects peek() to not modify the internal iterator state, especially since the doc comment of the function says that it "Does not advance to the next token".

A similar PR was made recently but closed by the author before it could be discussed: #20379

[squeek502]

One kinda nice property of the existing implementation is that there is a reduction of redundant work done when calling peek and then next.

For example, if we have the string:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaZZ

and use tokenizeScalar(str, 'a'), then:

• calling peek would advance the index to just before the ZZ, and return the ZZ slice
• calling next afterwards would just need to re-read ZZ and return it

With peek not advancing index, then:

• calling peek would iterate over all the a's and return the ZZ
• calling next would again iterate over all the a's and return the ZZ

See the original PR that implemented peek for the very similar justification used at the time: #11925 (comment)

[gpanders]

While my preference is for peek() to not modify the iterator state, I wouldn't object to the status quo so long as it's clearly documented that self.index is modified (the existing doc comment is a bit misleading, or at least ambiguous).

[andrewrk]

The problem statement here seems weak. The API upholds its guarantees. In what context was this discovered and determined to be problematic?

[gpanders]

The context was looking at the following segment of code:

var iter = std.mem.tokenizeScalar(u8, line, ' ');
var ws_start: usize = 0;
while (iter.peek() != null) {
    const whitespace = line[ws_start..iter.index];
    const word = iter.next().?;
    ws_start = iter.index;
    // the rest of the code which operates on `word` and `whitespace`
}

When I encountered this code I was confused how it worked at all based on the assumption that peek() does not modify the iterator state (if that were true, ws_start would always equal iter.index and whitespace would always be an empty slice). It was not until I looked at the implementation of TokenIterator.peek() that I found that index was modified.

(The above code has since been rewritten to avoid this confusion in either case, but this is what prompted the initial question and PR.)

The API upholds its guarantees

Strictly speaking this is true, though explicitly stating that the index is/can be modified might help to avoid future confusion. If a functional change is unwanted I can repurpose this PR into a documentation change.