Skip to content

Commit

Permalink
Ignore user-defined Span conversions
Browse files Browse the repository at this point in the history
  • Loading branch information
jjonescz committed Jun 17, 2024
1 parent 10691c9 commit 0a3b139
Show file tree
Hide file tree
Showing 10 changed files with 477 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ private static void AssertTrivialConversion(ConversionKind kind)
case ConversionKind.InterpolatedStringHandler:
case ConversionKind.InlineArray:
case ConversionKind.ImplicitSpan:
case ConversionKind.ExplicitSpan:
isTrivial = true;
break;

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

// 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 @@ -831,7 +833,7 @@ public bool IsReference
{
get
{
return Kind == ConversionKind.ImplicitSpan;
return Kind is ConversionKind.ImplicitSpan or ConversionKind.ExplicitSpan;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@ internal enum ConversionKind : byte
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
ExplicitSpan, // A conversion from array to (ReadOnly)Span
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public static bool IsImplicitConversion(this ConversionKind conversionKind)
case ExplicitPointerToInteger:
case ExplicitIntegerToPointer:
case IntPtr:
case ExplicitSpan:
return false;

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,11 @@ private Conversion ClassifyExplicitBuiltInOnlyConversion(TypeSymbol source, Type
return Conversion.ExplicitDynamic;
}

if (HasExplicitSpanConversion(source, destination, ref useSiteInfo))
{
return Conversion.ExplicitSpan;
}

return Conversion.NoConversion;
}

Expand Down Expand Up @@ -995,6 +1000,7 @@ private static bool ExplicitConversionMayDifferFromImplicit(Conversion implicitC
case ConversionKind.ImplicitTupleLiteral:
case ConversionKind.ImplicitNullable:
case ConversionKind.ConditionalExpression:
case ConversionKind.ImplicitSpan:
return true;

default:
Expand Down Expand Up @@ -1903,7 +1909,7 @@ public Conversion ConvertExtensionMethodThisArg(TypeSymbol parameterType, TypeSy
public Conversion ClassifyImplicitExtensionMethodThisArgConversion(BoundExpression sourceExpressionOpt, TypeSymbol sourceType, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert(sourceExpressionOpt is null || Compilation is not null);
Debug.Assert(sourceExpressionOpt == null || (object)sourceExpressionOpt.Type == sourceType);
Debug.Assert(sourceExpressionOpt == null || TypeSymbol.Equals(sourceExpressionOpt.Type, sourceType, TypeCompareKind.ConsiderEverything));
Debug.Assert((object)destination != null);

if ((object)sourceType != null)
Expand Down Expand Up @@ -3921,7 +3927,8 @@ private static bool IsIntegerTypeSupportingPointerConversions(TypeSymbol type)
return false;
}

private bool HasImplicitSpanConversion(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
#nullable enable
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)
Expand Down Expand Up @@ -3961,5 +3968,61 @@ bool hasIdentityConversion(TypeWithAnnotations source, TypeWithAnnotations desti
HasTopLevelNullabilityIdentityConversion(source, destination);
}
}

/// <remarks>
/// This does not check implicit span conversions, that should be done by the caller.
/// </remarks>
private bool HasExplicitSpanConversion(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 `Ti`
// to `System.Span<Ui>` or `System.ReadOnlySpan<Ui>`
// provided an explicit reference conversion exists from `Ti` to `Ui`.
if (source is ArrayTypeSymbol { IsSZArray: true, ElementTypeWithAnnotations: { } elementType } &&
(destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions) ||
destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions)))
{
var spanElementType = ((NamedTypeSymbol)destination).TypeArgumentsWithDefinitionUseSiteDiagnostics(ref useSiteInfo)[0];
return HasIdentityOrReferenceConversion(elementType.Type, spanElementType.Type, ref useSiteInfo) &&
HasTopLevelNullabilityIdentityConversion(elementType, spanElementType);
}

return false;
}

