Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Fix LargeArrayBuilder.CopyTo returning incorrect end-of-copy position #23865

Merged
merged 2 commits into from
Sep 9, 2017
Merged

Fix LargeArrayBuilder.CopyTo returning incorrect end-of-copy position #23865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9db0d1
Fix LargeArrayBuilder.CopyTo returning incorrect end-of-copy position…
OmarTawfik Sep 7, 2017
78da72f
Not using a local function
OmarTawfik Sep 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 40 additions & 17 deletions src/Common/src/System/Collections/Generic/LargeArrayBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ internal CopyPosition(int row, int column)
/// </summary>
internal int Column { get; }

/// <summary>
/// If this position is at the end of the current buffer, returns the position
/// at the start of the next buffer. Otherwise, returns this position.
/// </summary>
/// <param name="endColumn">The length of the current buffer.</param>
public CopyPosition Normalize(int endColumn)
{
Debug.Assert(Column <= endColumn);

return Column == endColumn ?
new CopyPosition(Row + 1, 0) :
this;
}

/// <summary>
/// Gets a string suitable for display in the debugger.
/// </summary>
Expand Down Expand Up @@ -186,7 +200,7 @@ public void CopyTo(T[] array, int arrayIndex, int count)
arrayIndex += toCopy;
}
}

/// <summary>
/// Copies the contents of this builder to the specified array.
/// </summary>
Expand All @@ -197,9 +211,10 @@ public void CopyTo(T[] array, int arrayIndex, int count)
/// <returns>The position in this builder that was copied up to.</returns>
public CopyPosition CopyTo(CopyPosition position, T[] array, int arrayIndex, int count)
{
Debug.Assert(array != null);
Debug.Assert(arrayIndex >= 0);
Debug.Assert(count >= 0 && count <= Count);
Debug.Assert(array?.Length - arrayIndex >= count);
Debug.Assert(count > 0 && count <= Count);
Debug.Assert(array.Length - arrayIndex >= count);

// Go through each buffer, which contains one 'row' of items.
// The index in each buffer is referred to as the 'column'.
Expand All @@ -216,25 +231,33 @@ public CopyPosition CopyTo(CopyPosition position, T[] array, int arrayIndex, int
int row = position.Row;
int column = position.Column;

for (; count > 0; row++, column = 0)
{
T[] buffer = GetBuffer(index: row);
T[] buffer = GetBuffer(row);
Debug.Assert(buffer.Length > column);

// During this iteration, copy until we satisfy `count` or reach the
// end of the current buffer.
int copyCount = Math.Min(buffer.Length, count);
int copied = Math.Min(buffer.Length - column, count);
Array.Copy(buffer, column, array, arrayIndex, copied);

if (copyCount > 0)
{
Array.Copy(buffer, column, array, arrayIndex, copyCount);
arrayIndex += copied;
count -= copied;

arrayIndex += copyCount;
count -= copyCount;
column += copyCount;
}
if (count == 0)
{
return new CopyPosition(row, column + copied).Normalize(buffer.Length);
}

return new CopyPosition(row: row, column: column);
do
{
buffer = GetBuffer(++row);
Debug.Assert(buffer.Length > 0);

copied = Math.Min(buffer.Length, count);
Array.Copy(buffer, 0, array, arrayIndex, copied);

arrayIndex += copied;
count -= copied;
} while (count > 0);

return new CopyPosition(row, copied).Normalize(buffer.Length);
}

/// <summary>
Expand Down
10 changes: 7 additions & 3 deletions src/Common/src/System/Collections/Generic/SparseArrayBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ public SparseArrayBuilder(bool initialize)
/// <param name="count">The number of items to copy.</param>
public void CopyTo(T[] array, int arrayIndex, int count)
{
Debug.Assert(array != null);
Debug.Assert(arrayIndex >= 0);
Debug.Assert(count >= 0 && count <= Count);
Debug.Assert(array?.Length - arrayIndex >= count);
Debug.Assert(array.Length - arrayIndex >= count);

int copied = 0;
var position = CopyPosition.Start;
Expand Down Expand Up @@ -149,8 +150,11 @@ public void CopyTo(T[] array, int arrayIndex, int count)
count -= reservedCount;
}

// Finish copying after the final marker.
_builder.CopyTo(position, array, arrayIndex, count);
if (count > 0)
{
// Finish copying after the final marker.
_builder.CopyTo(position, array, arrayIndex, count);
}
}

/// <summary>
Expand Down
118 changes: 118 additions & 0 deletions src/System.Linq/tests/ConcatTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,5 +421,123 @@ public void GetEnumerableOfConcatCollectionChainFollowedByEnumerableNodeShouldBe
Assert.Equal(0xf00, en.Current);
}
}

[Theory]
[MemberData(nameof(GetToArrayDataSources))]
public void CollectionInterleavedWithLazyEnumerables_ToArray(IEnumerable<int>[] arrays)
{
// See https://github.com/dotnet/corefx/issues/23680

IEnumerable<int> concats = arrays[0];

for (int i = 1; i < arrays.Length; i++)
{
concats = concats.Concat(arrays[i]);
}

int[] results = concats.ToArray();

for (int i = 0; i < results.Length; i++)
{
Assert.Equal(i, results[i]);
}
}

