Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LINQ Last() in ConcatNIterator to also check base case (Concat2Iterator) #108486

Merged

Conversation

andrewjsaid
Copy link
Contributor

fixes #108477

ConcatNIterator.PreviousN returns null in the base case as _tail is Concat2Iterator. I've copied the ConcatNIterator<TSource>? node, previousN = this; pattern used in other methods over to here and added test cases

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @andrewjsaid!

This looks good to me, but I'd like @krwq or @eiriktsarpalis to review as well before merge. Once the fix is merged, we'll backport this to .NET 9 as well.

@krwq
Copy link
Member

krwq commented Oct 8, 2024

@andrewjsaid thanks for the contribution! Have you perhaps checked if other APIs in the speed optimizations might have similar issues? (i.e. TryGetFirst or similar)

@andrewjsaid
Copy link
Contributor Author

@krwq You're welcome. Yes I did check and it looks like Last() was the only one missing it.

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@eiriktsarpalis eiriktsarpalis merged commit 124dd32 into dotnet:main Oct 8, 2024
81 of 85 checks passed
@eiriktsarpalis
Copy link
Member

/backport to release/9.0

Copy link
Contributor

github-actions bot commented Oct 8, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Linq community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Last() throws an exception when applied on a concatenation of a collection with two empty collections
5 participants