Skip to content

Commit cbfad3c

Browse files
stephentoubstephentoub
committed
Merge pull request dotnet/corefx#6843 from JonHanna/fix_6819_more
Fix off-by-one similar to dotnet/corefx#6819 in other cases. Commit migrated from dotnet/corefx@c877993
2 parents 0ec8c59 + c2b381e commit cbfad3c

7 files changed

Lines changed: 121 additions & 62 deletions

File tree

‎src/libraries/System.Linq/src/System/Linq/Partition.cs‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -140,19 +140,19 @@ public int GetCount(bool onlyIfCheap)
140140
internal sealed class OrderedPartition<TElement> : IPartition<TElement>
141141
{
142142
private readonly OrderedEnumerable<TElement> _source;
143-
private readonly int _minIndex;
144-
private readonly int _maxIndex;
143+
private readonly int _minIndexInclusive;
144+
private readonly int _maxIndexInclusive;
145145

146-
public OrderedPartition(OrderedEnumerable<TElement> source, int minIdx, int maxIdx)
146+
public OrderedPartition(OrderedEnumerable<TElement> source, int minIdxInclusive, int maxIdxInclusive)
147147
{
148148
_source = source;
149-
_minIndex = minIdx;
150-
_maxIndex = maxIdx;
149+
_minIndexInclusive = minIdxInclusive;
150+
_maxIndexInclusive = maxIdxInclusive;
151151
}
152152

153153
public IEnumerator<TElement> GetEnumerator()
154154
{
155-
return _source.GetEnumerator(_minIndex, _maxIndex);
155+
return _source.GetEnumerator(_minIndexInclusive, _maxIndexInclusive);
156156
}
157157

158158
IEnumerator IEnumerable.GetEnumerator()
@@ -162,26 +162,26 @@ IEnumerator IEnumerable.GetEnumerator()
162162

163163
public IPartition<TElement> Skip(int count)
164164
{
165-
int minIndex = _minIndex + count;
166-
return (uint)minIndex > (uint)_maxIndex ? EmptyPartition<TElement>.Instance : new OrderedPartition<TElement>(_source, minIndex, _maxIndex);
165+
int minIndex = _minIndexInclusive + count;
166+
return (uint)minIndex > (uint)_maxIndexInclusive ? EmptyPartition<TElement>.Instance : new OrderedPartition<TElement>(_source, minIndex, _maxIndexInclusive);
167167
}
168168

169169
public IPartition<TElement> Take(int count)
170170
{
171-
int maxIndex = _minIndex + count - 1;
172-
if ((uint)maxIndex >= (uint)_maxIndex)
171+
int maxIndex = _minIndexInclusive + count - 1;
172+
if ((uint)maxIndex >= (uint)_maxIndexInclusive)
173173
{
174174
return this;
175175
}
176176

177-
return new OrderedPartition<TElement>(_source, _minIndex, maxIndex);
177+
return new OrderedPartition<TElement>(_source, _minIndexInclusive, maxIndex);
178178
}
179179

180180
public TElement TryGetElementAt(int index, out bool found)
181181
{
182-
if ((uint)index <= (uint)(_maxIndex - _minIndex))
182+
if ((uint)index <= (uint)(_maxIndexInclusive - _minIndexInclusive))
183183
{
184-
return _source.TryGetElementAt(index + _minIndex, out found);
184+
return _source.TryGetElementAt(index + _minIndexInclusive, out found);
185185
}
186186

187187
found = false;
@@ -190,27 +190,27 @@ public TElement TryGetElementAt(int index, out bool found)
190190

191191
public TElement TryGetFirst(out bool found)
192192
{
193-
return _source.TryGetElementAt(_minIndex, out found);
193+
return _source.TryGetElementAt(_minIndexInclusive, out found);
194194
}
195195

196196
public TElement TryGetLast(out bool found)
197197
{
198-
return _source.TryGetLast(_minIndex, _maxIndex, out found);
198+
return _source.TryGetLast(_minIndexInclusive, _maxIndexInclusive, out found);
199199
}
200200

201201
public TElement[] ToArray()
202202
{
203-
return _source.ToArray(_minIndex, _maxIndex);
203+
return _source.ToArray(_minIndexInclusive, _maxIndexInclusive);
204204
}
205205

206206
public List<TElement> ToList()
207207
{
208-
return _source.ToList(_minIndex, _maxIndex);
208+
return _source.ToList(_minIndexInclusive, _maxIndexInclusive);
209209
}
210210

211211
public int GetCount(bool onlyIfCheap)
212212
{
213-
return _source.GetCount(_minIndex, _maxIndex, onlyIfCheap);
213+
return _source.GetCount(_minIndexInclusive, _maxIndexInclusive, onlyIfCheap);
214214
}
215215
}
216216

@@ -219,8 +219,8 @@ public static partial class Enumerable
219219
private sealed class ListPartition<TSource> : Iterator<TSource>, IPartition<TSource>
220220
{
221221
private readonly IList<TSource> _source;
222-
private readonly int _minIndex;
223-
private readonly int _maxIndex;
222+
private readonly int _minIndexInclusive;
223+
private readonly int _maxIndexInclusive;
224224
private int _index;
225225

226226
public ListPartition(IList<TSource> source, int minIndexInclusive, int maxIndexInclusive)
@@ -229,19 +229,19 @@ public ListPartition(IList<TSource> source, int minIndexInclusive, int maxIndexI
229229
Debug.Assert(minIndexInclusive >= 0);
230230
Debug.Assert(minIndexInclusive <= maxIndexInclusive);
231231
_source = source;
232-
_minIndex = minIndexInclusive;
233-
_maxIndex = maxIndexInclusive;
232+
_minIndexInclusive = minIndexInclusive;
233+
_maxIndexInclusive = maxIndexInclusive;
234234
_index = minIndexInclusive;
235235
}
236236

237237
public override Iterator<TSource> Clone()
238238
{
239-
return new ListPartition<TSource>(_source, _minIndex, _maxIndex);
239+
return new ListPartition<TSource>(_source, _minIndexInclusive, _maxIndexInclusive);
240240
}
241241

242242
public override bool MoveNext()
243243
{
244-
if ((_state == 1 & _index <= _maxIndex) && _index < _source.Count)
244+
if ((_state == 1 & _index <= _maxIndexInclusive) && _index < _source.Count)
245245
{
246246
_current = _source[_index];
247247
++_index;
@@ -254,27 +254,27 @@ public override bool MoveNext()
254254

255255
public override IEnumerable<TResult> Select<TResult>(Func<TSource, TResult> selector)
256256
{
257-
return new SelectListPartitionIterator<TSource, TResult>(_source, selector, _minIndex, _maxIndex);
257+
return new SelectListPartitionIterator<TSource, TResult>(_source, selector, _minIndexInclusive, _maxIndexInclusive);
258258
}
259259

260260
public IPartition<TSource> Skip(int count)
261261
{
262-
int minIndex = _minIndex + count;
263-
return minIndex >= _maxIndex ? EmptyPartition<TSource>.Instance : new ListPartition<TSource>(_source, minIndex, _maxIndex);
262+
int minIndex = _minIndexInclusive + count;
263+
return (uint)minIndex > (uint)_maxIndexInclusive ? EmptyPartition<TSource>.Instance : new ListPartition<TSource>(_source, minIndex, _maxIndexInclusive);
264264
}
265265

266266
public IPartition<TSource> Take(int count)
267267
{
268-
int maxIndex = _minIndex + count - 1;
269-
return (uint)maxIndex >= (uint)_maxIndex ? this : new ListPartition<TSource>(_source, _minIndex, maxIndex);
268+
int maxIndex = _minIndexInclusive + count - 1;
269+
return (uint)maxIndex >= (uint)_maxIndexInclusive ? this : new ListPartition<TSource>(_source, _minIndexInclusive, maxIndex);
270270
}
271271

272272
public TSource TryGetElementAt(int index, out bool found)
273273
{
274-
if ((uint)index <= (uint)(_maxIndex - _minIndex) && index < _source.Count - _minIndex)
274+
if ((uint)index <= (uint)(_maxIndexInclusive - _minIndexInclusive) && index < _source.Count - _minIndexInclusive)
275275
{
276276
found = true;
277-
return _source[_minIndex + index];
277+
return _source[_minIndexInclusive + index];
278278
}
279279

280280
found = false;
@@ -283,10 +283,10 @@ public TSource TryGetElementAt(int index, out bool found)
283283

284284
public TSource TryGetFirst(out bool found)
285285
{
286-
if (_source.Count > _minIndex)
286+
if (_source.Count > _minIndexInclusive)
287287
{
288288
found = true;
289-
return _source[_minIndex];
289+
return _source[_minIndexInclusive];
290290
}
291291

292292
found = false;
@@ -296,10 +296,10 @@ public TSource TryGetFirst(out bool found)
296296
public TSource TryGetLast(out bool found)
297297
{
298298
int lastIndex = _source.Count - 1;
299-
if (lastIndex >= _minIndex)
299+
if (lastIndex >= _minIndexInclusive)
300300
{
301301
found = true;
302-
return _source[Math.Min(lastIndex, _maxIndex)];
302+
return _source[Math.Min(lastIndex, _maxIndexInclusive)];
303303
}
304304

305305
found = false;
@@ -311,12 +311,12 @@ private int Count
311311
get
312312
{
313313
int count = _source.Count;
314-
if (count <= _minIndex)
314+
if (count <= _minIndexInclusive)
315315
{
316316
return 0;
317317
}
318318

319-
return Math.Min(count - 1, _maxIndex) - _minIndex + 1;
319+
return Math.Min(count - 1, _maxIndexInclusive) - _minIndexInclusive + 1;
320320
}
321321
}
322322

@@ -329,7 +329,7 @@ public TSource[] ToArray()
329329
}
330330

331331
TSource[] array = new TSource[count];
332-
for (int i = 0, curIdx = _minIndex; i != array.Length; ++i, ++curIdx)
332+
for (int i = 0, curIdx = _minIndexInclusive; i != array.Length; ++i, ++curIdx)
333333
{
334334
array[i] = _source[curIdx];
335335
}
@@ -346,8 +346,8 @@ public List<TSource> ToList()
346346
}
347347

348348
List<TSource> list = new List<TSource>(count);
349-
int end = _minIndex + count;
350-
for (int i = _minIndex; i != end; ++i)
349+
int end = _minIndexInclusive + count;
350+
for (int i = _minIndexInclusive; i != end; ++i)
351351
{
352352
list.Add(_source[i]);
353353
}

‎src/libraries/System.Linq/src/System/Linq/Select.cs‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -686,8 +686,8 @@ private sealed class SelectListPartitionIterator<TSource, TResult> : Iterator<TR
686686
{
687687
private readonly IList<TSource> _source;
688688
private readonly Func<TSource, TResult> _selector;
689-
private readonly int _minIndex;
690-
private readonly int _maxIndex;
689+
private readonly int _minIndexInclusive;
690+
private readonly int _maxIndexInclusive;
691691
private int _index;
692692

693693
public SelectListPartitionIterator(IList<TSource> source, Func<TSource, TResult> selector, int minIndexInclusive, int maxIndexInclusive)
@@ -698,19 +698,19 @@ public SelectListPartitionIterator(IList<TSource> source, Func<TSource, TResult>
698698
Debug.Assert(minIndexInclusive <= maxIndexInclusive);
699699
_source = source;
700700
_selector = selector;
701-
_minIndex = minIndexInclusive;
702-
_maxIndex = maxIndexInclusive;
701+
_minIndexInclusive = minIndexInclusive;
702+
_maxIndexInclusive = maxIndexInclusive;
703703
_index = minIndexInclusive;
704704
}
705705

706706
public override Iterator<TResult> Clone()
707707
{
708-
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndex, _maxIndex);
708+
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndexInclusive, _maxIndexInclusive);
709709
}
710710

711711
public override bool MoveNext()
712712
{
713-
if ((_state == 1 & _index <= _maxIndex) && _index < _source.Count)
713+
if ((_state == 1 & _index <= _maxIndexInclusive) && _index < _source.Count)
714714
{
715715
_current = _selector(_source[_index]);
716716
++_index;
@@ -723,28 +723,28 @@ public override bool MoveNext()
723723

724724
public override IEnumerable<TResult2> Select<TResult2>(Func<TResult, TResult2> selector)
725725
{
726-
return new SelectListPartitionIterator<TSource, TResult2>(_source, CombineSelectors(_selector, selector), _minIndex, _maxIndex);
726+
return new SelectListPartitionIterator<TSource, TResult2>(_source, CombineSelectors(_selector, selector), _minIndexInclusive, _maxIndexInclusive);
727727
}
728728

729729
public IPartition<TResult> Skip(int count)
730730
{
731731
Debug.Assert(count > 0);
732-
int minIndex = _minIndex + count;
733-
return minIndex >= _maxIndex ? EmptyPartition<TResult>.Instance : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, minIndex, _maxIndex);
732+
int minIndex = _minIndexInclusive + count;
733+
return (uint)minIndex > (uint)_maxIndexInclusive ? EmptyPartition<TResult>.Instance : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, minIndex, _maxIndexInclusive);
734734
}
735735

736736
public IPartition<TResult> Take(int count)
737737
{
738-
int maxIndex = _minIndex + count - 1;
739-
return (uint)maxIndex >= (uint)_maxIndex ? this : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndex, maxIndex);
738+
int maxIndex = _minIndexInclusive + count - 1;
739+
return (uint)maxIndex >= (uint)_maxIndexInclusive ? this : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndexInclusive, maxIndex);
740740
}
741741

742742
public TResult TryGetElementAt(int index, out bool found)
743743
{
744-
if ((uint)index <= (uint)(_maxIndex - _minIndex) && index < _source.Count - _minIndex)
744+
if ((uint)index <= (uint)(_maxIndexInclusive - _minIndexInclusive) && index < _source.Count - _minIndexInclusive)
745745
{
746746
found = true;
747-
return _selector(_source[_minIndex + index]);
747+
return _selector(_source[_minIndexInclusive + index]);
748748
}
749749

750750
found = false;
@@ -753,10 +753,10 @@ public TResult TryGetElementAt(int index, out bool found)
753753

754754
public TResult TryGetFirst(out bool found)
755755
{
756-
if (_source.Count > _minIndex)
756+
if (_source.Count > _minIndexInclusive)
757757
{
758758
found = true;
759-
return _selector(_source[_minIndex]);
759+
return _selector(_source[_minIndexInclusive]);
760760
}
761761

762762
found = false;
@@ -766,10 +766,10 @@ public TResult TryGetFirst(out bool found)
766766
public TResult TryGetLast(out bool found)
767767
{
768768
int lastIndex = _source.Count - 1;
769-
if (lastIndex >= _minIndex)
769+
if (lastIndex >= _minIndexInclusive)
770770
{
771771
found = true;
772-
return _selector(_source[Math.Min(lastIndex, _maxIndex)]);
772+
return _selector(_source[Math.Min(lastIndex, _maxIndexInclusive)]);
773773
}
774774

775775
found = false;
@@ -781,12 +781,12 @@ private int Count
781781
get
782782
{
783783
int count = _source.Count;
784-
if (count <= _minIndex)
784+
if (count <= _minIndexInclusive)
785785
{
786786
return 0;
787787
}
788788

789-
return Math.Min(count - 1, _maxIndex) - _minIndex + 1;
789+
return Math.Min(count - 1, _maxIndexInclusive) - _minIndexInclusive + 1;
790790
}
791791
}
792792

@@ -799,7 +799,7 @@ public TResult[] ToArray()
799799
}
800800

801801
TResult[] array = new TResult[count];
802-
for (int i = 0, curIdx = _minIndex; i != array.Length; ++i, ++curIdx)
802+
for (int i = 0, curIdx = _minIndexInclusive; i != array.Length; ++i, ++curIdx)
803803
{
804804
array[i] = _selector(_source[curIdx]);
805805
}
@@ -816,8 +816,8 @@ public List<TResult> ToList()
816816
}
817817

818818
List<TResult> list = new List<TResult>(count);
819-
int end = _minIndex + count;
820-
for (int i = _minIndex; i != end; ++i)
819+
int end = _minIndexInclusive + count;
820+
for (int i = _minIndexInclusive; i != end; ++i)
821821
{
822822
list.Add(_selector(_source[i]));
823823
}

‎src/libraries/System.Linq/tests/OrderedSubsetting.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,15 @@ public void Count()
329329
Assert.Equal(1, Enumerable.Range(0, 100).Shuffle().OrderBy(i => i).Take(2).Skip(1).Count());
330330
}
331331

332+
[Fact]
333+
public void SkipTakesOnlyOne()
334+
{
335+
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Take(1));
336+
Assert.Equal(new[] { 2 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Skip(1).Take(1));
337+
Assert.Equal(new[] { 3 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Take(3).Skip(2));
338+
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Take(3).Take(1));
339+
}
340+
332341
[Fact]
333342
public void EmptyToArray()
334343
{

‎src/libraries/System.Linq/tests/RangeTests.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,15 @@ public void SkipExcessive()
167167
Assert.Empty(Enumerable.Range(10, 10).Skip(20));
168168
}
169169

170+
[Fact]
171+
public void SkipTakeCanOnlyBeOne()
172+
{
173+
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Take(1));
174+
Assert.Equal(new[] { 2 }, Enumerable.Range(1, 10).Skip(1).Take(1));
175+
Assert.Equal(new[] { 3 }, Enumerable.Range(1, 10).Take(3).Skip(2));
176+
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Take(3).Take(1));
177+
}
178+
170179
[Fact]
171180
public void ElementAt()
172181
{

0 commit comments

Comments
 (0)