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

Do not include function type conversions in user-defined conversions #56416

Merged
merged 6 commits into from
Sep 24, 2021
Merged

Do not include function type conversions in user-defined conversions #56416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
df95d51
Handle user-defined conversions from lambda expressions or method gro…
cston Sep 15, 2021
6539d9a
Remove function type conversions from standard conversions
cston Sep 17, 2021
0b5884c
Extract local
cston Sep 19, 2021
64c63cf
PR feedback
cston Sep 22, 2021
a37e5f9
PR feedback
cston Sep 22, 2021
7b745c1
Remove implicit conversion case
cston Sep 22, 2021
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
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Semantics/BestTypeInferrer.cs
View file
Open in desktop
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.ClassifyImplicitConversionFromTypeWhenNeitherOrBothFunctionTypes(type1, type2, ref useSiteInfo).Exists;
var t2tot1 = conversionsWithoutNullability.ClassifyImplicitConversionFromTypeWhenNeitherOrBothFunctionTypes(type2, type1, ref useSiteInfo).Exists;

if (t1tot2 && t2tot1)
{
Expand Down
194 changes: 142 additions & 52 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Open in desktop
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 { } sourceFunctionType)
{
if (HasImplicitFunctionTypeConversion(sourceFunctionType, 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="HasImplicitFunctionTypeToFunctionTypeConversion"/> depending on whether the
/// types are <see cref="FunctionTypeSymbol"/> instances.
/// Used by method type inference and best common type only.
/// </summary>
public Conversion ClassifyImplicitConversionFromTypeWhenNeitherOrBothFunctionTypes(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 HasImplicitFunctionTypeToFunctionTypeConversion(sourceFunctionType, destinationFunctionType, ref useSiteInfo) ?
Conversion.FunctionType :
Conversion.NoConversion;
}

Debug.Assert(false);
return Conversion.NoConversion;
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

return Conversion.NoConversion

It is not clear why conversions to System.MulticastDelegate and the like are not checked. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

The only callers of this method are BestTypeInferrer and MethodTypeInferrer in cases where "bounds" are merged. In those cases, there are either two function types or two types that are not function types, so conversions from function type to non-function type do not need to be considered.

}
#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 @@ -563,59 +647,65 @@ private Conversion ClassifyStandardImplicitConversion(BoundExpression sourceExpr

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

if (HasIdentityConversionInternal(source, destination))
Conversion classifyConversion(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
return Conversion.Identity;
}
Debug.Assert((object)source != null);
Debug.Assert((object)destination != null);

if (HasImplicitNumericConversion(source, destination))
{
return Conversion.ImplicitNumeric;
}
if (HasIdentityConversionInternal(source, destination))
{
return Conversion.Identity;
}

var nullableConversion = ClassifyImplicitNullableConversion(source, destination, ref useSiteInfo);
if (nullableConversion.Exists)
{
return nullableConversion;
}
if (HasImplicitNumericConversion(source, destination))
{
return Conversion.ImplicitNumeric;
}

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

if (HasImplicitReferenceConversion(source, destination, ref useSiteInfo))
{
return Conversion.ImplicitReference;
}
if (source is FunctionTypeSymbol)
{
Debug.Assert(false);
return Conversion.NoConversion;
}

if (HasBoxingConversion(source, destination, ref useSiteInfo))
{
return Conversion.Boxing;
}
if (HasImplicitReferenceConversion(source, destination, ref useSiteInfo))
{
return Conversion.ImplicitReference;
}

if (HasImplicitPointerToVoidConversion(source, destination))
{
return Conversion.PointerToVoid;
}
if (HasBoxingConversion(source, destination, ref useSiteInfo))
{
return Conversion.Boxing;
}

if (HasImplicitPointerConversion(source, destination, ref useSiteInfo))
{
return Conversion.ImplicitPointer;
}
if (HasImplicitPointerToVoidConversion(source, destination))
{
return Conversion.PointerToVoid;
}

var tupleConversion = ClassifyImplicitTupleConversion(source, destination, ref useSiteInfo);
if (tupleConversion.Exists)
{
return tupleConversion;
}
if (HasImplicitPointerConversion(source, destination, ref useSiteInfo))
{
return Conversion.ImplicitPointer;
}

return Conversion.NoConversion;
var tupleConversion = ClassifyImplicitTupleConversion(source, destination, ref useSiteInfo);
if (tupleConversion.Exists)
{
return tupleConversion;
}

return Conversion.NoConversion;
}
}

