Skip to content

Commit

Permalink
Prefer implicit span conversions (#73765)
Browse files Browse the repository at this point in the history
* Add standard implicit array-to-Span conversions

* Mitigate breaks in bootstrapped Roslyn

* Add more tests

* Improve code

* Extend tests

* Use implicit conversion instead of constructor

* Improve code and tests

* Update tests

* Prefer implicit span conversions

* Extend tests

* Extend tests

* Extend tests

* Update new tests after merge

* Revert "Mitigate breaks in bootstrapped Roslyn"

This reverts commit a9556fa.

* Mitigate remaining bootstrap break

* Fix another instance of the same bootstrap break
  • Loading branch information
jjonescz authored Jun 14, 2024
1 parent c24c886 commit 638cf61
Show file tree
Hide file tree
Showing 17 changed files with 197 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2767,6 +2767,16 @@ private BetterResult BetterConversionFromExpression(BoundExpression node, TypeSy
return BetterResult.Neither;
}

switch ((conv1.Kind, conv2.Kind))
{
case (ConversionKind.ImplicitSpan, ConversionKind.ImplicitSpan):
break;
case (_, ConversionKind.ImplicitSpan):
return BetterResult.Right;
case (ConversionKind.ImplicitSpan, _):
return BetterResult.Left;
}

// - T1 is a better conversion target than T2 and either C1 and C2 are both conditional expression
// conversions or neither is a conditional expression conversion.
return BetterConversionTarget(node, t1, conv1, t2, conv2, ref useSiteInfo, out okToDowngradeToNeither);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void DiagnosticAnalyzerAllInOne()
missingSyntaxKinds.Add(SyntaxKind.SpreadElement);

var analyzer = new CSharpTrackingDiagnosticAnalyzer();
var options = new AnalyzerOptions([new TestAdditionalText()]);
var options = new AnalyzerOptions(new[] { new TestAdditionalText() }.ToImmutableArray<AdditionalText>());
CreateCompilationWithMscorlib45(source).VerifyAnalyzerDiagnostics(new[] { analyzer }, options);
analyzer.VerifyAllAnalyzerMembersWereCalled();
analyzer.VerifyAnalyzeSymbolCalledForAllSymbolKinds();
Expand Down Expand Up @@ -99,7 +99,7 @@ public class C
public void AnalyzerDriverIsSafeAgainstAnalyzerExceptions()
{
var compilation = CreateCompilationWithMscorlib45(TestResource.AllInOneCSharpCode);
var options = new AnalyzerOptions([new TestAdditionalText()]);
var options = new AnalyzerOptions(new[] { new TestAdditionalText() }.ToImmutableArray<AdditionalText>());

ThrowingDiagnosticAnalyzer<SyntaxKind>.VerifyAnalyzerEngineIsSafeAgainstExceptions(analyzer =>
compilation.GetAnalyzerDiagnostics(new[] { analyzer }, options));
Expand All @@ -111,7 +111,7 @@ public void AnalyzerOptionsArePassedToAllAnalyzers()
var text = new StringText(string.Empty, encodingOpt: null);
AnalyzerOptions options = new AnalyzerOptions
(
[new TestAdditionalText("myfilepath", text)]
new[] { new TestAdditionalText("myfilepath", text) }.ToImmutableArray<AdditionalText>()
);

var compilation = CreateCompilationWithMscorlib45(TestResource.AllInOneCSharpCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,7 @@ static void Main()
""";
comp = CreateCompilation(
new[] { sourceA, sourceB2 },
parseOptions: TestOptions.Regular12,
targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(
// 1.cs(5,9): error CS0121: The call is ambiguous between the following methods or properties: 'Program.SpanDerived(Span<string>)' and 'Program.SpanDerived(object[])'
Expand All @@ -1584,6 +1585,24 @@ static void Main()
// 1.cs(6,9): error CS0121: The call is ambiguous between the following methods or properties: 'Program.ArrayDerived(Span<object>)' and 'Program.ArrayDerived(string[])'
// ArrayDerived([string.Empty]); // ambiguous
Diagnostic(ErrorCode.ERR_AmbigCall, "ArrayDerived").WithArguments("Program.ArrayDerived(System.Span<object>)", "Program.ArrayDerived(string[])").WithLocation(6, 9));

var expectedDiagnostics = new[]
{
// 1.cs(6,9): error CS0121: The call is ambiguous between the following methods or properties: 'Program.ArrayDerived(Span<object>)' and 'Program.ArrayDerived(string[])'
// ArrayDerived([string.Empty]); // ambiguous
Diagnostic(ErrorCode.ERR_AmbigCall, "ArrayDerived").WithArguments("Program.ArrayDerived(System.Span<object>)", "Program.ArrayDerived(string[])").WithLocation(6, 9)
};

comp = CreateCompilation(
new[] { sourceA, sourceB2 },
parseOptions: TestOptions.RegularNext,
targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(expectedDiagnostics);

comp = CreateCompilation(
new[] { sourceA, sourceB2 },
targetFramework: TargetFramework.Net80);
comp.VerifyEmitDiagnostics(expectedDiagnostics);
}

[WorkItem("https://github.com/dotnet/roslyn/issues/69634")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
Expand Down
189 changes: 153 additions & 36 deletions src/Compilers/CSharp/Test/Emit3/FirstClassSpanTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,13 @@ static class E
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "2").VerifyDiagnostics();

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

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

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

[Theory, MemberData(nameof(LangVersions))]
Expand Down Expand Up @@ -261,11 +259,13 @@ static class E
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "2").VerifyDiagnostics();

// PROTOTYPE: Can we avoid this break?

var expectedDiagnostics = new[]
{
// (5,5): error CS0121: The call is ambiguous between the following methods or properties: 'E.M<T>(Span<T>, T)' and 'E.M<T>(IEnumerable<T>, T)'
// (5,5): error CS1113: Extension method 'E.M<int>(Span<int>, int)' defined on value type 'Span<int>' cannot be used to create delegates
// E.R(arr.M);
Diagnostic(ErrorCode.ERR_AmbigCall, "arr.M").WithArguments("E.M<T>(System.Span<T>, T)", "E.M<T>(System.Collections.Generic.IEnumerable<T>, T)").WithLocation(5, 5)
Diagnostic(ErrorCode.ERR_ValueTypeExtDelegate, "arr.M").WithArguments("E.M<int>(System.Span<int>, int)", "System.Span<int>").WithLocation(5, 5)
};

CreateCompilationWithSpan(source, parseOptions: TestOptions.RegularNext).VerifyDiagnostics(expectedDiagnostics);
Expand Down Expand Up @@ -2345,6 +2345,87 @@ .locals init (object[] V_0)
verifier.VerifyIL("C.M", expectedIl);
}

[Fact]
public void OverloadResolution_SpanVsIEnumerable()
{
var source = """
using System;
using System.Collections.Generic;
var a = new int[0];
C.M(a);
static class C
{
public static void M(Span<int> x) => Console.Write(1);
public static void M(IEnumerable<int> x) => Console.Write(2);
}
""";

CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12).VerifyDiagnostics(
// (5,3): error CS0121: The call is ambiguous between the following methods or properties: 'C.M(Span<int>)' and 'C.M(IEnumerable<int>)'
// C.M(a);
Diagnostic(ErrorCode.ERR_AmbigCall, "M").WithArguments("C.M(System.Span<int>)", "C.M(System.Collections.Generic.IEnumerable<int>)").WithLocation(5, 3));

var expectedOutput = "1";

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

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

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_SpanVsIEnumerable_CollectionExpression(LanguageVersion langVersion)
{
var source = """
using System;
using System.Collections.Generic;
C.M([]);
static class C
{
public static void M(Span<int> x) => Console.Write(1);
public static void M(IEnumerable<int> x) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
CompileAndVerify(comp, expectedOutput: "1").VerifyDiagnostics();
}

[Fact]
public void OverloadResolution_SpanVsIEnumerable_Ctor()
{
var source = """
using System;
using System.Collections.Generic;
var a = new int[0];
var c = new C(a);
class C
{
public C(Span<int> x) => Console.Write(1);
public C(IEnumerable<int> x) => Console.Write(2);
}
""";

CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12).VerifyDiagnostics(
// (5,13): error CS0121: The call is ambiguous between the following methods or properties: 'C.C(Span<int>)' and 'C.C(IEnumerable<int>)'
// var c = new C(a);
Diagnostic(ErrorCode.ERR_AmbigCall, "C").WithArguments("C.C(System.Span<int>)", "C.C(System.Collections.Generic.IEnumerable<int>)").WithLocation(5, 13));

var expectedOutput = "1";

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

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

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_ReadOnlySpanVsArray_01(LanguageVersion langVersion)
{
Expand Down Expand Up @@ -2391,8 +2472,8 @@ public static void M(ReadOnlySpan<object> x) { }
Diagnostic(ErrorCode.ERR_AmbigCall, "M").WithArguments("C.M(string[])", "C.M(System.ReadOnlySpan<object>)").WithLocation(5, 3));
}

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_ReadOnlySpanVsArray_03(LanguageVersion langVersion)
[Fact]
public void OverloadResolution_ReadOnlySpanVsArray_03()
{
var source = """
using System;
Expand All @@ -2410,8 +2491,16 @@ static class C
public static void M(ReadOnlySpan<object> x) => Console.Write(" r" + x[0]);
}
""";
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "aa rSystem.String[] ra ra ra").VerifyDiagnostics();

var expectedOutput = "ra rSystem.String[] ra ra ra";

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

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

[Theory, MemberData(nameof(LangVersions))]
Expand Down Expand Up @@ -2504,7 +2593,7 @@ static class C
}
""";
var comp = CreateCompilationWithSpan(source);
CompileAndVerify(comp, expectedOutput: "aa rSystem.String[] ra ra ra").VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "ra rSystem.String[] ra ra ra").VerifyDiagnostics();
}

[Fact]
Expand Down Expand Up @@ -2566,21 +2655,17 @@ static class C
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "oa").VerifyDiagnostics();

// PROTOTYPE: This break should go away with betterness rule.
var expectedOutput = "sa";

var expectedDiagnostics = new[]
{
// (4,3): error CS0121: The call is ambiguous between the following methods or properties: 'C.M(object[])' and 'C.M(ReadOnlySpan<string>)'
// a.M();
Diagnostic(ErrorCode.ERR_AmbigCall, "M").WithArguments("C.M(object[])", "C.M(System.ReadOnlySpan<string>)").WithLocation(4, 3)
};
comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.RegularNext);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();

CreateCompilationWithSpan(source, parseOptions: TestOptions.RegularNext).VerifyDiagnostics(expectedDiagnostics);
CreateCompilationWithSpan(source).VerifyDiagnostics(expectedDiagnostics);
comp = CreateCompilationWithSpan(source);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_ReadOnlySpanVsArray_ExtensionMethodReceiver_03(LanguageVersion langVersion)
[Fact]
public void OverloadResolution_ReadOnlySpanVsArray_ExtensionMethodReceiver_03()
{
var source = """
using System;
Expand All @@ -2594,8 +2679,16 @@ static class C
public static void M(this ReadOnlySpan<object> x) => Console.Write(" r" + x[0]);
}
""";
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "aa").VerifyDiagnostics();

var expectedOutput = "ra";

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

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

[Theory, MemberData(nameof(LangVersions))]
Expand Down Expand Up @@ -2637,8 +2730,8 @@ static class C
CompileAndVerify(comp, expectedOutput: "112").VerifyDiagnostics();
}

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_SpanVsReadOnlySpan_02(LanguageVersion langVersion)
[Fact]
public void OverloadResolution_SpanVsReadOnlySpan_02()
{
var source = """
using System;
Expand All @@ -2655,8 +2748,16 @@ static class C
public static void M(ReadOnlySpan<object> arg) => Console.Write(2);
}
""";
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "1121").VerifyDiagnostics();

var expectedOutput = "1122";

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

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

[Theory, MemberData(nameof(LangVersions))]
Expand Down Expand Up @@ -2788,8 +2889,8 @@ public static void E(this ReadOnlySpan<object> arg) { }
// PROTOTYPE: Should work in C# 13 when ROS->ROS conversion is implemented.
}

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_ReadOnlySpanVsArrayVsSpan(LanguageVersion langVersion)
[Fact]
public void OverloadResolution_ReadOnlySpanVsArrayVsSpan()
{
var source = """
using System;
Expand Down Expand Up @@ -2817,8 +2918,16 @@ static class C
public static void M(IEnumerable<object> x) => Console.Write(" e" + x.First());
}
""";
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "aa rSystem.String[] ra ra ra ab rSystem.Object[] rb rb").VerifyDiagnostics();

var expectedOutput = "ra rSystem.String[] ra ra ra ab rSystem.Object[] rb rb";

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

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

[Fact]
Expand Down Expand Up @@ -2851,11 +2960,11 @@ static class C
}
""";
var comp = CreateCompilationWithSpan(source);
CompileAndVerify(comp, expectedOutput: "aa rSystem.String[] ra ra ra ab rSystem.Object[] rb rb").VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "ra rSystem.String[] ra ra ra ab rSystem.Object[] rb rb").VerifyDiagnostics();
}

[Theory, MemberData(nameof(LangVersions))]
public void OverloadResolution_ReadOnlySpanVsArrayVsSpan_ExtensionMethodReceiver(LanguageVersion langVersion)
[Fact]
public void OverloadResolution_ReadOnlySpanVsArrayVsSpan_ExtensionMethodReceiver()
{
var source = """
using System;
Expand All @@ -2876,7 +2985,15 @@ static class C
public static void M(this IEnumerable<object> x) => Console.Write(" e" + x.First());
}
""";
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular.WithLanguageVersion(langVersion));
var comp = CreateCompilationWithSpan(source, parseOptions: TestOptions.Regular12);
CompileAndVerify(comp, expectedOutput: "aa ab").VerifyDiagnostics();

var expectedOutput = "ra ab";

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

comp = CreateCompilationWithSpan(source);
CompileAndVerify(comp, expectedOutput: expectedOutput).VerifyDiagnostics();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
Expand Down
Loading

0 comments on commit 638cf61

Please sign in to comment.