Add MemoryExtensions.Min/Max by manandre · Pull Request #128306 · dotnet/runtime

manandre · 2026-05-17T21:55:23Z

This pull request adds new Min and Max extension methods for ReadOnlySpan<T> to the System.Memory library, allowing users to efficiently find the minimum and maximum values in a span, optionally using a custom comparer. Comprehensive unit tests are included to verify correct behavior for various data types and edge cases.

New `Min` and `Max` methods for `ReadOnlySpan<T>`:

Added Min<T> and Max<T> extension methods to MemoryExtensions for ReadOnlySpan<T>, supporting both default and custom comparers, and handling nullable and reference types gracefully. These methods throw an exception on empty spans of non-nullable value types and return null for empty spans of reference or nullable types. [1] [2]

Unit tests:

Introduced comprehensive tests in ReadOnlySpan/MinMax.cs to cover empty spans, all-null values, custom comparers, and typical scenarios for both value and reference types.
Registered the new test file in the project file System.Memory.Tests.csproj to ensure the tests are run as part of the test suite.

Fixes #127083

dotnet-policy-service · 2026-05-17T21:56:31Z

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Min and Max extension methods on ReadOnlySpan<T> (with optional IComparer<T>), mirroring LINQ's Enumerable.Min/Max semantics for spans. Includes ref assembly updates and tests.

Changes:

New Min/Max ReadOnlySpan extension methods with empty-span and null-element handling.
Reference assembly exposes the four new API overloads.
New test file covering empty, all-null, mixed-null, default comparer, and custom comparer scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs	Implements `Min`/`Max` with a generic comparer fast path and null-skipping inner core.
src/libraries/System.Memory/ref/System.Memory.cs	Adds public surface for the four new overloads.
src/libraries/System.Memory/tests/System.Memory.Tests.csproj	Includes the new test file.
src/libraries/System.Memory/tests/ReadOnlySpan/MinMax.cs	Tests for empty/null/normal/custom-comparer behaviors.

MihaZupan · 2026-05-18T13:33:17Z

Enumerable's Min/Max already have vectorized logic implementing this. We should move that logic into these new methods, then switch LINQ to call them instead

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

tannergooding · 2026-05-19T13:33:52Z

+        /// </remarks>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static T? Min<T>(this ReadOnlySpan<T> span) =>
+            Min(span, Comparer<T>.Default);


Much like with LINQ, this should be passing comparer: null and then we should be treating a null comparer as selecting the default, not throwing.

@tannergooding Should we then update the approved API to explicitly accept a null comparer?

public T? Min<T>(this ReadOnlySpan<T> span, IComparer<T>? comparer); public T? Max<T>(this ReadOnlySpan<T> span, IComparer<T>? comparer);

Nullability annotations are often overlooked in the API review. IComparer<T> parameters is a case that we almost universally allow to be nullable (much like IFormattable, IEqualityComparer, and a couple others) and where we often explicitly want it to be so we can do specific optimizations.

I've left an explicit comment indicating the oversight, so please adjust the PR accordingly.

tannergooding · 2026-05-19T13:42:33Z

+        /// <para>If <typeparamref name="T" /> is a reference type and the span sequence is empty, this method returns <see langword="null" />.</para>
+        /// <para>Null values are ignored when determining the minimum value. If the span contains at least one non-null value, the minimum of those values is returned. If the span does not contain any non-null values, <see langword="null" /> is returned.</para>
+        /// </remarks>
+        public static T? Min<T>(this ReadOnlySpan<T> span, IComparer<T> comparer)


It's unclear why this deviates from the enumerable logic in implementation.

I'd generally expect the two entry points to be nearly identical. Covering the acceleration and other checks like: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Linq/src/System/Linq/Min.cs#L292-L373

The biggest difference should be that if (!enumerator.MoveNext()) becomes if (i >= span.Length). The JIT should already know that span.Length can't be negative and should know the same of i since we initialize it to 0 and only ever do ++

I have aligned the implementation on the enumerable logic.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

