Skip to content

Commit

Permalink
Prefer array to ReadOnlySpan over Span (#75853)
Browse files Browse the repository at this point in the history 
* Prefer array to ROS over Span

* Update tests

* Add more tests

* Test ambiguity
  • Loading branch information
@jjonescz
jjonescz authored Nov 21, 2024
1 parent 94fff7a commit 50a5a2f
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 6 deletions.
13 changes: 13 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2980,6 +2980,19 @@ private BetterResult BetterConversionFromExpression(BoundExpression node, TypeSy
switch ((conv1.Kind, conv2.Kind))
{
case (ConversionKind.ImplicitSpan, ConversionKind.ImplicitSpan):
// If the expression is of an array type, prefer ReadOnlySpan over Span (to avoid ArrayTypeMismatchExceptions).
if (node.Type is ArrayTypeSymbol)
{
if (t1.IsReadOnlySpan() && t2.IsSpan())
{
return BetterResult.Left;
}

if (t1.IsSpan() && t2.IsReadOnlySpan())
{
return BetterResult.Right;
}
}
break;
case (_, ConversionKind.ImplicitSpan):
return BetterResult.Right;
Expand Down
162 changes: 156 additions & 6 deletions src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,37 @@ static class C
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
public void BreakingChange_TypeInference_SpanVsIEnumerable_02_BothSpanAndReadOnlySpan()
{
var source = """
using System;
using System.Collections.Generic;
string[] s = new[] { "a" };
object[] o = s;
C.R(o);
static class C
{
public static void R<T>(IEnumerable<T> e) => Console.Write(1);
public static void R<T>(ReadOnlySpan<T> s) => Console.Write(2);
public static void R<T>(Span<T> s) => Console.Write(3);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "1").VerifyDiagnostics();

var expectedOutput = "2";

comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
public void BreakingChange_Conversion_SpanVsIEnumerable()
{
Expand Down Expand Up @@ -518,6 +549,37 @@ static class E
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
public void BreakingChange_Conversion_SpanVsIEnumerable_BothSpanAndReadOnlySpan()
{
var source = """
using System;
using System.Collections.Generic;
string[] s = new[] { "a" };
object[] o = s;
o.R<object>();
static class E
{
public static void R<T>(this IEnumerable<T> e) => Console.Write(1);
public static void R<T>(this ReadOnlySpan<T> s) => Console.Write(2);
public static void R<T>(this Span<T> s) => Console.Write(3);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "1").VerifyDiagnostics();

var expectedOutput = "2";

comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
public void BreakingChange_Conversion_SpanVsArray()
{
Expand Down Expand Up @@ -631,6 +693,36 @@ static class C
CreateCompilationWithSpanAndMemoryExtensions(source).VerifyDiagnostics(expectedDiagnostics);
}

[Fact]
public void BreakingChange_OverloadResolution_Betterness_01()
{
var source = """
using System;
using System.Collections.Generic;
var x = new int[0];
C.M(x, x);
static class C
{
public static void M(IEnumerable<int> a, ReadOnlySpan<int> b) => Console.Write(1);
public static void M(Span<int> a, Span<int> b) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "2").VerifyDiagnostics();

var expectedDiagnostics = new[]
{
// (5,3): error CS0121: The call is ambiguous between the following methods or properties: 'C.M(IEnumerable<int>, ReadOnlySpan<int>)' and 'C.M(Span<int>, Span<int>)'
// C.M(x, x);
Diagnostic(ErrorCode.ERR_AmbigCall, "M").WithArguments("C.M(System.Collections.Generic.IEnumerable<int>, System.ReadOnlySpan<int>)", "C.M(System.Span<int>, System.Span<int>)").WithLocation(5, 3)
};

CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext).VerifyDiagnostics(expectedDiagnostics);
CreateCompilationWithSpanAndMemoryExtensions(source).VerifyDiagnostics(expectedDiagnostics);
}

[Theory, CombinatorialData]
public void Conversion_Array_Span_Implicit(
[CombinatorialLangVersions] LanguageVersion langVersion,
Expand Down Expand Up @@ -7857,8 +7949,8 @@ static class C
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_SpanVsReadOnlySpan_01(LanguageVersion langVersion)
[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_01()
{
var source = """
using System;
Expand All @@ -7873,8 +7965,16 @@ static class C
public static void M(ReadOnlySpan<int> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "112").VerifyDiagnostics();

var expectedOutput = "212";

comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
Expand All @@ -7898,7 +7998,7 @@ static class C
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: ExecutionConditionUtil.IsCoreClr ? "1123" : "1121").VerifyDiagnostics();

var expectedOutput = "1122";
var expectedOutput = "2122";

comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
Expand Down Expand Up @@ -7939,6 +8039,31 @@ static class C
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_04()
{
var source = """
using System;
C.M1(new object[0]);
C.M1(new string[0]);
C.M2(new object[0]);
C.M2(new string[0]);
static class C
{
public static void M1(Span<string> arg) => Console.Write(1);
public static void M1(ReadOnlySpan<object> arg) => Console.Write(2);
public static void M2(Span<object> arg) => Console.Write(1);
public static void M2(ReadOnlySpan<string> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics();
}

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_ExtensionMethodReceiver_01()
{
Expand All @@ -7958,7 +8083,7 @@ static class C
// (new int[0]).E();
Diagnostic(ErrorCode.ERR_BadInstanceArgType, "new int[0]").WithArguments("int[]", "E", "C.E(System.Span<int>)", "System.Span<int>").WithLocation(3, 2));

var expectedOutput = "1";
var expectedOutput = "2";

var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
Expand Down Expand Up @@ -8009,7 +8134,7 @@ static class C
// (new string[0]).E();
Diagnostic(ErrorCode.ERR_BadInstanceArgType, "new object[0]").WithArguments("object[]", "E", "C.E(System.Span<object>)", "System.Span<object>").WithLocation(4, 2));

var expectedOutput = "21";
var expectedOutput = "22";

var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
Expand Down Expand Up @@ -8050,6 +8175,31 @@ static class C
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_ExtensionMethodReceiver_05()
{
var source = """
using System;
(new object[0]).M1();
(new string[0]).M1();
(new object[0]).M2();
(new string[0]).M2();
static class E
{
public static void M1(this Span<string> arg) => Console.Write(1);
public static void M1(this ReadOnlySpan<object> arg) => Console.Write(2);
public static void M2(this Span<object> arg) => Console.Write(1);
public static void M2(this ReadOnlySpan<string> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics();
}

[Fact]
public void OverloadResolution_ReadOnlySpanVsReadOnlySpan()
{
Expand Down

0 comments on commit 50a5a2f

Please sign in to comment.