private Conversion ClassifyImplicitBuiltInConversionSlow(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
Expand Down Expand Up @@ -867,7 +957,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 +1278,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 +1296,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 @@ -1227,7 +1318,6 @@ private Conversion ClassifyExplicitOnlyConversionFromExpression(BoundExpression
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 @@ -2576,7 +2666,7 @@ private bool HasImplicitFunctionTypeConversion(FunctionTypeSymbol source, TypeSy
{
if (destination is FunctionTypeSymbol destinationFunctionType)
{
return HasImplicitSignatureConversion(source, destinationFunctionType, ref useSiteInfo);
return HasImplicitFunctionTypeToFunctionTypeConversion(source, destinationFunctionType, ref useSiteInfo);
}

return IsValidFunctionTypeConversionTarget(destination, ref useSiteInfo);
Expand Down Expand Up @@ -2604,7 +2694,7 @@ internal bool IsValidFunctionTypeConversionTarget(TypeSymbol destination, ref Co
return false;
}

private bool HasImplicitSignatureConversion(FunctionTypeSymbol sourceType, FunctionTypeSymbol destinationType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
private bool HasImplicitFunctionTypeToFunctionTypeConversion(FunctionTypeSymbol sourceType, FunctionTypeSymbol destinationType, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var sourceDelegate = sourceType.GetInternalDelegateType();
var destinationDelegate = destinationType.GetInternalDelegateType();
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/UserDefinedImplicitConversions.cs
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,15 +606,15 @@ private void MakeExplicitParameterTypeInferences(BoundExpression argument, TypeW
!MakeExplicitParameterTypeInferences((BoundTupleLiteral)argument, target, kind, ref useSiteInfo))
{
// Either the argument is not a tuple literal, or we were unable to do the inference from its elements, let's try to infer from argument type
if (IsReallyAType(argument.GetTypeOrFunctionType()))
var argumentType = _extensions.GetTypeWithAnnotations(argument);
if (IsReallyAType(argumentType.Type))
{
ExactOrBoundsInference(kind, _extensions.GetTypeWithAnnotations(argument), target, ref useSiteInfo);
ExactOrBoundsInference(kind, argumentType, target, ref useSiteInfo);
}
else if (IsUnfixedTypeParameter(target) && kind is ExactOrBoundsKind.LowerBound)
{
var ordinal = ((TypeParameterSymbol)target.Type).Ordinal;
var typeWithAnnotations = _extensions.GetTypeWithAnnotations(argument);
_nullableAnnotationLowerBounds[ordinal] = _nullableAnnotationLowerBounds[ordinal].Join(typeWithAnnotations.NullableAnnotation);
_nullableAnnotationLowerBounds[ordinal] = _nullableAnnotationLowerBounds[ordinal].Join(argumentType.NullableAnnotation);
}
}
}
Expand Down Expand Up @@ -2793,7 +2793,7 @@ private static bool ImplicitConversionExists(TypeWithAnnotations sourceWithAnnot
return false;
}

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

Expand Down
14 changes: 11 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/UsingStatementBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,17 @@ 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);
var conversions = originalBinder.Conversions;
if (fromExpression)
{
Debug.Assert(expressionOpt is { });
return conversions.ClassifyImplicitConversionFromExpression(expressionOpt, targetInterface, ref useSiteInfo);
}
else
{
Debug.Assert(declarationTypeOpt is { });
return conversions.ClassifyImplicitConversionFromType(declarationTypeOpt, targetInterface, ref useSiteInfo);
}
}

TypeSymbol getDisposableInterface(bool isAsync)
Expand Down
Loading