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.

Sign up for GitHub

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

Jump to bottom

Return singleton enumerators from IEnumerable.GetEnumerator for empty collections #82499

Merged
merged 5 commits into from Feb 23, 2023
Merged

Return singleton enumerators from IEnumerable.GetEnumerator for empty collections #82499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f5b1cc8
Return singleton enumerators from IEnumerable.GetEnumerator for empty…
stephentoub Feb 22, 2023
357903f
Merge branch 'main' into singletonemptyenumerator
stephentoub Feb 23, 2023
a5329bf
Address PR feedback
stephentoub Feb 23, 2023
889720c
Fix a few tests
stephentoub Feb 23, 2023
0ec6463
Merge branch 'main' into singletonemptyenumerator
stephentoub Feb 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libraries/Common/src/System/Collections/Generic/EnumerableHelpers.cs
View file
Open in desktop
Expand Up @@ -8,6 +8,10 @@ namespace System.Collections.Generic
/// </summary>
internal static partial class EnumerableHelpers
{
/// <summary>Gets an enumerator singleton for an empty collection.</summary>
internal static IEnumerator<T> GetEmptyEnumerator<T>() =>
((IEnumerable<T>)Array.Empty<T>()).GetEnumerator();

/// <summary>Converts an enumerable to an array using the same logic as List{T}.</summary>
/// <param name="source">The enumerable to convert.</param>
/// <param name="length">The number of items stored in the resulting array, 0-indexed.</param>
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/tests/System/Collections/IDictionary.Generic.Tests.cs
View file
Open in desktop
Expand Up @@ -421,7 +421,7 @@ public void IDictionary_Generic_Keys_Enumeration_ParentDictionaryModifiedInvalid
ICollection<TKey> keys = dictionary.Keys;
IEnumerator<TKey> keysEnum = keys.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
{
Assert.Throws<InvalidOperationException>(() => keysEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => keysEnum.Reset());
Expand Down Expand Up @@ -528,7 +528,7 @@ public void IDictionary_Generic_Values_Enumeration_ParentDictionaryModifiedInval
ICollection<TValue> values = dictionary.Values;
IEnumerator<TValue> valuesEnum = values.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_Generic_Keys_Values_Enumeration_ThrowsInvalidOperation_WhenParentModified)
{
Assert.Throws<InvalidOperationException>(() => valuesEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => valuesEnum.Reset());
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/tests/System/Collections/IDictionary.NonGeneric.Tests.cs
View file
Open in desktop
Expand Up @@ -392,7 +392,7 @@ public void IDictionary_NonGeneric_Keys_Enumeration_ParentDictionaryModifiedInva
ICollection keys = dictionary.Keys;
IEnumerator keysEnum = keys.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
{
Assert.Throws<InvalidOperationException>(() => keysEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => keysEnum.Reset());
Expand Down Expand Up @@ -487,7 +487,7 @@ public virtual void IDictionary_NonGeneric_Values_Enumeration_ParentDictionaryMo
ICollection values = dictionary.Values;
IEnumerator valuesEnum = values.GetEnumerator();
dictionary.Add(GetNewKey(dictionary), CreateTValue(3432));
if (IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : IDictionary_NonGeneric_Keys_Values_ParentDictionaryModifiedInvalidates)
{
Assert.Throws<InvalidOperationException>(() => valuesEnum.MoveNext());
Assert.Throws<InvalidOperationException>(() => valuesEnum.Reset());
Expand Down
58 changes: 47 additions & 11 deletions src/libraries/Common/tests/System/Collections/IEnumerable.Generic.Tests.cs
View file
Open in desktop
Expand Up @@ -85,6 +85,20 @@ public abstract partial class IEnumerable_Generic_Tests<T> : TestBase<T>
/// </summary>
protected virtual bool Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException => true;

/// <summary>
/// When calling MoveNext or Reset after modification of an empty enumeration, the resulting behavior is
/// undefined. Tests are included to cover two behavioral scenarios:
/// - Throwing an InvalidOperationException
/// - Execute MoveNext or Reset.
///
/// If this property is set to true, the tests ensure that the exception is thrown. The default value is
/// <see cref="Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException"/>.
/// </summary>
protected virtual bool Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException => Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException;

/// <summary>Whether the enumerator returned from GetEnumerator is a singleton instance when the collection is empty.</summary>
protected virtual bool Enumerator_Empty_UsesSingletonInstance => false;

/// <summary>
/// Specifies whether this IEnumerable follows some sort of ordering pattern.
/// </summary>
Expand Down Expand Up @@ -302,6 +316,30 @@ protected enum EnumerableOrder

#region GetEnumerator()

[Fact]
public void IEnumerable_NonGeneric_GetEnumerator_EmptyCollection_UsesSingleton()
{
IEnumerable enumerable = GenericIEnumerableFactory(0);

IEnumerator enumerator1 = enumerable.GetEnumerator();
try
{
IEnumerator enumerator2 = enumerable.GetEnumerator();
try
{
Assert.Equal(Enumerator_Empty_UsesSingletonInstance, ReferenceEquals(enumerator1, enumerator2));
}
finally
{
if (enumerator2 is IDisposable d2) d2.Dispose();
}
}
finally
{
if (enumerator1 is IDisposable d1) d1.Dispose();
}
}

[Theory]
[MemberData(nameof(ValidCollectionSizes))]
public void IEnumerable_Generic_GetEnumerator_NoExceptionsWhileGetting(int count)
Expand Down Expand Up @@ -381,7 +419,7 @@ public void IEnumerable_Generic_Enumerator_MoveNext_ModifiedBeforeEnumeration_Th
{
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.MoveNext());
}
Expand Down Expand Up @@ -427,7 +465,7 @@ public void IEnumerable_Generic_Enumerator_MoveNext_ModifiedDuringEnumeration_Th
enumerator.MoveNext();
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.MoveNext());
}
Expand Down Expand Up @@ -471,7 +509,7 @@ public void IEnumerable_Generic_Enumerator_MoveNext_ModifiedAfterEnumeration_Thr
while (enumerator.MoveNext()) ;
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.MoveNext());
}
Expand Down Expand Up @@ -602,8 +640,7 @@ public void IEnumerable_Generic_Enumerator_Current_BeforeFirstMoveNext_Undefined
IEnumerable<T> enumerable = GenericIEnumerableFactory(count);
using (IEnumerator<T> enumerator = enumerable.GetEnumerator())
{
if (Enumerator_Current_UndefinedOperation_Throws ||
(count == 0 && Enumerator_Empty_Current_UndefinedOperation_Throws))
if (count == 0 ? Enumerator_Empty_Current_UndefinedOperation_Throws : Enumerator_Current_UndefinedOperation_Throws)
Assert.Throws<InvalidOperationException>(() => enumerator.Current);
else
current = enumerator.Current;
Expand All @@ -619,8 +656,7 @@ public void IEnumerable_Generic_Enumerator_Current_AfterEndOfEnumerable_Undefine
using (IEnumerator<T> enumerator = enumerable.GetEnumerator())
{
while (enumerator.MoveNext()) ;
if (Enumerator_Current_UndefinedOperation_Throws ||
(count == 0 && Enumerator_Empty_Current_UndefinedOperation_Throws))
if (count == 0 ? Enumerator_Empty_Current_UndefinedOperation_Throws : Enumerator_Current_UndefinedOperation_Throws)
Assert.Throws<InvalidOperationException>(() => enumerator.Current);
else
current = enumerator.Current;
Expand All @@ -639,7 +675,7 @@ public void IEnumerable_Generic_Enumerator_Current_ModifiedDuringEnumeration_Und
{
if (ModifyEnumerable(enumerable))
{
if (Enumerator_Current_UndefinedOperation_Throws)
if (count == 0 ? Enumerator_Empty_Current_UndefinedOperation_Throws : Enumerator_Current_UndefinedOperation_Throws)
Assert.Throws<InvalidOperationException>(() => enumerator.Current);
else
current = enumerator.Current;
Expand Down Expand Up @@ -694,7 +730,7 @@ public void IEnumerable_Generic_Enumerator_Reset_ModifiedBeforeEnumeration_Throw
{
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}
Expand Down Expand Up @@ -737,7 +773,7 @@ public void IEnumerable_Generic_Enumerator_Reset_ModifiedDuringEnumeration_Throw
enumerator.MoveNext();
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}
Expand Down Expand Up @@ -781,7 +817,7 @@ public void IEnumerable_Generic_Enumerator_Reset_ModifiedAfterEnumeration_Throws
while (enumerator.MoveNext()) ;
if (ModifyEnumerable(enumerable))
{
if (Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
if (count == 0 ? Enumerator_Empty_ModifiedDuringEnumeration_ThrowsInvalidOperationException : Enumerator_ModifiedDuringEnumeration_ThrowsInvalidOperationException)
{
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}
Expand Down