Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 4 additions & 0 deletions src/libraries/System.Memory/ref/System.Memory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ public static partial class MemoryExtensions
public static int BinarySearch<T, TComparer>(this System.Span<T> span, T value, TComparer comparer) where TComparer : System.Collections.Generic.IComparer<T>, allows ref struct { throw null; }
[System.Runtime.CompilerServices.OverloadResolutionPriorityAttribute(-1)]
public static int BinarySearch<T, TComparable>(this System.Span<T> span, TComparable comparable) where TComparable : System.IComparable<T>, allows ref struct { throw null; }
public static T? Max<T>(this System.ReadOnlySpan<T> span) { throw null; }
public static T? Max<T>(this System.ReadOnlySpan<T> span, System.Collections.Generic.IComparer<T> comparer) { throw null; }
public static T? Min<T>(this System.ReadOnlySpan<T> span) { throw null; }
public static T? Min<T>(this System.ReadOnlySpan<T> span, System.Collections.Generic.IComparer<T> comparer) { throw null; }
[System.Runtime.CompilerServices.OverloadResolutionPriorityAttribute(-1)]
Comment thread
manandre marked this conversation as resolved.
Outdated
Comment thread
manandre marked this conversation as resolved.
Outdated
public static int CommonPrefixLength<T>(this System.Span<T> span, System.ReadOnlySpan<T> other) { throw null; }
[System.Runtime.CompilerServices.OverloadResolutionPriorityAttribute(-1)]
Expand Down
86 changes: 86 additions & 0 deletions src/libraries/System.Memory/tests/ReadOnlySpan/MinMax.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using Xunit;

