Skip to content

Commit 9b4f83e

Browse files
authored
Update Enumerable.Sequence based on post-merge feedback (#116793)
* Update Enumerable.Sequence based on post-merge feedback - Use T.IsPositive/IsZero - Put zero check first - Throw for NaN - Rename a parameter to clarify its purpose - Add some comments * Add exception docs
1 parent 3ae4edb commit 9b4f83e

File tree

4 files changed

+108
-64
lines changed

4 files changed

+108
-64
lines changed

src/libraries/System.Linq.AsyncEnumerable/src/System/Linq/Sequence.cs

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,18 @@ namespace System.Linq
1111
{
1212
public static partial class AsyncEnumerable
1313
{
14-
/// <summary>Generates a sequence that begins with <paramref name="start"/> and yields additional values each incremented by <paramref name="step"/> until <paramref name="endInclusive"/> is reached.</summary>
14+
/// <summary>Generates a sequence that begins with <paramref name="start"/> and yields additional values each incremented by <paramref name="step"/> until <paramref name="endInclusive"/> is reached.</summary>
1515
/// <typeparam name="T">The type of the value to be yielded in the result sequence.</typeparam>
1616
/// <param name="start">The starting value. This value will always be included in the resulting sequence.</param>
1717
/// <param name="endInclusive">The ending bound beyond which values will not be included in the sequence.</param>
1818
/// <param name="step">The amount by which the next value in the sequence should be incremented from the previous value.</param>
1919
/// <returns>An <see cref="IAsyncEnumerable{T}"/> that contains the sequence.</returns>
2020
/// <exception cref="ArgumentNullException"><paramref name="start"/> is <see langword="null"/>.</exception>
21-
/// <exception cref="ArgumentNullException"><paramref name="step"/> is <see langword="null"/>.</exception>
2221
/// <exception cref="ArgumentNullException"><paramref name="endInclusive"/> is <see langword="null"/>.</exception>
22+
/// <exception cref="ArgumentNullException"><paramref name="step"/> is <see langword="null"/>.</exception>
23+
/// <exception cref="ArgumentOutOfRangeException"><paramref name="start"/> is NaN.</exception>
24+
/// <exception cref="ArgumentOutOfRangeException"><paramref name="endInclusive"/> is NaN.</exception>
25+
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is NaN.</exception>
2326
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is greater than zero but <paramref name="endInclusive"/> is less than <paramref name="start"/>.</exception>
2427
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is less than zero but <paramref name="endInclusive"/> is greater than <paramref name="start"/>.</exception>
2528
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is zero and <paramref name="endInclusive"/> does not equal <paramref name="start"/>.</exception>
@@ -30,17 +33,43 @@ public static IAsyncEnumerable<T> Sequence<T>(T start, T endInclusive, T step) w
3033
ThrowHelper.ThrowArgumentNullException(nameof(start));
3134
}
3235

36+
if (T.IsNaN(start))
37+
{
38+
ThrowHelper.ThrowArgumentOutOfRangeException(nameof(start));
39+
}
40+
3341
if (endInclusive is null)
3442
{
3543
ThrowHelper.ThrowArgumentNullException(nameof(endInclusive));
3644
}
3745

46+
if (T.IsNaN(endInclusive))
47+
{
48+
ThrowHelper.ThrowArgumentOutOfRangeException(nameof(endInclusive));
49+
}
50+
3851
if (step is null)
3952
{
4053
ThrowHelper.ThrowArgumentNullException(nameof(step));
4154
}
4255

43-
if (step > T.Zero)
56+
if (T.IsNaN(step))
57+
{
58+
ThrowHelper.ThrowArgumentOutOfRangeException(nameof(step));
59+
}
60+
61+
if (T.IsZero(step))
62+
{
63+
// step == 0. If start != endInclusive, then the sequence would be infinite. As such, we validate
64+
// that they're equal, and if they are, we return a sequence that yields the start/endInclusive value once.
65+
if (start != endInclusive)
66+
{
67+
ThrowHelper.ThrowArgumentOutOfRangeException(nameof(step));
68+
}
69+
70+
return Repeat(start, 1);
71+
}
72+
else if (T.IsPositive(step))
4473
{
4574
// Presumed to be the most common case, step > 0. Validate that endInclusive >= start, as otherwise we can't easily
4675
// guarantee that the sequence will terminate.
@@ -52,7 +81,7 @@ public static IAsyncEnumerable<T> Sequence<T>(T start, T endInclusive, T step) w
5281
// Otherwise, just produce an incrementing sequence.
5382
return IncrementingIterator(start, endInclusive, step);
5483
}
55-
else if (step < T.Zero)
84+
else
5685
{
5786
// step < 0. Validate that endInclusive <= start, as otherwise we can't easily guarantee that the sequence will terminate.
5887
if (endInclusive > start)
@@ -63,31 +92,20 @@ public static IAsyncEnumerable<T> Sequence<T>(T start, T endInclusive, T step) w
6392
// Then produce the decrementing sequence.
6493
return DecrementingIterator(start, endInclusive, step);
6594
}
66-
else
67-
{
68-
// step == 0. If start != endInclusive, then the sequence would be infinite. As such, we validate
69-
// that they're equal, and if they are, we return a sequence that yields the start/endInclusive value once.
70-
if (start != endInclusive)
71-
{
72-
ThrowHelper.ThrowArgumentOutOfRangeException(nameof(step));
73-
}
74-
75-
return Repeat(start, 1);
76-
}
7795

78-
static async IAsyncEnumerable<T> IncrementingIterator(T start, T endInclusive, T step)
96+
static async IAsyncEnumerable<T> IncrementingIterator(T current, T endInclusive, T step)
7997
{
8098
Debug.Assert(step > T.Zero);
8199

82-
yield return start;
100+
yield return current;
83101

84102
while (true)
85103
{
86-
T next = start + step;
104+
T next = current + step;
87105

88-
if (next >= endInclusive || next <= start)
106+
if (next >= endInclusive || next <= current) // handle overflow and saturation
89107
{
90-
if (next == endInclusive && start != next)
108+
if (next == endInclusive && current != next)
91109
{
92110
yield return next;
93111
}
@@ -96,24 +114,24 @@ static async IAsyncEnumerable<T> IncrementingIterator(T start, T endInclusive, T
96114
}
97115

98116
yield return next;
99-
start = next;
117+
current = next;
100118
}
101119
}
102120

103121

104-
static async IAsyncEnumerable<T> DecrementingIterator(T start, T endInclusive, T step)
122+
static async IAsyncEnumerable<T> DecrementingIterator(T current, T endInclusive, T step)
105123
{
106124
Debug.Assert(step < T.Zero);
107125

108-
yield return start;
126+
yield return current;
109127

110128
while (true)
111129
{
112-
T next = start + step;
130+
T next = current + step;
113131

114-
if (next <= endInclusive || next >= start)
132+
if (next <= endInclusive || next >= current) // handle overflow and saturation
115133
{
116-
if (next == endInclusive && start != next)
134+
if (next == endInclusive && current != next)
117135
{
118136
yield return next;
119137
}
@@ -122,7 +140,7 @@ static async IAsyncEnumerable<T> DecrementingIterator(T start, T endInclusive, T
122140
}
123141

124142
yield return next;
125-
start = next;
143+
current = next;
126144
}
127145
}
128146
}

src/libraries/System.Linq.AsyncEnumerable/tests/SequenceTests.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,15 @@ namespace System.Linq.Tests
1313
public class SequenceTests : AsyncEnumerableTests
1414
{
1515
[Fact]
16-
public void NullArguments_Throws()
16+
public void InvalidArguments_Throws()
1717
{
1818
AssertExtensions.Throws<ArgumentNullException>("start", () => AsyncEnumerable.Sequence((ReferenceAddable)null!, new(1), new(2)));
1919
AssertExtensions.Throws<ArgumentNullException>("endInclusive", () => AsyncEnumerable.Sequence(new(1), (ReferenceAddable)null!, new(2)));
2020
AssertExtensions.Throws<ArgumentNullException>("step", () => AsyncEnumerable.Sequence(new(1), new(2), (ReferenceAddable)null!));
21+
22+
AssertExtensions.Throws<ArgumentOutOfRangeException>("start", () => AsyncEnumerable.Sequence(float.NaN, 1.0f, 1.0f));
23+
AssertExtensions.Throws<ArgumentOutOfRangeException>("endInclusive", () => AsyncEnumerable.Sequence(1.0f, float.NaN, 1.0f));
24+
AssertExtensions.Throws<ArgumentOutOfRangeException>("step", () => AsyncEnumerable.Sequence(1.0f, 1.0f, float.NaN));
2125
}
2226

2327
[Fact]
@@ -251,16 +255,16 @@ private sealed class ReferenceAddable(int value) : INumber<ReferenceAddable>
251255
public static bool IsImaginaryNumber(ReferenceAddable value) => throw new NotImplementedException();
252256
public static bool IsInfinity(ReferenceAddable value) => throw new NotImplementedException();
253257
public static bool IsInteger(ReferenceAddable value) => throw new NotImplementedException();
254-
public static bool IsNaN(ReferenceAddable value) => throw new NotImplementedException();
255-
public static bool IsNegative(ReferenceAddable value) => throw new NotImplementedException();
258+
public static bool IsNaN(ReferenceAddable value) => false;
259+
public static bool IsNegative(ReferenceAddable value) => false;
256260
public static bool IsNegativeInfinity(ReferenceAddable value) => throw new NotImplementedException();
257261
public static bool IsNormal(ReferenceAddable value) => throw new NotImplementedException();
258262
public static bool IsOddInteger(ReferenceAddable value) => throw new NotImplementedException();
259-
public static bool IsPositive(ReferenceAddable value) => throw new NotImplementedException();
263+
public static bool IsPositive(ReferenceAddable value) => false;
260264
public static bool IsPositiveInfinity(ReferenceAddable value) => throw new NotImplementedException();
261265
public static bool IsRealNumber(ReferenceAddable value) => throw new NotImplementedException();
262266
public static bool IsSubnormal(ReferenceAddable value) => throw new NotImplementedException();
263-
public static bool IsZero(ReferenceAddable value) => throw new NotImplementedException();
267+
public static bool IsZero(ReferenceAddable value) => false;
264268
public static ReferenceAddable MaxMagnitude(ReferenceAddable x, ReferenceAddable y) => throw new NotImplementedException();
265269
public static ReferenceAddable MaxMagnitudeNumber(ReferenceAddable x, ReferenceAddable y) => throw new NotImplementedException();
266270
public static ReferenceAddable MinMagnitude(ReferenceAddable x, ReferenceAddable y) => throw new NotImplementedException();

src/libraries/System.Linq/src/System/Linq/Sequence.cs

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,18 @@ namespace System.Linq
1111
{
1212
public static partial class Enumerable
1313
{
14-
/// <summary>Generates a sequence that begins with <paramref name="start"/> and yields additional values each incremented by <paramref name="step"/> until <paramref name="endInclusive"/> is reached.</summary>
14+
/// <summary>Generates a sequence that begins with <paramref name="start"/> and yields additional values each incremented by <paramref name="step"/> until <paramref name="endInclusive"/> is reached.</summary>
1515
/// <typeparam name="T">The type of the value to be yielded in the result sequence.</typeparam>
1616
/// <param name="start">The starting value. This value will always be included in the resulting sequence.</param>
1717
/// <param name="endInclusive">The ending bound beyond which values will not be included in the sequence.</param>
1818
/// <param name="step">The amount by which the next value in the sequence should be incremented from the previous value.</param>
1919
/// <returns>An <see cref="IEnumerable{T}"/> that contains the sequence.</returns>
2020
/// <exception cref="ArgumentNullException"><paramref name="start"/> is <see langword="null"/>.</exception>
21-
/// <exception cref="ArgumentNullException"><paramref name="step"/> is <see langword="null"/>.</exception>
2221
/// <exception cref="ArgumentNullException"><paramref name="endInclusive"/> is <see langword="null"/>.</exception>
22+
/// <exception cref="ArgumentNullException"><paramref name="step"/> is <see langword="null"/>.</exception>
23+
/// <exception cref="ArgumentOutOfRangeException"><paramref name="start"/> is NaN.</exception>
24+
/// <exception cref="ArgumentOutOfRangeException"><paramref name="endInclusive"/> is NaN.</exception>
25+
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is NaN.</exception>
2326
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is greater than zero but <paramref name="endInclusive"/> is less than <paramref name="start"/>.</exception>
2427
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is less than zero but <paramref name="endInclusive"/> is greater than <paramref name="start"/>.</exception>
2528
/// <exception cref="ArgumentOutOfRangeException"><paramref name="step"/> is zero and <paramref name="endInclusive"/> does not equal <paramref name="start"/>.</exception>
@@ -30,17 +33,43 @@ public static IEnumerable<T> Sequence<T>(T start, T endInclusive, T step) where
3033
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.start);
3134
}
3235

36+
if (T.IsNaN(start))
37+
{
38+
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.start);
39+
}
40+
3341
if (endInclusive is null)
3442
{
3543
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.endInclusive);
3644
}
3745

46+
if (T.IsNaN(endInclusive))
47+
{
48+
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.endInclusive);
49+
}
50+
3851
if (step is null)
3952
{
4053
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.step);
4154
}
4255

