API Proposal: SequenceReader<T>.TryPeek overloads for look ahead

[davidfowl]

Today it isn't easy to get the value at a specific position from the ReadOnlySequence<T> making it hard to write parsers that look ahead at data without moving the reader.

public partial ref struct SequenceReader<T>
{
    bool TryPeek(SequencePosition position, out T value);
    bool TryPeek(int count, out T value);
}

cc @softworkz

[JeremyKuhne]

I think a better choice here would be:

public partial ref struct SequenceReader<T>
{
    bool TryPeek(int skip, out T value);
}

Picking an arbitrary start position makes the reader confusing- it has a position and is mostly forward-only. In addition the perf characteristics would be way out of line with the related methods.

[softworkz]

You mean that this method should actually advance the reader?

[JeremyKuhne]

You mean that this method should actually advance the reader?

No, the peek methods shouldn't advance the reader at all. The idea here is that you can peek ahead for a value without having to get a set of values or rewinding the reader. Say, peek for a CR then peek again for a LF, rather than a CR/LF (as a simple example).

[softworkz]

Ah ok. So what are you implying by changing:

bool TryPeek(int count, out T value);

to

bool TryPeek(int skip, out T value);

Was it just about a better parameter naming then?

[JeremyKuhne]

Was it just about a better parameter naming then?

Yep.

[softworkz]

I apologize - I misunderstood.

[ahsonkhan]

Are we fine with having to incur the cost of slicing the sequence and fetching the span again (for the API that takes a SequencePosition)? The other existing APIs on SequenceReader do not have this overhead. This concern would apply to all the APIs that take an arbitrary SequencePosition.

Maybe, something like this:

public readonly bool TryPeek(SequencePosition position, out T value)
{
    ReadOnlySequence<T> sequence = Sequence.Slice(position);

    ReadOnlySpan<T> span = sequence.FirstSpan;

    if (!span.IsEmpty)
    {
        value = span[0];
        return true;
    }
    else
    {
        value = default;
        return false;
    }
}

[JeremyKuhne]

Video

Approved: dotnet/corefx#40962 (comment):

        public readonly bool TryPeek(long offset, out T value);

[ahsonkhan]

@JeremyKuhne - the parameter name isn't skip. It should be offset.

Also, don't forget the readonly keyword (+ the comment link needs to be updated).

[JeremyKuhne]

the parameter name isn't skip. It should be offset.

Copy/paste fail. Fixing.