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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Prefer ReadOnlySpan over Span as better conversion target #76300

Show file tree
Hide file tree
Changes from 1 commit
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
42 changes: 29 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,19 @@ 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;
}
}
jjonescz marked this conversation as resolved.
Show resolved Hide resolved

// 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 +3592,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
118 changes: 116 additions & 2 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 @@ -8062,7 +8062,121 @@ static class C
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "2112").VerifyDiagnostics();
}
jjonescz marked this conversation as resolved.
Show resolved Hide resolved

[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);
}

[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_07()
{
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<string>(D d) => default;
public static implicit operator ReadOnlySpan<object>(D d) => default;
}
""";
CreateCompilationWithSpanAndMemoryExtensions(source, parseOptions: TestOptions.Regular13).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),
// (4,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(4, 3));

var expectedOutput = "11";

var 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]
Expand Down Expand Up @@ -8198,7 +8312,7 @@ static class E
}
""";
var comp = CreateCompilationWithSpanAndMemoryExtensions(source);
CompileAndVerify(comp, expectedOutput: "2212").VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "2112").VerifyDiagnostics();
}

[Fact]
Expand Down