+        /// <typeparam name="T">The type of the elements in the span.</typeparam>
+        /// <param name="span">The span of values to determine the minimum value of.</param>
+        /// <param name="comparer">The <see cref="IComparer{T}"/> to compare values.</param>
+        /// <returns>The minimum value in the span.</returns>
+        /// <exception cref="InvalidOperationException"><paramref name="span"/> is empty and <typeparamref name="T"/> is a non-nullable value type.</exception>
+        /// <remarks>
+        /// <para>If <typeparamref name="T" /> is a reference type and the span sequence is empty, this method returns <see langword="null" />.</para>
+        /// <para>Null values are ignored when determining the minimum value. If the span contains at least one non-null value, the minimum of those values is returned. If the span does not contain any non-null values, <see langword="null" /> is returned.</para>
+        /// </remarks>
+        public static T? Min<T>(this ReadOnlySpan<T> span, IComparer<T>? comparer)
+        {
+            comparer ??= Comparer<T>.Default;


MihaZupan

Please remove the duplicate logic in Linq and call these new helpers instead.

MihaZupan · 2026-05-19T22:51:34Z

+        /// <para>If <typeparamref name="T" /> is a reference type and the span sequence is empty, this method returns <see langword="null" />.</para>
+        /// <para>Null values are ignored when determining the minimum value. If the span contains at least one non-null value, the minimum of those values is returned. If the span does not contain any non-null values, <see langword="null" /> is returned.</para>
+        /// </remarks>
+        public static T? Min<T>(this ReadOnlySpan<T> span, IComparer<T>? comparer)


Please move all of these methods into a new MemoryExtensions.MinMax.cs file (see Trim as an example).

MihaZupan · 2026-05-19T22:52:37Z

+                ref Unsafe.As<TFrom, TTo>(ref MemoryMarshal.GetReference(span)),
+                span.Length);
+
+        private interface IMinMaxDirection


Can we reuse the other interface we're adding instead of having two sets?

MihaZupan · 2026-05-19T22:53:00Z

+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static ReadOnlySpan<TTo> Cast<TFrom, TTo>(ReadOnlySpan<TFrom> span)


You can delete this helper and call Unsafe.BitCast<T, ...>(span) instead

MihaZupan · 2026-05-19T22:53:33Z

+            }
+            else
+            {
+                if (i >= span.Length)


Suggested change

if (i >= span.Length)

if (span.IsEmpty)

Add MemoryExtensions.Min/Max

baff8f3

Copilot AI review requested due to automatic review settings May 17, 2026 21:55

github-actions Bot added the area-System.Memory label May 17, 2026

dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label May 17, 2026

Copilot AI reviewed May 17, 2026

View reviewed changes

This was referenced May 18, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

AF: *(_UNCHECKED_OBJECTREF *)handle == NULL (HndCreateHandle called by getJitHandleForObject) #117138

Open

[wasm] Tests failing with DirectoryNotFoundException trying to load test data #128293

Open

manandre added 2 commits May 18, 2026 21:05

Add specific exception message when span is empty

6ef9e87

Reject null comparer

a1023cc

Copilot AI review requested due to automatic review settings May 18, 2026 22:00

Copilot AI reviewed May 18, 2026

View reviewed changes

manandre added 2 commits May 18, 2026 22:43

Update documentation

d214de8

Remove cache file

f5107bd

Copilot AI review requested due to automatic review settings May 18, 2026 22:55

Copilot AI reviewed May 18, 2026

View reviewed changes

Apply Copilot suggestions

93c617a

tannergooding reviewed May 19, 2026

View reviewed changes

manandre added 3 commits May 19, 2026 22:02

Align implementation on enumerable logic

b4fb827

Add fast paths

8d32597

Add tests for integer types

69566d1

Copilot AI review requested due to automatic review settings May 19, 2026 21:49

Copilot AI reviewed May 19, 2026

View reviewed changes

manandre added 2 commits May 20, 2026 00:07

Add tests for NaN case

c2c8dfa

Small fixes

55b1cf7

Copilot AI review requested due to automatic review settings May 19, 2026 22:21

Copilot AI reviewed May 19, 2026

View reviewed changes

manandre added 2 commits May 20, 2026 00:35

Reduce code duplication

0acc339

Add test

9c71813

Copilot AI review requested due to automatic review settings May 19, 2026 22:41

Copilot AI reviewed May 19, 2026

View reviewed changes

MihaZupan reviewed May 19, 2026

View reviewed changes

Conversation

manandre commented May 17, 2026

New Min and Max methods for ReadOnlySpan<T>:

Unit tests:

Uh oh!

dotnet-policy-service Bot commented May 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MihaZupan commented May 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MihaZupan left a comment

Choose a reason for hiding this comment

New `Min` and `Max` methods for `ReadOnlySpan<T>`: