Skip to content

Commit

Permalink
Remove function type conversions from standard conversions
Browse files Browse the repository at this point in the history
  • Loading branch information
@cston
cston committed Sep 19, 2021
1 parent df95d51 commit f8c5df3
Show file tree
Hide file tree
Showing 7 changed files with 201 additions and 147 deletions.
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Semantics/BestTypeInferrer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
}

var conversionsWithoutNullability = conversions.WithNullability(false);
var t1tot2 = conversionsWithoutNullability.ClassifyImplicitConversionFromType(type1, type2, ref useSiteInfo).Exists;
var t2tot1 = conversionsWithoutNullability.ClassifyImplicitConversionFromType(type2, type1, ref useSiteInfo).Exists;
var t1tot2 = conversionsWithoutNullability.ClassifyImplicitConversionFromTypeOrImplicitFunctionTypeConversion(type1, type2, ref useSiteInfo).Exists;
var t2tot1 = conversionsWithoutNullability.ClassifyImplicitConversionFromTypeOrImplicitFunctionTypeConversion(type2, type1, ref useSiteInfo).Exists;

if (t1tot2 && t2tot1)
{
Expand Down
134 changes: 108 additions & 26 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ internal ConversionsBase WithNullability(bool includeNullability)

internal AssemblySymbol CorLibrary { get { return corLibrary; } }

#nullable enable
/// <summary>
/// Determines if the source expression is convertible to the destination type via
/// any built-in or user-defined implicit conversion.
Expand All @@ -84,10 +85,10 @@ public Conversion ClassifyImplicitConversionFromExpression(BoundExpression sourc
Debug.Assert(sourceExpression != null);
Debug.Assert((object)destination != null);

var sourceType = sourceExpression.GetTypeOrFunctionType();
var sourceType = sourceExpression.Type;

//PERF: identity conversion is by far the most common implicit conversion, check for that first
if ((object)sourceType != null && HasIdentityConversionInternal(sourceType, destination))
if (sourceType is { } && HasIdentityConversionInternal(sourceType, destination))
{
return Conversion.Identity;
}
Expand All @@ -98,7 +99,7 @@ public Conversion ClassifyImplicitConversionFromExpression(BoundExpression sourc
return conversion;
}

if ((object)sourceType != null)
if (sourceType is { })
{
// Try using the short-circuit "fast-conversion" path.
Conversion fastConversion = FastClassifyConversion(sourceType, destination);
Expand All @@ -118,6 +119,13 @@ public Conversion ClassifyImplicitConversionFromExpression(BoundExpression sourc
}
}
}
else if (sourceExpression.GetFunctionType() is { })
{
if (IsValidFunctionTypeConversionTarget(destination, ref useSiteInfo))
{
return Conversion.FunctionType;
}
}

conversion = GetImplicitUserDefinedConversion(sourceExpression, sourceType, destination, ref useSiteInfo);
if (conversion.Exists)
Expand Down Expand Up @@ -171,6 +179,34 @@ public Conversion ClassifyImplicitConversionFromType(TypeSymbol source, TypeSymb
return GetImplicitUserDefinedConversion(null, source, destination, ref useSiteInfo);
}

/// <summary>
/// Helper method that calls <see cref="ClassifyImplicitConversionFromType"/> or
/// <see cref="HasImplicitFunctionTypeConversion"/> depending on whether the
/// types are <see cref="FunctionTypeSymbol"/> instances.
/// Used by method type inference and best common type only.
/// </summary>
public Conversion ClassifyImplicitConversionFromTypeOrImplicitFunctionTypeConversion(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var sourceFunctionType = source as FunctionTypeSymbol;
var destinationFunctionType = destination as FunctionTypeSymbol;

if (sourceFunctionType is null && destinationFunctionType is null)
{
return ClassifyImplicitConversionFromType(source, destination, ref useSiteInfo);
}

if (sourceFunctionType is { } && destinationFunctionType is { })
{
return HasImplicitFunctionTypeConversion(sourceFunctionType, destinationFunctionType, ref useSiteInfo) ?
Conversion.FunctionType :
Conversion.NoConversion;
}

Debug.Assert(false);
return Conversion.NoConversion;
}
#nullable disable

/// <summary>
/// Determines if the source expression of given type is convertible to the destination type via
/// any built-in or user-defined conversion.
Expand Down Expand Up @@ -513,10 +549,57 @@ public Conversion ClassifyStandardConversion(BoundExpression sourceExpression, T
return Conversion.NoConversion;
}

private static bool IsStandardImplicitConversionFromExpression(ConversionKind kind)
{
if (IsStandardImplicitConversionFromType(kind))
{
return true;
}

// See comment in ClassifyStandardImplicitConversion(BoundExpression, ...)
// where the set of standard implicit conversions is extended from the spec
// to include conversions from expression.
switch (kind)
{
case ConversionKind.AnonymousFunction:
case ConversionKind.MethodGroup:
case ConversionKind.ImplicitEnumeration:
case ConversionKind.ImplicitDynamic:
case ConversionKind.ImplicitNullToPointer:
case ConversionKind.ImplicitTupleLiteral:
case ConversionKind.StackAllocToPointerType:
case ConversionKind.StackAllocToSpanType:
return true;
default:
return false;
}
}

// See https://github.com/dotnet/csharplang/blob/main/spec/conversions.md#standard-conversions:
// "The standard conversions are those pre-defined conversions that can occur as part of a user-defined conversion."
private static bool IsStandardImplicitConversionFromType(ConversionKind kind)
{
switch (kind)
{
case ConversionKind.Identity:
case ConversionKind.ImplicitNumeric:
case ConversionKind.ImplicitNullable:
case ConversionKind.ImplicitReference:
case ConversionKind.Boxing:
case ConversionKind.ImplicitConstant:
case ConversionKind.ImplicitPointer:
case ConversionKind.ImplicitPointerToVoid:
case ConversionKind.ImplicitTuple:
return true;
default:
return false;
}
}

private Conversion ClassifyStandardImplicitConversion(BoundExpression sourceExpression, TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert(sourceExpression != null || (object)source != null);
Debug.Assert(sourceExpression == null || (object)sourceExpression.GetTypeOrFunctionType() == (object)source);
Debug.Assert(sourceExpression == null || (object)sourceExpression.Type == (object)source);
Debug.Assert((object)destination != null);

// SPEC: The following implicit conversions are classified as standard implicit conversions:
Expand Down Expand Up @@ -550,6 +633,7 @@ private Conversion ClassifyStandardImplicitConversion(BoundExpression sourceExpr
Conversion conversion = ClassifyImplicitBuiltInConversionFromExpression(sourceExpression, source, destination, ref useSiteInfo);
if (conversion.Exists)
{
Debug.Assert(IsStandardImplicitConversionFromExpression(conversion.Kind));
return conversion;
}

Expand All @@ -562,9 +646,17 @@ private Conversion ClassifyStandardImplicitConversion(BoundExpression sourceExpr
}

private Conversion ClassifyStandardImplicitConversion(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var conversion = ClassifyStandardImplicitConversionInternal(source, destination, ref useSiteInfo);
Debug.Assert(conversion.Kind == ConversionKind.NoConversion || IsStandardImplicitConversionFromType(conversion.Kind));
return conversion;
}

private Conversion ClassifyStandardImplicitConversionInternal(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert((object)source != null);
Debug.Assert((object)destination != null);
Debug.Assert(source is not FunctionTypeSymbol);

if (HasIdentityConversionInternal(source, destination))
{
Expand All @@ -582,13 +674,6 @@ private Conversion ClassifyStandardImplicitConversion(TypeSymbol source, TypeSym
return nullableConversion;
}

if (source is FunctionTypeSymbol functionType)
{
return HasImplicitFunctionTypeConversion(functionType, destination, ref useSiteInfo) ?
Conversion.FunctionType :
Conversion.NoConversion;
}

if (HasImplicitReferenceConversion(source, destination, ref useSiteInfo))
{
return Conversion.ImplicitReference;
Expand Down Expand Up @@ -867,7 +952,7 @@ private static bool ExplicitConversionMayDifferFromImplicit(Conversion implicitC
private Conversion ClassifyImplicitBuiltInConversionFromExpression(BoundExpression sourceExpression, TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert(sourceExpression != null || (object)source != null);
Debug.Assert(sourceExpression == null || (object)sourceExpression.GetTypeOrFunctionType() == (object)source);
Debug.Assert(sourceExpression == null || (object)sourceExpression.Type == (object)source);
Debug.Assert((object)destination != null);

if (HasImplicitDynamicConversionFromExpression(source, destination))
Expand Down Expand Up @@ -1188,6 +1273,7 @@ internal static bool HasImplicitConstantExpressionConversion(BoundExpression sou
return false;
}

#nullable enable
private Conversion ClassifyExplicitOnlyConversionFromExpression(BoundExpression sourceExpression, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo, bool forCast)
{
Debug.Assert(sourceExpression != null);
Expand All @@ -1205,8 +1291,8 @@ private Conversion ClassifyExplicitOnlyConversionFromExpression(BoundExpression
}
}

var sourceType = sourceExpression.GetTypeOrFunctionType();
if ((object)sourceType != null)
var sourceType = sourceExpression.Type;
if (sourceType is { })
{
// Try using the short-circuit "fast-conversion" path.
Conversion fastConversion = FastClassifyConversion(sourceType, destination);
Expand All @@ -1223,11 +1309,17 @@ private Conversion ClassifyExplicitOnlyConversionFromExpression(BoundExpression
}
}
}
else if (sourceExpression.GetFunctionType() is { })
{
if (IsValidFunctionTypeConversionTarget(destination, ref useSiteInfo))
{
return Conversion.FunctionType;
}
}

return GetExplicitUserDefinedConversion(sourceExpression, sourceType, destination, ref useSiteInfo);
}

#nullable enable
private static bool HasImplicitEnumerationConversion(BoundExpression source, TypeSymbol destination)
{
Debug.Assert((object)source != null);
Expand Down Expand Up @@ -2572,16 +2664,6 @@ private bool HasImplicitConversionFromDelegate(TypeSymbol source, TypeSymbol des
return false;
}

private bool HasImplicitFunctionTypeConversion(FunctionTypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (destination is FunctionTypeSymbol destinationFunctionType)
{
return HasImplicitSignatureConversion(source, destinationFunctionType, ref useSiteInfo);
}

return IsValidFunctionTypeConversionTarget(destination, ref useSiteInfo);
}

internal bool IsValidFunctionTypeConversionTarget(TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (destination.SpecialType == SpecialType.System_MulticastDelegate)
Expand All @@ -2604,7 +2686,7 @@ internal bool IsValidFunctionTypeConversionTarget(TypeSymbol destination, ref Co
return false;
}

private bool HasImplicitSignatureConversion(FunctionTypeSymbol sourceType, FunctionTypeSymbol destinationType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
private bool HasImplicitFunctionTypeConversion(FunctionTypeSymbol sourceType, FunctionTypeSymbol destinationType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var sourceDelegate = sourceType.GetInternalDelegateType();
var destinationDelegate = destinationType.GetInternalDelegateType();
Expand Down
4 changes: 1 addition & 3 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/UserDefinedImplicitConversions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind)
// Not "standard".
case ConversionKind.ImplicitUserDefined:
case ConversionKind.ExplicitUserDefined:
//case ConversionKind.FunctionType:

// Not implicit.
case ConversionKind.ExplicitNumeric:
Expand Down Expand Up @@ -642,9 +643,6 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind)

// Added for C# 7.1
case ConversionKind.DefaultLiteral:

// Added for C# 10.
case ConversionKind.FunctionType:
return true;

default:
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2793,7 +2793,7 @@ private static bool ImplicitConversionExists(TypeWithAnnotations sourceWithAnnot
return false;
}

return conversions.ClassifyImplicitConversionFromType(source, destination, ref useSiteInfo).Exists;
return conversions.ClassifyImplicitConversionFromTypeOrImplicitFunctionTypeConversion(source, destination, ref useSiteInfo).Exists;
}
#nullable disable

Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/UsingStatementBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ bool populateDisposableConversionOrDisposeMethod(bool fromExpression, out Conver
Conversion classifyConversion(bool fromExpression, TypeSymbol targetInterface, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
return fromExpression ?
originalBinder.Conversions.ClassifyImplicitConversionFromExpression(expressionOpt, targetInterface, ref useSiteInfo) :
originalBinder.Conversions.ClassifyImplicitConversionFromType(declarationTypeOpt, targetInterface, ref useSiteInfo);
originalBinder.Conversions.ClassifyImplicitConversionFromExpression(expressionOpt!, targetInterface, ref useSiteInfo) :
originalBinder.Conversions.ClassifyImplicitConversionFromType(declarationTypeOpt!, targetInterface, ref useSiteInfo);
}

TypeSymbol getDisposableInterface(bool isAsync)
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6327,10 +6327,10 @@ private static Conversion GenerateConversion(Conversions conversions, BoundExpre
return useExpression ?
(fromExplicitCast ?
conversions.ClassifyConversionFromExpression(sourceExpression, destinationType, ref discardedUseSiteInfo, forCast: true) :
conversions.ClassifyImplicitConversionFromExpression(sourceExpression, destinationType, ref discardedUseSiteInfo)) :
conversions.ClassifyImplicitConversionFromExpression(sourceExpression!, destinationType, ref discardedUseSiteInfo)) :
(fromExplicitCast ?
conversions.ClassifyConversionFromType(sourceType, destinationType, ref discardedUseSiteInfo, forCast: true) :
conversions.ClassifyImplicitConversionFromType(sourceType, destinationType, ref discardedUseSiteInfo));
conversions.ClassifyImplicitConversionFromType(sourceType!, destinationType, ref discardedUseSiteInfo));
}

/// <summary>
Expand Down
Loading

0 comments on commit f8c5df3

Please sign in to comment.