-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Update Enumerable.Sequence based on post-merge feedback #116793
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
Conversation
stephentoub
commented
Jun 18, 2025
- Use T.IsPositive/IsZero
- Put zero check first
- Throw for NaN
- Rename a parameter to clarify its purpose
- Add some comments
|
Tagging subscribers to this area: @dotnet/area-system-linq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Sequence and AsyncSequence APIs to validate NaN, handle a zero step specially, renames a test method, and fleshes out the ReferenceAddable stub with the new INumber<T> members.
- Add
T.IsNaNchecks before range and step logic - Introduce explicit handling when
step == 0to either repeat once or throw - Rename tests to
InvalidArguments_Throwsand implement new INumber members inReferenceAddable
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Linq/tests/SequenceTests.cs | Renamed test and added NaN-argument assertions |
| src/libraries/System.Linq/src/System/Linq/Sequence.cs | Added NaN and zero-step checks and refactored iterator locals |
| src/libraries/System.Linq.AsyncEnumerable/tests/SequenceTests.cs | Renamed test and added NaN-argument assertions |
| src/libraries/System.Linq.AsyncEnumerable/src/System/Linq/Sequence.cs | Added NaN and zero-step checks and refactored iterator locals |
Comments suppressed due to low confidence (2)
src/libraries/System.Linq/tests/SequenceTests.cs:23
- Add tests for the zero-step behavior: when
step == T.Zeroandstart == endInclusive, the sequence should yield exactly one element, and whenstart != endInclusive, it should throwArgumentOutOfRangeException.
AssertExtensions.Throws<ArgumentOutOfRangeException>("step", () => Enumerable.Sequence(1.0f, 1.0f, float.NaN));
src/libraries/System.Linq.AsyncEnumerable/tests/SequenceTests.cs:24
- Similarly, add async tests covering the
step == T.Zerocases: one element whenstart == endInclusiveand throwing whenstart != endInclusive.
AssertExtensions.Throws<ArgumentOutOfRangeException>("step", () => AsyncEnumerable.Sequence(1.0f, 1.0f, float.NaN));
- Use T.IsPositive/IsZero - Put zero check first - Throw for NaN - Rename a parameter to clarify its purpose - Add some comments
588c1b0 to
a10100c
Compare
|
/ba-g infra failures |