43-
if (step > T.Zero)
56+
if (T.IsNaN(step))
57+
{
58+
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.step);
59+
}
60+
61+
if (T.IsZero(step))
62+
{
63+
// If start != endInclusive, then the sequence would be infinite. As such, we validate
64+
// that they're equal, and if they are, we return a sequence that yields the start/endInclusive value once.
65+
if (start != endInclusive)
66+
{
67+
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.step);
68+
}
69+
70+
return Repeat(start, 1);
71+
}
72+
else if (T.IsPositive(step))
4473
{
4574
// Presumed to be the most common case, step > 0. Validate that endInclusive >= start, as otherwise we can't easily
4675
// guarantee that the sequence will terminate.
@@ -67,7 +96,7 @@ public static IEnumerable<T> Sequence<T>(T start, T endInclusive, T step) where
6796
// Otherwise, just produce an incrementing sequence.
6897
return IncrementingIterator(start, endInclusive, step);
6998
}
70-
else if (step < T.Zero)
99+
else
71100
{
72101
// step < 0. Validate that endInclusive <= start, as otherwise we can't easily guarantee that the sequence will terminate.
73102
if (endInclusive > start)
@@ -78,17 +107,6 @@ public static IEnumerable<T> Sequence<T>(T start, T endInclusive, T step) where
78107
// Then produce the decrementing sequence.
79108
return DecrementingIterator(start, endInclusive, step);
80109
}
81-
else
82-
{
83-
// step == 0. If start != endInclusive, then the sequence would be infinite. As such, we validate
84-
// that they're equal, and if they are, we return a sequence that yields the start/endInclusive value once.
85-
if (start != endInclusive)
86-
{
87-
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.step);
88-
}
89-
90-
return Repeat(start, 1);
91-
}
92110

