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.

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

Add standard implicit array-to-Span conversions #73577

Merged
13 changes: 13 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,19 @@ void checkConstraintLanguageVersionAndRuntimeSupportForConversion(SyntaxNode syn

CheckInlineArrayTypeIsSupported(syntax, source.Type, elementField.Type, diagnostics);
}
else if (conversion.IsSpan)
{
// PROTOTYPE: Check runtime APIs used for other span conversions once they are implemented.
if (destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions))
{
_ = GetWellKnownTypeMember(WellKnownMember.System_ReadOnlySpan_T__ctor_Array, diagnostics, syntax: syntax);
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Debug.Assert(destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions));
_ = GetWellKnownTypeMember(WellKnownMember.System_Span_T__ctor_Array, diagnostics, syntax: syntax);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ private BoundCall BindInvocationExpressionContinued(
ParameterSymbol receiverParameter = method.Parameters.First();

// we will have a different receiver if ReplaceTypeOrValueReceiver has unwrapped TypeOrValue
if ((object)receiver != receiverArgument)
if ((object)receiver != methodGroup.Receiver)
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
{
// Because the receiver didn't pass through CoerceArguments, we need to apply an appropriate conversion here.
Debug.Assert(argsToParams.IsDefault || argsToParams[0] == 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ private static void AssertTrivialConversion(ConversionKind kind)
case ConversionKind.InterpolatedString:
case ConversionKind.InterpolatedStringHandler:
case ConversionKind.InlineArray:
case ConversionKind.ImplicitSpan:
Copy link
Member

@333fred 333fred May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is actually true? Won't we need to store the method symbol of the conversion on the conversion object? #Resolved

Copy link
Member Author

@jjonescz jjonescz May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't seem necessary, we can find the method during lowering. It's the same thing InlineArray conversion does. I think the difference between trivial conversions like ImplicitSpan/InlineArray and complex conversions that need a method symbol like user-defined conversions is that the latter need to do a more complex lookup to find the method symbol whereas the former just obtain a well-known member.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a matter of when we can find the method, the question is the impact to our public API; namely, will we expose the method symbol we look up in Conversion.MethodSymbol or not. I'm currently a bit torn; given that I think we will likely spec which methods the compiler calls for each conversion, it seems like we should include that method in the conversion. On the other hand, it may be that we need to have multiple methods (Span<string> -> ReadOnlySpan<object>, for example, will probably need to go to ReadOnlySpan<string> in between), and that dual method becomes more complicated to represent.

Copy link
Member Author

@jjonescz jjonescz Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the question is the impact to our public API

I see, that sounds like a question for API review.

The docs of Conversion.MethodSymbol currently state that it's set only for user-defined and method group conversions. And indeed, for example, InlineArray conversion also does not expose the MethodSymbol.

There are other places that seem to rely on the fact that Conversion.MethodSymbol is set only for user-defined and method group conversions, e.g., Binder.ReportDiagnosticsIfObsolete - we report obsolete diagnostics only for user-defined methods and not built-in conversions.

isTrivial = true;
break;

Expand Down Expand Up @@ -294,6 +295,7 @@ internal static Conversion GetTrivialConversion(ConversionKind kind)
internal static Conversion ImplicitPointer => new Conversion(ConversionKind.ImplicitPointer);
internal static Conversion FunctionType => new Conversion(ConversionKind.FunctionType);
internal static Conversion InlineArray => new Conversion(ConversionKind.InlineArray);
internal static Conversion ImplicitSpan => new Conversion(ConversionKind.ImplicitSpan);

// trivial conversions that could be underlying in nullable conversion
// NOTE: tuple conversions can be underlying as well, but they are not trivial
Expand Down Expand Up @@ -819,6 +821,20 @@ public bool IsReference
}
}

/// <summary>
/// Returns true if the conversion is a span conversion.
/// </summary>
/// <remarks>
/// Span conversion is available since C# 13 as part of the "first-class Span types" feature.
/// </remarks>
internal bool IsSpan // PROTOTYPE: Make part of public API
{
get
{
return Kind == ConversionKind.ImplicitSpan;
}
}

/// <summary>
/// Returns true if the conversion is an implicit user-defined conversion or explicit user-defined conversion.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,7 @@ internal enum ConversionKind : byte
InterpolatedStringHandler, // A conversion from an interpolated string literal to a type attributed with InterpolatedStringBuilderAttribute

InlineArray, // A conversion from an inline array to Span/ReadOnlySpan

ImplicitSpan, // A conversion between array, (ReadOnly)Span, string - part of the "first-class Span types" feature
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public static bool IsImplicitConversion(this ConversionKind conversionKind)
case ObjectCreation:
case InlineArray:
case CollectionExpression:
case ImplicitSpan:
return true;

case ExplicitNumeric:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ private static bool IsStandardImplicitConversionFromExpression(ConversionKind ki
case ConversionKind.StackAllocToPointerType:
case ConversionKind.StackAllocToSpanType:
case ConversionKind.InlineArray:
case ConversionKind.ImplicitSpan:
return true;
default:
return false;
Expand Down Expand Up @@ -1014,6 +1015,11 @@ private Conversion ClassifyImplicitBuiltInConversionFromExpression(BoundExpressi
return Conversion.ImplicitDynamic;
}

if (HasImplicitSpanConversion(source, destination, ref useSiteInfo))
{
return Conversion.ImplicitSpan;
}

// The following conversions only exist for certain form of expressions,
// if we have no expression none if them is applicable.
if (sourceExpression == null)
Expand Down Expand Up @@ -1916,6 +1922,11 @@ public Conversion ClassifyImplicitExtensionMethodThisArgConversion(BoundExpressi
{
return Conversion.ImplicitReference;
}

if (HasImplicitSpanConversion(sourceType, destination, ref useSiteInfo))
{
return Conversion.ImplicitSpan;
}
}

if (sourceExpressionOpt?.Kind == BoundKind.TupleLiteral)
Expand Down Expand Up @@ -1977,6 +1988,7 @@ public static bool IsValidExtensionMethodThisArgConversion(Conversion conversion
case ConversionKind.Identity:
case ConversionKind.Boxing:
case ConversionKind.ImplicitReference:
case ConversionKind.ImplicitSpan:
return true;

case ConversionKind.ImplicitTuple:
Expand Down Expand Up @@ -3025,7 +3037,7 @@ internal bool HasImplicitConversionToOrImplementsVarianceCompatibleInterface(Bou
// The rules for variant interface and delegate conversions are the same:
//
// An interface/delegate type S is convertible to an interface/delegate type T
// if and only if T is U<S1, ... Sn> and T is U<T1, ... Tn> such that for all
// if and only if S is U<S1, ... Sn> and T is U<T1, ... Tn> such that for all
// parameters of U:
//
// * if the ith parameter of U is invariant then Si is exactly equal to Ti.
Expand Down Expand Up @@ -3908,5 +3920,46 @@ private static bool IsIntegerTypeSupportingPointerConversions(TypeSymbol type)

return false;
}

private bool HasImplicitSpanConversion(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
// PROTOTYPE: Is it fine that this conversion does not exists when Compilation is null?
if (Compilation?.IsFeatureEnabled(MessageID.IDS_FeatureFirstClassSpan) != true)
{
return false;
}

// SPEC: From any single-dimensional `array_type` with element type `Ei`...
if (source is ArrayTypeSymbol { IsSZArray: true, ElementTypeWithAnnotations: { } elementType })
{
// SPEC: ...to `System.Span<Ei>`.
if (destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions))
{
var spanElementType = ((NamedTypeSymbol)destination).TypeArgumentsWithDefinitionUseSiteDiagnostics(ref useSiteInfo)[0];
return hasIdentityConversion(elementType, spanElementType);
}

// SPEC: ...to `System.ReadOnlySpan<Ui>`, provided that `Ei` is covariance-convertible to `Ui`.
if (destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions))
{
var spanElementType = ((NamedTypeSymbol)destination).TypeArgumentsWithDefinitionUseSiteDiagnostics(ref useSiteInfo)[0];
return hasCovariantConversion(elementType, spanElementType, ref useSiteInfo);
}
}

return false;

bool hasCovariantConversion(TypeWithAnnotations source, TypeWithAnnotations destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
return hasIdentityConversion(source, destination) ||
HasImplicitReferenceConversion(source, destination, ref useSiteInfo);
}

bool hasIdentityConversion(TypeWithAnnotations source, TypeWithAnnotations destination)
{
return HasIdentityConversionInternal(source.Type, destination.Type) &&
HasTopLevelNullabilityIdentityConversion(source, destination);
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,8 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind)
case ConversionKind.ImplicitPointer:
// Added for C# 12
case ConversionKind.InlineArray:
// Added for C# 13
case ConversionKind.ImplicitSpan:
return true;

default:
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7956,4 +7956,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_BadAllowByRefLikeEnumerator" xml:space="preserve">
<value>foreach statement cannot operate on enumerators of type '{0}' because it is a type parameter that allows ref struct and it is not known at compile time to implement IDisposable.</value>
</data>
<data name="IDS_FeatureFirstClassSpan" xml:space="preserve">
<value>first-class Span types</value>
</data>
</root>
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ internal enum MessageID
IDS_FeatureRefUnsafeInIteratorAsync = MessageBase + 12843,

IDS_FeatureRefStructInterfaces = MessageBase + 12844,

IDS_FeatureFirstClassSpan = MessageBase + 12845,
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -472,6 +474,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureParamsCollections:
case MessageID.IDS_FeatureRefUnsafeInIteratorAsync:
case MessageID.IDS_FeatureRefStructInterfaces:
case MessageID.IDS_FeatureFirstClassSpan:
return LanguageVersion.Preview;

// C# 12.0 features.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8890,6 +8890,7 @@ private TypeWithState VisitConversion(
break;

case ConversionKind.InlineArray:
case ConversionKind.ImplicitSpan:
if (checkConversion)
{
conversion = GenerateConversion(_conversions, conversionOperand, operandType.Type, targetType, fromExplicitCast, extensionMethodThisArgument, isChecked: conversionOpt?.Checked ?? false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,8 @@ public override BoundNode VisitConversion(BoundConversion node)
}
break;

// PROTOTYPE: ImplicitSpan

default:

if (_inExpressionLambda && node.Conversion.Method is MethodSymbol method && (method.IsAbstract || method.IsVirtual) && method.IsStatic)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,31 @@ private BoundExpression MakeConversionNodeCore(
return _factory.Call(null, createSpan, rewrittenOperand, _factory.Literal(length), useStrictArgumentRefKinds: true);
}

case ConversionKind.ImplicitSpan:
{
var spanType = (NamedTypeSymbol)rewrittenType;

WellKnownMember ctorMember;
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
if (spanType.OriginalDefinition.Equals(_compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions))
{
ctorMember = WellKnownMember.System_ReadOnlySpan_T__ctor_Array;
}
else
{
Debug.Assert(spanType.OriginalDefinition.Equals(_compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions));
ctorMember = WellKnownMember.System_Span_T__ctor_Array;
}

if (!TryGetWellKnownTypeMember(rewrittenOperand.Syntax, ctorMember, out MethodSymbol? ctor))
{
throw ExceptionUtilities.Unreachable();
}
else
{
return new BoundObjectCreationExpression(rewrittenOperand.Syntax, ctor.AsMember((NamedTypeSymbol)rewrittenType), rewrittenOperand);
}
}

default:
break;
}
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading