Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer ReadOnlySpan over Span as better conversion target #76300

Merged
merged 5 commits into from
Jan 15, 2025
Merged

Prefer ReadOnlySpan over Span as better conversion target #76300

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43f97dc
Prefer ReadOnlySpan over Span as better conversion target
jjonescz Dec 6, 2024
c87a6c4
Test identity element conversions
jjonescz Dec 9, 2024
041e102
Exclude span types from the second rule
jjonescz Dec 10, 2024
734a34f
Handle Span vs non-span type
jjonescz Dec 11, 2024
ccd3f42
Extend tests
jjonescz Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 35 additions & 13 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2980,19 +2980,6 @@ 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 Expand Up @@ -3454,6 +3441,25 @@ private BetterResult BetterConversionTargetCore(
return BetterResult.Neither;
}

// SPEC: T₁ is System.ReadOnlySpan<E₁>, T₂ is System.Span<E₂>, and an identity conversion from E₁ to E₂ exists
if (Compilation.IsFeatureEnabled(MessageID.IDS_FeatureFirstClassSpan))
{
if (isBetterSpanConversionTarget(type1, type2))
{
return BetterResult.Left;
}
else if (isBetterSpanConversionTarget(type2, type1))
{
return BetterResult.Right;
}
// The next case (a type is a better target if an implicit conversion from it to the other type exists)
// does not apply to span conversions except the covariant ReadOnlySpan->ReadOnlySpan conversion.
else if (type1.IsSpan() || type2.IsSpan())
{
return BetterResult.Neither;
}
}

// Given two different types T1 and T2, T1 is a better conversion target than T2 if no implicit conversion from T2 to T1 exists,
// and at least one of the following holds:
bool type1ToType2 = Conversions.ClassifyImplicitConversionFromType(type1, type2, ref useSiteInfo).IsImplicit;
Expand Down Expand Up @@ -3592,6 +3598,22 @@ private BetterResult BetterConversionTargetCore(
}

return BetterResult.Neither;

static bool isBetterSpanConversionTarget(TypeSymbol type1, TypeSymbol type2)
{
// SPEC: T₁ is System.ReadOnlySpan<E₁>, T₂ is System.Span<E₂>, and an identity conversion from E₁ to E₂ exists
if (type1.IsReadOnlySpan() && type2.IsSpan())
{
var type1Element = ((NamedTypeSymbol)type1).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0].Type;
var type2Element = ((NamedTypeSymbol)type2).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0].Type;
if (Conversions.HasIdentityConversion(type1Element, type2Element))
{
return true;
}
}

return false;
}
}

