Add MemoryExtensions overloads with comparer

[stephentoub]

Closes #28934

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[Copilot]

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

Files not reviewed (9)

• src/libraries/System.Memory/tests/System.Memory.Tests.csproj: Language not supported
• src/libraries/System.Memory/tests/ReadOnlySpan/IndexOfSequence.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/LastIndexOfSequence.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/EndsWith.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/SequenceCompareTo.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/LastIndexOf.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/IndexOf.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/StartsWith.T.cs: Evaluated as low risk
• src/libraries/System.Memory/ref/System.Memory.cs: Evaluated as low risk

Comments suppressed due to low confidence (4)

src/libraries/System.Linq/src/System/Linq/Contains.cs:21

• The new overload for Contains using a comparer should have corresponding unit tests to ensure it behaves correctly with different comparers.

if (source.TryGetSpan(out ReadOnlySpan<TSource> span))

src/libraries/System.Linq/src/System/Linq/Contains.cs:32

• The check for value types should be removed as it is redundant. The logic for handling null comparers should be consistent regardless of whether TSource is a value type or not.

if (typeof(TSource).IsValueType)

src/libraries/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceUtils.cs:22

• The Contains method used here does not specify a comparer, which might lead to incorrect behavior if the default equality comparer is not appropriate for the type of elements in supportedAttributes.

if (supportedAttributes is null || !supportedAttributes.Contains(key))

src/libraries/System.Memory/tests/ReadOnlySpan/Contains.T.cs:124

• The test should include a case where the comparer is null to ensure the method handles it correctly.

Assert.All(GetDefaultEqualityComparers<string>(), comparer => Assert.False(new ReadOnlySpan<string>(Array.Empty<string>()).Contains("a", comparer)));

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

[Copilot]

Copilot reviewed 8 out of 17 changed files in this pull request and generated 1 comment.

Files not reviewed (9)

• src/libraries/System.Memory/tests/System.Memory.Tests.csproj: Language not supported
• src/libraries/System.Linq/src/System/Linq/Contains.cs: Evaluated as low risk
• src/libraries/System.Memory/ref/System.Memory.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/Comparers.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/Contains.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/Count.T.cs: Evaluated as low risk
• src/libraries/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceUtils.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/IndexOfSequence.T.cs: Evaluated as low risk
• src/libraries/System.Memory/tests/ReadOnlySpan/LastIndexOfSequence.T.cs: Evaluated as low risk

Comments suppressed due to low confidence (1)

src/libraries/System.Memory/tests/ReadOnlySpan/SequenceCompareTo.T.cs:140

• The method 'GetDefaultComparers' is not defined in the provided code. Ensure that 'GetDefaultComparers' is defined and correctly implemented to avoid compilation errors.

Assert.All(GetDefaultComparers<string>(), comparer => Assert.Equal(0, new ReadOnlySpan<string>(a, 1, 0).SequenceCompareTo<string>(new ReadOnlySpan<string>(a, 2, 0), comparer)));

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more