From f9f6da22637e187c7e30846bd2be003ecd0a8180 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 7 Mar 2024 20:59:27 -0500 Subject: [PATCH] Fix tiny regression in Enumerable.ElementAt This just moves a few ns around, mainly putting the check for IList before the check for Iterator, matching how it was previously. --- .../src/System/Linq/AppendPrepend.SpeedOpt.cs | 4 +- .../System.Linq/src/System/Linq/ElementAt.cs | 47 +++++++++++-------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Linq/src/System/Linq/AppendPrepend.SpeedOpt.cs b/src/libraries/System.Linq/src/System/Linq/AppendPrepend.SpeedOpt.cs index 8562c169d321a..01adb0af01e45 100644 --- a/src/libraries/System.Linq/src/System/Linq/AppendPrepend.SpeedOpt.cs +++ b/src/libraries/System.Linq/src/System/Linq/AppendPrepend.SpeedOpt.cs @@ -171,9 +171,7 @@ public override int GetCount(bool onlyIfCheap) } index--; - return - _source is Iterator iterator ? iterator.TryGetElementAt(index, out found) : - TryGetElementAtNonIterator(_source, index, out found); + return _source.TryGetElementAt(index, out found); } return base.TryGetElementAt(index, out found); diff --git a/src/libraries/System.Linq/src/System/Linq/ElementAt.cs b/src/libraries/System.Linq/src/System/Linq/ElementAt.cs index 0383a87114feb..97b87f9eba999 100644 --- a/src/libraries/System.Linq/src/System/Linq/ElementAt.cs +++ b/src/libraries/System.Linq/src/System/Linq/ElementAt.cs @@ -16,7 +16,18 @@ public static TSource ElementAt(this IEnumerable source, int i ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source); } - TSource? element = TryGetElementAt(source, index, out bool found, guardIListLength: false); + if (source is IList list) + { + return list[index]; + } + + bool found; + TSource? element = +#if !OPTIMIZE_FOR_SIZE + source is Iterator iterator ? iterator.TryGetElementAt(index, out found) : +#endif + TryGetElementAtNonIterator(source, index, out found); + if (!found) { ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index); @@ -102,31 +113,27 @@ public static TSource ElementAt(this IEnumerable source, Index return element; } - private static TSource? TryGetElementAt(this IEnumerable source, int index, out bool found, bool guardIListLength = true) => + private static TSource? TryGetElementAt(this IEnumerable source, int index, out bool found) + { + if (source is IList list) + { + return (found = (uint)index < (uint)list.Count) ? + list[index] : + default; + } + + return #if !OPTIMIZE_FOR_SIZE - source is Iterator iterator ? iterator.TryGetElementAt(index, out found) : + source is Iterator iterator ? iterator.TryGetElementAt(index, out found) : #endif - TryGetElementAtNonIterator(source, index, out found, guardIListLength); + TryGetElementAtNonIterator(source, index, out found); + } - private static TSource? TryGetElementAtNonIterator(IEnumerable source, int index, out bool found, bool guardIListLength = true) + private static TSource? TryGetElementAtNonIterator(IEnumerable source, int index, out bool found) { Debug.Assert(source is not null); - if (source is IList list) - { - // Historically, ElementAt would simply delegate to IList[int] without first checking the bounds. - // That in turn meant that whatever exception the IList[int] throws for out-of-bounds access would - // propagate, e.g. ImmutableArray throws IndexOutOfRangeException whereas List throws ArgumentOutOfRangeException. - // Other uses of this, though, do need to guard, such as ElementAtOrDefault and all the various - // internal TryGetElementAt helpers. So, we have a guardIListLength parameter to allow the caller - // to specify whether to guard or not. - if (!guardIListLength || (uint)index < (uint)list.Count) - { - found = true; - return list[index]; - } - } - else if (index >= 0) + if (index >= 0) { using IEnumerator e = source.GetEnumerator(); while (e.MoveNext())