private bool IsMethodGroupConversionIncompatibleWithDelegate(BoundMethodGroup node, NamedTypeSymbol delegateType, Conversion conv)
Expand Down
267 changes: 247 additions & 20 deletions src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8043,15 +8043,8 @@ static class C
[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_04()
{
var source = """
var source1 = """
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);
Expand All @@ -8061,8 +8054,235 @@ static class C
public static void M2(ReadOnlySpan<string> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics();

{
var source2 = """
C.M1(new object[0]);
C.M2(new object[0]);
""";

var expectedOutput = "21";

var comp = CreateCompilationWithSpanAndMemoryExtensions([source1, source2], parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

comp = CreateCompilationWithSpanAndMemoryExtensions([source1, source2], parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

comp = CreateCompilationWithSpanAndMemoryExtensions([source1, source2]);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

{
var source3 = """
C.M1(new string[0]);
""";

var expectedDiagnostics = new[]
{
// (1,3): error CS0121: The call is ambiguous between the following methods or properties: 'C.M1(Span<string>)' and 'C.M1(ReadOnlySpan<object>)'
// C.M1(new string[0]);
Diagnostic(ErrorCode.ERR_AmbigCall, "M1").WithArguments("C.M1(System.Span<string>)", "C.M1(System.ReadOnlySpan<object>)").WithLocation(1, 3)
};

CreateCompilationWithSpanAndMemoryExtensions([source1, source3],
parseOptions: TestOptions.Regular13).VerifyDiagnostics(expectedDiagnostics);
CreateCompilationWithSpanAndMemoryExtensions([source1, source3],
parseOptions: TestOptions.RegularNext).VerifyDiagnostics(expectedDiagnostics);
CreateCompilationWithSpanAndMemoryExtensions([source1, source3]).VerifyDiagnostics(expectedDiagnostics);
}

{
var source4 = """
C.M2(new string[0]);
""";

CreateCompilationWithSpanAndMemoryExtensions([source1, source4],
parseOptions: TestOptions.Regular13).VerifyDiagnostics(
// (1,3): error CS0121: The call is ambiguous between the following methods or properties: 'C.M2(Span<object>)' and 'C.M2(ReadOnlySpan<string>)'
// C.M2(new string[0]);
Diagnostic(ErrorCode.ERR_AmbigCall, "M2").WithArguments("C.M2(System.Span<object>)", "C.M2(System.ReadOnlySpan<string>)").WithLocation(1, 3));

var expectedOutput = "2";

var comp = CreateCompilationWithSpanAndMemoryExtensions([source1, source4], parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

comp = CreateCompilationWithSpanAndMemoryExtensions([source1, source4]);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}
}

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_05()
{
var source = """
using System;

C.M(new D());
C.M(new D());

static class C
{
public static void M(Span<int> arg) => Console.Write(1);
public static void M(ReadOnlySpan<int> arg) => Console.Write(2);
}

class D
{
public static implicit operator Span<int>(D d) => default;
public static implicit operator ReadOnlySpan<int>(D d) => default;
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "11", verify: Verification.FailsILVerify).VerifyDiagnostics();

var expectedOutput = "22";

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

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

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_06()
{
var source = """
using System;

C.M(new D());
C.M(new D());

static class C
{
public static void M(Span<string> arg) => Console.Write(1);
public static void M(ReadOnlySpan<object> arg) => Console.Write(2);
}

class D
{
public static implicit operator Span<object>(D d) => default;
public static implicit operator ReadOnlySpan<string>(D d) => default;
}
""";
CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13).VerifyDiagnostics(
// (3,5): error CS1503: Argument 1: cannot convert from 'D' to 'System.Span<string>'
// C.M(new D());
Diagnostic(ErrorCode.ERR_BadArgType, "new D()").WithArguments("1", "D", "System.Span<string>").WithLocation(3, 5),
// (4,5): error CS1503: Argument 1: cannot convert from 'D' to 'System.Span<string>'
// C.M(new D());
Diagnostic(ErrorCode.ERR_BadArgType, "new D()").WithArguments("1", "D", "System.Span<string>").WithLocation(4, 5));

var expectedDiagnostics = new[]
{
// (3,5): error CS0457: Ambiguous user defined conversions 'D.implicit operator Span<object>(D)' and 'D.implicit operator ReadOnlySpan<string>(D)' when converting from 'D' to 'ReadOnlySpan<object>'
// C.M(new D());
Diagnostic(ErrorCode.ERR_AmbigUDConv, "new D()").WithArguments("D.implicit operator System.Span<object>(D)", "D.implicit operator System.ReadOnlySpan<string>(D)", "D", "System.ReadOnlySpan<object>").WithLocation(3, 5),
// (4,5): error CS0457: Ambiguous user defined conversions 'D.implicit operator Span<object>(D)' and 'D.implicit operator ReadOnlySpan<string>(D)' when converting from 'D' to 'ReadOnlySpan<object>'
// C.M(new D());
Diagnostic(ErrorCode.ERR_AmbigUDConv, "new D()").WithArguments("D.implicit operator System.Span<object>(D)", "D.implicit operator System.ReadOnlySpan<string>(D)", "D", "System.ReadOnlySpan<object>").WithLocation(4, 5)
};

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

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_SpanVsReadOnlySpan_07(LanguageVersion langVersion)
{
var source = """
using System;

C.M(new D());

static class C
{
public static void M(Span<string> arg) => Console.Write(1);
public static void M(ReadOnlySpan<object> arg) => Console.Write(2);
}

class D
{
public static implicit operator Span<string>(D d) => default;
public static implicit operator ReadOnlySpan<object>(D d) => default;
}
""";
CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion)).VerifyDiagnostics(
// (3,3): error CS0121: The call is ambiguous between the following methods or properties: 'C.M(Span<string>)' and 'C.M(ReadOnlySpan<object>)'
// C.M(new D());
Diagnostic(ErrorCode.ERR_AmbigCall, "M").WithArguments("C.M(System.Span<string>)", "C.M(System.ReadOnlySpan<object>)").WithLocation(3, 3));
}

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_08()
{
var source = """
using System;

(int, int)[] t = [(1, 2)];

C.M1(t);
C.M1(t);

C.M2(t);
C.M2(t);

static class C
{
public static void M1(Span<(int X, int Y)> arg) => Console.Write(1);
public static void M1(ReadOnlySpan<(int, int)> arg) => Console.Write(2);

public static void M2(Span<(int X, int Y)> arg) => Console.Write(1);
public static void M2(ReadOnlySpan<(int A, int B)> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "1111").VerifyDiagnostics();

var expectedOutput = "2222";

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

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

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_09()
{
var source = """
using System;

object[] a = [];

C.M1(a);
C.M1(a);

C.M2(a);
C.M2(a);

static class C
{
public static void M1(Span<object> arg) => Console.Write(1);
public static void M1(ReadOnlySpan<dynamic> arg) => Console.Write(2);

public static void M2(Span<dynamic> arg) => Console.Write(1);
public static void M2(ReadOnlySpan<object> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13);
CompileAndVerify(comp, expectedOutput: "1111").VerifyDiagnostics();

var expectedOutput = "2222";

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

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

[Fact]
Expand Down Expand Up @@ -8179,15 +8399,8 @@ static class C
[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_ExtensionMethodReceiver_05()
{
var source = """
var source1 = """
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);
Expand All @@ -8197,8 +8410,22 @@ static class E
public static void M2(this ReadOnlySpan<string> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics();

var source2 = """
(new object[0]).M1();
(new object[0]).M2();
(new string[0]).M2();
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions([source1, source2]);
CompileAndVerify(comp, expectedOutput: "212").VerifyDiagnostics();

var source3 = """
(new string[0]).M1();
""";
CreateCompilationWithSpanAndMemoryExtensions([source1, source3]).VerifyDiagnostics(
// (1,17): error CS0121: The call is ambiguous between the following methods or properties: 'E.M1(Span<string>)' and 'E.M1(ReadOnlySpan<object>)'
// (new string[0]).M1();
Diagnostic(ErrorCode.ERR_AmbigCall, "M1").WithArguments("E.M1(System.Span<string>)", "E.M1(System.ReadOnlySpan<object>)").WithLocation(1, 17));
}

[Fact]
Expand Down