Fix IAsyncEnumerable SkipLast for count = 0 by thibault-reigner · Pull Request #1782 · dotnet/reactive

I see...so there is a case in which we have to return source unchanged but have to 'hide' the object identity. I wasn't aware of that.

I am thinking about not touching the implementation of SkipLast, but instead having a generic way to wrap source but keep its behaviour. AsAsyncEnumerable comes to mind, but that'll only return the object itself.

Is there more cases like this one, where Core is called only to hide source? If so, could you provide a more generic solution, like e.g. a Wrap method? I think that would be the cleanest solution because we wouldn't touch any implementations and correctness of the changes would be immediately obvious.

In any case, good find!

I see...so there is a case in which we have to return source unchanged but have to 'hide' the object identity. I wasn't aware of that.

I am thinking about not touching the implementation of SkipLast, but instead having a generic way to wrap source but keep its behaviour. AsAsyncEnumerable comes to mind, but that'll only return the object itself.

Is there more cases like this one, where Core is called only to hide source? If so, could you provide a more generic solution, like e.g. a Wrap method? I think that would be the cleanest solution because we wouldn't touch any implementations and correctness of the changes would be immediately obvious.

In any case, good find!

This seems to be a bit out of scope of this PR and quite some work, as it requires to search for such pattern in all extensions methods of IAsyncEnumerable.

Also, I noticed that there is an extensive use of GetAsyncEnumerator() + MoveNextAsync() instead of await foreach in this library, most likely due to the fact it predates the introduction of IAsyncEnumerable and C# 8 await foreach.
It led to extra complexity, and here bug, with no easy way to compare methods with their synchronous variations for IEnumerable (which here was bug-free), so maybe a transition would highlight some extra bugs but also potential factorization.

Do you think such rework would be worth it to spot places where some simplfying logic such as the Wrap you suggest are relevant ?