private static IEnumerable<object[]> GetToArrayDataSources()
{
// Marker at the end
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(new int[] { 0 }),
new TestEnumerable<int>(new int[] { 1 }),
new TestEnumerable<int>(new int[] { 2 }),
new int[] { 3 },
}
};

// Marker at beginning
yield return new object[]
{
new IEnumerable<int>[]
{
new int[] { 0 },
new TestEnumerable<int>(new int[] { 1 }),
new TestEnumerable<int>(new int[] { 2 }),
new TestEnumerable<int>(new int[] { 3 }),
}
};

// Marker in middle
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(new int[] { 0 }),
new int[] { 1 },
new TestEnumerable<int>(new int[] { 2 }),
}
};

// Non-marker in middle
yield return new object[]
{
new IEnumerable<int>[]
{
new int[] { 0 },
new TestEnumerable<int>(new int[] { 1 }),
new int[] { 2 },
}
};

// Big arrays (marker in middle)
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(Enumerable.Range(0, 100).ToArray()),
Enumerable.Range(100, 100).ToArray(),
new TestEnumerable<int>(Enumerable.Range(200, 100).ToArray()),
}
};

// Big arrays (non-marker in middle)
yield return new object[]
{
new IEnumerable<int>[]
{
Enumerable.Range(0, 100).ToArray(),
new TestEnumerable<int>(Enumerable.Range(100, 100).ToArray()),
Enumerable.Range(200, 100).ToArray(),
}
};

// Interleaved (first marker)
yield return new object[]
{
new IEnumerable<int>[]
{
new int[] { 0 },
new TestEnumerable<int>(new int[] { 1 }),
new int[] { 2 },
new TestEnumerable<int>(new int[] { 3 }),
new int[] { 4 },
}
};

// Interleaved (first non-marker)
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(new int[] { 0 }),
new int[] { 1 },
new TestEnumerable<int>(new int[] { 2 }),
new int[] { 3 },
new TestEnumerable<int>(new int[] { 4 }),
}
};
}
}
}
111 changes: 111 additions & 0 deletions src/System.Linq/tests/SelectManyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,5 +477,116 @@ public void ThrowOverflowExceptionOnConstituentLargeCounts(int[] counts)
IEnumerable<int> iterator = counts.SelectMany(c => Enumerable.Range(1, c));
Assert.Throws<OverflowException>(() => iterator.Count());
}

[Theory]
[MemberData(nameof(GetToArrayDataSources))]
public void CollectionInterleavedWithLazyEnumerables_ToArray(IEnumerable<int>[] arrays)
{
// See https://github.com/dotnet/corefx/issues/23680

int[] results = arrays.SelectMany(ar => ar).ToArray();

for (int i = 0; i < results.Length; i++)
{
Assert.Equal(i, results[i]);
}
}

private static IEnumerable<object[]> GetToArrayDataSources()
{
// Marker at the end
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(new int[] { 0 }),
new TestEnumerable<int>(new int[] { 1 }),
new TestEnumerable<int>(new int[] { 2 }),
new int[] { 3 },
}
};

// Marker at beginning
yield return new object[]
{
new IEnumerable<int>[]
{
new int[] { 0 },
new TestEnumerable<int>(new int[] { 1 }),
new TestEnumerable<int>(new int[] { 2 }),
new TestEnumerable<int>(new int[] { 3 }),
}
};

// Marker in middle
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(new int[] { 0 }),
new int[] { 1 },
new TestEnumerable<int>(new int[] { 2 }),
}
};

// Non-marker in middle
yield return new object[]
{
new IEnumerable<int>[]
{
new int[] { 0 },
new TestEnumerable<int>(new int[] { 1 }),
new int[] { 2 },
}
};

// Big arrays (marker in middle)
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(Enumerable.Range(0, 100).ToArray()),
Enumerable.Range(100, 100).ToArray(),
new TestEnumerable<int>(Enumerable.Range(200, 100).ToArray()),
}
};

// Big arrays (non-marker in middle)
yield return new object[]
{
new IEnumerable<int>[]
{
Enumerable.Range(0, 100).ToArray(),
new TestEnumerable<int>(Enumerable.Range(100, 100).ToArray()),
Enumerable.Range(200, 100).ToArray(),
}
};

// Interleaved (first marker)
yield return new object[]
{
new IEnumerable<int>[]
{
new int[] { 0 },
new TestEnumerable<int>(new int[] { 1 }),
new int[] { 2 },
new TestEnumerable<int>(new int[] { 3 }),
new int[] { 4 },
}
};

// Interleaved (first non-marker)
yield return new object[]
{
new IEnumerable<int>[]
{
new TestEnumerable<int>(new int[] { 0 }),
new int[] { 1 },
new TestEnumerable<int>(new int[] { 2 }),
new int[] { 3 },
new TestEnumerable<int>(new int[] { 4 }),
}
};
}
}
}