Upcoming C# 14 breaking change: first-class spans

[333fred]

Hey xUnit folks, your friendly neighborhood C# LDM member here. I wanted to let you know that a feature we're planning for C# 14, first-class spans, is going to introduce a new source-breaking change for Assert.Equals in a particular scenario. It's already partly broken with reference types, but will break with value types as well. Here's the
samples:

Ambiguous today

string[] str = ["hello"];
Assert.Equal(["hello"], str); // error CS0121: The call is ambiguous between the following methods or properties: 'Assert.Equal<T>(T[], T[])' and 'Assert.Equal<T>(ReadOnlySpan<T>, Span<T>)'

// This one is more of an edge case, but is also an error
long[] l = [1];
Assert.Equal<long>([2], l); // error CS0121: The call is ambiguous between the following methods or properties: 'Assert.Equal<T>(T[], T[])' and 'Assert.Equal<T>(ReadOnlySpan<T>, Span<T>)' 

Ambiguous in C# 14

var x = new long[] { 1 };
Assert.Equal([2], x); // previously Assert.Equal<T>(T[], T[]), now ambiguous with Assert.Equal<T>(ReadOnlySpan<T>, Span<T>)

---

We've documented these ambiguities as part of the feature specification here, and discussed them during this LDM. To sum up, we see a couple of options available for fixing these potential ambiguities:

• Apply OverloadResolutionPriority(-1) to the array-based overloads, so everything goes through ReadOnlySpan.
• Expose an Assert.Equals<T>(ReadOnlySpan<T> expected, T[] actual), which will be preferred for Assert.Equals([1, 2], somethingThatIsAnArray).

Just wanted to give you a heads up so you could look at this now, rather than after C# 14 releases.

[bradwilson]

Thanks!

[bradwilson]

I opted to add the extra overload, because polyfilling OverloadResolutionPriority would still require a fairly new compiler to be effective.

Available in v2 2.9.1-pre.11
Available in v3 0.4.0-pre.3

https://xunit.net/docs/using-ci-builds

[sebastienros]

I am seeing build errors in aspnetcore even with the new version (2.9.2). Some of these appear when comparing string and StringSegment (https://source.dot.net/#Microsoft.Extensions.Primitives/StringSegment.cs). Do you think this could be a missing overload due to the C# change?

[bradwilson]

(ReadOnlySpan<T>, T[] is the new overload that was added which was supposed to resolve the issue)

[bradwilson]

This is an offending line: https://github.com/dotnet/aspnetcore/blob/b647ece5cbfccd21d57342bfab61480621ef0379/src/Http/Http/test/Features/QueryFeatureTests.cs#L116

[sebastienros]

I am fixing the issues but I have actually seen several different usages failing so far. The main one was <T, T> vs <ReadOnlySpan<T>, ReadOnlySpan<T> when comparing string and StringSegment.

I am not blocked but wanted to let you know that it's more than just what was added.

[333fred]

Unfortunately I'm not sure there's much xUnit could do wrt this type of break. It's a bit hard to reconstruct what's going on here just from looking at the code, but let me know if I've got this right:

StringSegment actual = ...;
string expected = ...;

Assert.Equal(expected, actual); // Ambiguous between Assert.Equal<StringSegment>(StringSegment, StringSegment) and Assert.Equal<char>(ReadOnlySpan<char>, ReadOnlySpan<char>)

If that's the scenario, what's going on here is that each parameter has a preferred overload: the StringSegment overload prefers T, since that's an identity conversion over the user-defined conversion to ReadOnlySpan<char>, and the string parameter prefers the ReadOnlySpan<char> conversion, since that's a built-in conversion over the user-defined conversion to StringSegment. These are then ambiguous, since neither is better than the other. @jjonescz and @jaredpar for thoughts as well, but I'm not really sure there's anything to do other than fix up the locations that the breaks occurred, since this is the type of breaking change we accepted with the first-class spans feature in the first place.

[bradwilson]

I think what we've all learned today is implicit conversion operators are evil. 😂