private bool IgnoreUserDefinedSpanConversions(TypeSymbol? source, TypeSymbol? target)
{
if (source is null || target is null)
{
return false;
}

// PROTOTYPE: Is it fine that this check is not performed when Compilation is null?
return Compilation?.IsFeatureEnabled(MessageID.IDS_FeatureFirstClassSpan) == true &&
(ignoreUserDefinedSpanConversionsInOneDirection(Compilation, source, target) ||
ignoreUserDefinedSpanConversionsInOneDirection(Compilation, target, source));

static bool ignoreUserDefinedSpanConversionsInOneDirection(CSharpCompilation compilation, TypeSymbol a, TypeSymbol b)
{
// SPEC: User-defined conversions are not considered when converting between
// SPEC: - any single-dimensional `array_type` and `System.Span<T>`/`System.ReadOnlySpan<T>`
if (a is ArrayTypeSymbol { IsSZArray: true } &&
(b.OriginalDefinition.Equals(compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions) ||
b.OriginalDefinition.Equals(compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions)))
{
return true;
}

// PROTOTYPE: - any combination of `System.Span<T>`/`System.ReadOnlySpan<T>`
// PROTOTYPE: - `string` and `System.ReadOnlySpan<char>`

return false;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ private void AddUserDefinedConversionsToExplicitCandidateSet(
return;
}

if (IgnoreUserDefinedSpanConversions(source, target))
{
return;
}

ImmutableArray<MethodSymbol> operators = declaringType.GetOperators(
isExplicit ? (isChecked ? WellKnownMemberNames.CheckedExplicitConversionName : WellKnownMemberNames.ExplicitConversionName) : WellKnownMemberNames.ImplicitConversionName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,11 @@ private void ComputeApplicableUserDefinedImplicitConversionSet(
return;
}

if (IgnoreUserDefinedSpanConversions(source, target))
{
return;
}

bool haveInterfaces = false;

foreach ((NamedTypeSymbol declaringType, TypeParameterSymbol constrainedToTypeOpt) in d)
Expand Down Expand Up @@ -639,6 +644,7 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind)
case ConversionKind.IntPtr:
case ConversionKind.ExplicitTupleLiteral:
case ConversionKind.ExplicitTuple:
case ConversionKind.ExplicitSpan:
return false;

// Spec'd in C# 4.
Expand Down
19 changes: 16 additions & 3 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7908,10 +7908,10 @@ private Conversion GenerateConversionForConditionalOperator(BoundExpression sour
return conversion;
}

private Conversion GenerateConversion(Conversions conversions, BoundExpression? sourceExpression, TypeSymbol? sourceType, TypeSymbol destinationType, bool fromExplicitCast, bool extensionMethodThisArgument, bool isChecked)
private Conversion GenerateConversion(Conversions conversions, BoundExpression? sourceExpression, TypeSymbol? sourceType, TypeSymbol destinationType, bool fromExplicitCast, bool extensionMethodThisArgument, bool isChecked, bool forceFromExpression = false)
{
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
bool useExpression = sourceType is null || UseExpressionForConversion(sourceExpression);
bool useExpression = forceFromExpression || sourceType is null || UseExpressionForConversion(sourceExpression);
if (extensionMethodThisArgument)
{
return conversions.ClassifyImplicitExtensionMethodThisArgConversion(
Expand Down Expand Up @@ -8890,14 +8890,27 @@ 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);
canConvertNestedNullability = conversion.Exists;
}
break;

case ConversionKind.ImplicitSpan:
case ConversionKind.ExplicitSpan:
if (checkConversion)
{
var previousKind = conversion.Kind;
conversion = GenerateConversion(_conversions, conversionOperand, operandType.Type, targetType, fromExplicitCast, extensionMethodThisArgument, isChecked: conversionOpt?.Checked ?? false,
// Span conversion is "from expression".
// PROTOTYPE: Should it be "from type" instead?
forceFromExpression: true);
Debug.Assert(!conversion.Exists || conversion.Kind == previousKind);
canConvertNestedNullability = conversion.Exists;
}
break;

default:
Debug.Assert(targetType.IsValueType || targetType.IsErrorType());
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ private BoundExpression MakeConversionNodeCore(
}

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

Expand Down
Loading

0 comments on commit 0a3b139

Please sign in to comment.