[API Proposal]: Remove where T : IEquatable<T>? constraint from MemoryExtensions methods

[stephentoub]

We should consider removing this constraint. It inhibits the use of almost all of the methods for Ts that would otherwise functionally work fine via EqualityComparer<T>.Default.Equals and adds relatively little benefit. Notably SequenceEqual and CommonPrefixLength don't have the constraint, but most everything else on MemoryExtensions does.

[ghost]

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

Issue Details

---

We should consider removing this constraint. It inhibits the use of almost all of the methods for Ts that would otherwise functionally work fine via EqualityComparer<T>.Default.Equals and adds relatively little benefit.

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Memory
Milestone:	8.0.0

[jkotas]

SequenceEqual and CommonPrefixLength don't have the constrain

SequenceEqual has the constrain: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs#L966

[stephentoub]

SequenceEqual has the constrain:

That overload does. We added this overload (which optionally accepts a comparer) which does not:

runtime/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs

Line 1696 in fa42038

public static bool SequenceEqual<T>(this Span<T> span, ReadOnlySpan<T> other, IEqualityComparer<T>? comparer = null) =>

[stephentoub]

Also note in the API review for that overload:
#48304 (comment)
"We could drop the constraint on the existing APIs and do the right thing internally, but we can do that later"

As an alternative to just dropping the constraint (or in addition to), we could also add new overloads of all the relevant methods, similarly without a constraint and similarly accepting an optional comparer.

[jkotas]

We have introduced the IEquatable<T> constraint on these methods to avoid paying for EqualityComparer<T> overhead. The JIT can optimize the EqualityComparer<T>.Default pretty well for (non-generic) structs today, but these optimizations still do not kick in for shared generics. Dropping the IEquatable<T> constraint would likely introduce regressions for cases where T is a reference type.

[stephentoub]

I'd be fine with the alternative if we believe we won't be able to solve those T==reference type overheads:
"As an alternative to just dropping the constraint (or in addition to), we could also add new overloads of all the relevant methods, similarly without a constraint and similarly accepting an optional comparer."

It's more overloads / more implementation (assuming we want to keep the separate implementations to avoid the shared generics overhead in the existing methods), but it's also more flexible, enabling the algorithms to be reused even with alternate comparison logic, and it wouldn't penalize existing usage, just enable usage not currently possible.

[stephentoub]

Just bumped into this again today. I wanted the equivalent of IndexOfAnyExcept(null) for an array of reference types. We don't appear to have anything to do that efficiently right now without using unsafe code that depends on runtime internal implementation details.

[tannergooding]

Would this potentially be a reason to push harder on bridged generic constraints so that users aren't completely blocked or do we think its better to just expose both sets of overloads?

[stephentoub]

Would this potentially be a reason to push harder on bridged generic constraints so that users aren't completely blocked or do we think its better to just expose both sets of overloads?

That wouldn't really help with this particular case, since the target legitimately doesn't implement the constraint. I think we need both features.

[tannergooding]

Seems like the best option would be to add the overloads then, given the concern around impact to shared generics (and runtimes like Mono where shared generics are all they really have)