93111
static RangeIterator<T>? TryUseRange<TLarger>(T start, T endInclusive, T step, TLarger maxValue) where TLarger : INumber<TLarger>
94112
{
@@ -101,19 +119,19 @@ public static IEnumerable<T> Sequence<T>(T start, T endInclusive, T step) where
101119
return null;
102120
}
103121

104-
static IEnumerable<T> IncrementingIterator(T start, T endInclusive, T step)
122+
static IEnumerable<T> IncrementingIterator(T current, T endInclusive, T step)
105123
{
106124
Debug.Assert(step > T.Zero);
107125

108-
yield return start;
126+
yield return current;
109127

110128
while (true)
111129
{
112-
T next = start + step;
130+
T next = current + step;
113131

114-
if (next >= endInclusive || next <= start)
132+
if (next >= endInclusive || next <= current) // handle overflow and saturation
115133
{
116-
if (next == endInclusive && start != next)
134+
if (next == endInclusive && current != next)
117135
{
118136
yield return next;
119137
}
@@ -122,24 +140,24 @@ static IEnumerable<T> IncrementingIterator(T start, T endInclusive, T step)
122140
}
123141

124142
yield return next;
125-
start = next;
143+
current = next;
126144
}
127145
}
128146

129147

130-
static IEnumerable<T> DecrementingIterator(T start, T endInclusive, T step)
148+
static IEnumerable<T> DecrementingIterator(T current, T endInclusive, T step)
131149
{
132150
Debug.Assert(step < T.Zero);
133151

134-
yield return start;
152+
yield return current;
135153

136154
while (true)
137155
{
138-
T next = start + step;
156+
T next = current + step;
139157

140-
if (next <= endInclusive || next >= start)
158+
if (next <= endInclusive || next >= current) // handle overflow and saturation
141159
{
142-
if (next == endInclusive && start != next)
160+
if (next == endInclusive && current != next)
143161
{
144162
yield return next;
145163
}
@@ -148,7 +166,7 @@ static IEnumerable<T> DecrementingIterator(T start, T endInclusive, T step)
148166
}
149167

150168
yield return next;
151-
start = next;
169+
current = next;
152170
}
153171
}
154172
}

0 commit comments

Comments
 (0)