From cce4d0bb0351c035df2ee9e9da2f6a98a5b88eac Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 11 Nov 2024 15:09:36 +0100 Subject: [PATCH 1/4] Prefer array to ROS over Span --- .../OverloadResolution/OverloadResolution.cs | 13 ++++ .../CSharp/Test/Emit3/FirstClassSpanTests.cs | 62 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index 48d5f0de526f6..6377e4a018359 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -2972,6 +2972,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; diff --git a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs index c97ea2eba9aca..aff531326ef8b 100644 --- a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs @@ -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(IEnumerable e) => Console.Write(1); + public static void R(ReadOnlySpan s) => Console.Write(2); + public static void R(Span 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() { @@ -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(); + + static class E + { + public static void R(this IEnumerable e) => Console.Write(1); + public static void R(this ReadOnlySpan s) => Console.Write(2); + public static void R(this Span 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() { From 745ca79128ea1035736478b338319df34e9cf394 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 11 Nov 2024 15:11:57 +0100 Subject: [PATCH 2/4] Update tests --- .../CSharp/Test/Emit3/FirstClassSpanTests.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs index aff531326ef8b..834c8441981e7 100644 --- a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs @@ -7919,8 +7919,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; @@ -7935,8 +7935,16 @@ static class C public static void M(ReadOnlySpan 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] @@ -7960,7 +7968,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(); @@ -8020,7 +8028,7 @@ static class C // (new int[0]).E(); Diagnostic(ErrorCode.ERR_BadInstanceArgType, "new int[0]").WithArguments("int[]", "E", "C.E(System.Span)", "System.Span").WithLocation(3, 2)); - var expectedOutput = "1"; + var expectedOutput = "2"; var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext); CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics(); @@ -8071,7 +8079,7 @@ static class C // (new string[0]).E(); Diagnostic(ErrorCode.ERR_BadInstanceArgType, "new object[0]").WithArguments("object[]", "E", "C.E(System.Span)", "System.Span").WithLocation(4, 2)); - var expectedOutput = "21"; + var expectedOutput = "22"; var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.RegularNext); CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics(); From 946ec84cd0065b74cab8f77d725711a489c0dd02 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 11 Nov 2024 15:23:28 +0100 Subject: [PATCH 3/4] Add more tests --- .../CSharp/Test/Emit3/FirstClassSpanTests.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs index 834c8441981e7..854475e898c4f 100644 --- a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs @@ -8009,6 +8009,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 arg) => Console.Write(1); + public static void M1(ReadOnlySpan arg) => Console.Write(2); + + public static void M2(Span arg) => Console.Write(1); + public static void M2(ReadOnlySpan arg) => Console.Write(2); + } + """; + var comp = CreateCompilationWithSpanAndMemoryExtensions(source); + CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics(); + } + [Fact] public void OverloadResolution_SpanVsReadOnlySpan_ExtensionMethodReceiver_01() { @@ -8120,6 +8145,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 arg) => Console.Write(1); + public static void M1(this ReadOnlySpan arg) => Console.Write(2); + + public static void M2(this Span arg) => Console.Write(1); + public static void M2(this ReadOnlySpan arg) => Console.Write(2); + } + """; + var comp = CreateCompilationWithSpanAndMemoryExtensions(source); + CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics(); + } + [Fact] public void OverloadResolution_ReadOnlySpanVsReadOnlySpan() { From cdeb600d4ee142e040dfdb236a02ccf5212709ff Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 12 Nov 2024 11:12:41 +0100 Subject: [PATCH 4/4] Test ambiguity --- .../CSharp/Test/Emit3/FirstClassSpanTests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs index 854475e898c4f..0bf1e55bfbc15 100644 --- a/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs @@ -693,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 a, ReadOnlySpan b) => Console.Write(1); + public static void M(Span a, Span 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, ReadOnlySpan)' and 'C.M(Span, Span)' + // C.M(x, x); + Diagnostic(ErrorCode.ERR_AmbigCall, "M").WithArguments("C.M(System.Collections.Generic.IEnumerable, System.ReadOnlySpan)", "C.M(System.Span, System.Span)").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,