namespace System.SpanTests
{
public static partial class ReadOnlySpanTests
{
[Fact]
public static void MinMax_Empty_NonNullableValueType_Throws()
{
ReadOnlySpan<int> span = ReadOnlySpan<int>.Empty;

TestHelpers.AssertThrows<InvalidOperationException, int>(span, (_span) => _span.Min());
Comment thread
manandre marked this conversation as resolved.
Outdated
TestHelpers.AssertThrows<InvalidOperationException, int>(span, (_span) => _span.Max());
Comment thread
manandre marked this conversation as resolved.
Outdated
TestHelpers.AssertThrows<InvalidOperationException, int>(span, (_span) => _span.Min(Comparer<int>.Default));
TestHelpers.AssertThrows<InvalidOperationException, int>(span, (_span) => _span.Max(Comparer<int>.Default));
}

[Fact]
public static void MinMax_Empty_ReferenceAndNullableValueType_ReturnsNull()
{
ReadOnlySpan<string?> strings = ReadOnlySpan<string?>.Empty;
ReadOnlySpan<int?> nullableInts = ReadOnlySpan<int?>.Empty;

Assert.Null(strings.Min());
Assert.Null(strings.Max());
Assert.Null(nullableInts.Min());
Assert.Null(nullableInts.Max());

Assert.Null(strings.Min(comparer: null!));
Assert.Null(strings.Max(comparer: null!));
Assert.Null(nullableInts.Min(comparer: null!));
Assert.Null(nullableInts.Max(comparer: null!));
}
Comment thread
manandre marked this conversation as resolved.

[Fact]
public static void MinMax_AllNull_ReturnsNull()
{
ReadOnlySpan<string?> strings = new string?[] { null, null, null };
ReadOnlySpan<int?> nullableInts = new int?[] { null, null, null };

Assert.Null(strings.Min());
Assert.Null(strings.Max());
Assert.Null(nullableInts.Min());
Assert.Null(nullableInts.Max());
}

[Fact]
public static void MinMax_NullNotFirst_NullIgnoredForComparison()
{
ReadOnlySpan<string?> strings = new string?[] { "charlie", null, "bravo", null, "delta" };
ReadOnlySpan<int?> nullableInts = new int?[] { 4, null, -1, null, 7 };

Assert.Equal("bravo", strings.Min());
Assert.Equal("delta", strings.Max());
Assert.Equal(-1, nullableInts.Min());
Assert.Equal(7, nullableInts.Max());
}

[Fact]
public static void MinMax_DefaultComparer_ProducesExpectedValues()
Comment thread
manandre marked this conversation as resolved.
{
ReadOnlySpan<int> ints = new int[] { 4, -1, 7, 2 };
ReadOnlySpan<string?> strings = new string?[] { null, "charlie", "bravo", null, "delta" };

Assert.Equal(-1, ints.Min());
Assert.Equal(7, ints.Max());

Assert.Equal("bravo", strings.Min());
Assert.Equal("delta", strings.Max());
}

[Fact]
public static void MinMax_CustomComparer_IsUsed()
{
ReadOnlySpan<int> ints = new int[] { 4, -1, 7, 2 };
IComparer<int> reverse = Comparer<int>.Create((left, right) => right.CompareTo(left));
Comment thread
manandre marked this conversation as resolved.

Assert.Equal(7, ints.Min(reverse));
Assert.Equal(-1, ints.Max(reverse));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<Compile Include="ParsersAndFormatters\Formatter\RealFormatterTests.cs" />
<Compile Include="$(CommonPath)..\tests\System\RealParserTestsBase.cs" Link="ParsersAndFormatters\Parser\RealParserTestsBase.cs" />
<Compile Include="ParsersAndFormatters\Parser\RealParserTests.cs" />
<Compile Include="ReadOnlySpan\MinMax.cs" />
<Compile Include="ReadOnlySpan\Comparers.cs" />
<Compile Include="ReadOnlySpan\Contains.byte.cs" />
<Compile Include="ReadOnlySpan\Contains.T.cs" />
Expand Down
126 changes: 126 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4312,6 +4312,132 @@ public static int BinarySearch<T, TComparer>(
return BinarySearch(span, comparable);
}

/// <summary>
/// Returns the minimum value in the span.
/// </summary>
/// <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>
/// <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>
Comment thread
manandre marked this conversation as resolved.
Comment thread
manandre marked this conversation as resolved.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static T? Min<T>(this ReadOnlySpan<T> span) =>
Min(span, comparer: null!);

Comment thread
tannergooding marked this conversation as resolved.
Comment thread
manandre marked this conversation as resolved.
Comment thread
manandre marked this conversation as resolved.
/// <summary>
/// Returns the minimum value in the span.
/// </summary>
/// <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>
public static T? Min<T>(this ReadOnlySpan<T> span, IComparer<T> comparer)
Comment thread
manandre marked this conversation as resolved.
Outdated
Comment thread
manandre marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ++

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have aligned the implementation on the enumerable logic.

{
if (span.IsEmpty)
{
if (default(T) is null)
{
return default;
}

ThrowHelper.ThrowInvalidOperationException();
Comment thread
manandre marked this conversation as resolved.
Outdated
}

return comparer is null || comparer == Comparer<T>.Default
? MinCore(span, Comparer<T>.Default)
: MinCore(span, comparer);

static T? MinCore<TComparer>(ReadOnlySpan<T> span, TComparer comparer)
where TComparer : IComparer<T>
Comment thread
manandre marked this conversation as resolved.
Outdated
{
T? value = span[0];
int i = 1;

while (value is null)
{
if ((uint)i >= (uint)span.Length)
{
return value;
}
value = span[i++];
}

for (; (uint)i < (uint)span.Length; i++)
{
T next = span[i];
if (next is not null && comparer.Compare(next, value) < 0)
Comment thread
manandre marked this conversation as resolved.
Outdated
{
value = next;
}
}

return value;
}
}

/// <summary>
/// Returns the maximum value in the span.
/// </summary>
/// <typeparam name="T">The type of the elements in the span.</typeparam>
/// <param name="span">The span of values to determine the maximum value of.</param>
/// <returns>The maximum value in the span.</returns>
/// <exception cref="InvalidOperationException"><paramref name="span"/> is empty and <typeparamref name="T"/> is a non-nullable value type.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static T? Max<T>(this ReadOnlySpan<T> span) =>
Max(span, comparer: null!);

/// <summary>
/// Returns the maximum value in the span.
/// </summary>
/// <typeparam name="T">The type of the elements in the span.</typeparam>
/// <param name="span">The span of values to determine the maximum value of.</param>
/// <param name="comparer">The <see cref="IComparer{T}"/> to compare values.</param>
/// <returns>The maximum value in the span.</returns>
/// <exception cref="InvalidOperationException"><paramref name="span"/> is empty and <typeparamref name="T"/> is a non-nullable value type.</exception>
public static T? Max<T>(this ReadOnlySpan<T> span, IComparer<T> comparer)
{
if (span.IsEmpty)
{
if (default(T) is null)
{
return default;
}

ThrowHelper.ThrowInvalidOperationException();
}

return comparer is null || comparer == Comparer<T>.Default
? MaxCore(span, Comparer<T>.Default)
: MaxCore(span, comparer);

static T? MaxCore<TComparer>(ReadOnlySpan<T> span, TComparer comparer)
where TComparer : IComparer<T>
{
T? value = span[0];
int i = 1;

while (value is null)
{
if ((uint)i >= (uint)span.Length)
{
return value;
}
value = span[i++];
}

for (; (uint)i < (uint)span.Length; i++)
{
T next = span[i];
if (next is not null && comparer.Compare(next, value) > 0)
{
value = next;
}
}

return value;
}
}
Comment thread
manandre marked this conversation as resolved.

/// <summary>
/// Sorts the elements in the entire <see cref="Span{T}" /> using the <see cref="IComparable{T}" /> implementation
/// of each element of the <see cref= "Span{T}" />
Expand Down
Loading