Do not include function type conversions in user-defined conversions #56416
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cston
commented
Sep 15, 2021
| @@ -642,6 +642,9 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind) | |||
|
|
|||
| // Added for C# 7.1 | |||
| case ConversionKind.DefaultLiteral: | |||
|
|
|||
| // Added for C# 10. | |||
| case ConversionKind.FunctionType: | |||
cston
force-pushed
the
56407
branch
3 times, most recently
from
c9825eb to
958c08f
Compare
cston
changed the title
…ups to Delegate, Expression, and base types
cston
force-pushed
the
56407
branch
2 times, most recently
from
68be73b to
f8c5df3
Compare
cston
changed the title
AlekseyTs
reviewed
Sep 21, 2021
| @@ -118,6 +119,13 @@ public Conversion ClassifyImplicitConversionFromExpression(BoundExpression sourc | |||
| } | |||
| } | |||
| } | |||
| else if (sourceExpression.GetFunctionType() is { }) | |||
| { | |||
| if (IsValidFunctionTypeConversionTarget(destination, ref useSiteInfo)) | |||
There was a problem hiding this comment.
The only code paths that should have to handle conversions between two function types are BestTypeInferrer and MethodTypeInferrer in cases where "bounds" are merged. Those code paths use ClassifyImplicitConversionFromTypeOrImplicitFunctionTypeConversion().
There was a problem hiding this comment.
Added handling of conversions between function types.
AlekseyTs
reviewed
Sep 21, 2021
| } | ||
|
|
||
| Debug.Assert(false); | ||
| return Conversion.NoConversion; |
There was a problem hiding this comment.
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.
AlekseyTs
reviewed
Sep 21, 2021
| return conversion; | ||
| } | ||
|
|
||
| private Conversion ClassifyStandardImplicitConversionInternal(TypeSymbol source, TypeSymbol destination, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo) |
AlekseyTs
reviewed
Sep 21, 2021
| { | ||
| Debug.Assert((object)source != null); | ||
| Debug.Assert((object)destination != null); | ||
| Debug.Assert(source is not FunctionTypeSymbol); |
AlekseyTs
reviewed
Sep 21, 2021
| { | ||
| if (IsValidFunctionTypeConversionTarget(destination, ref useSiteInfo)) | ||
| { | ||
| return Conversion.FunctionType; |
AlekseyTs
reviewed
Sep 21, 2021
| @@ -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) : | |||
|
Done with review pass (commit 3) |
ghost
modified the milestones:
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes "function type conversions" from the set of "standard conversions" so that function type conversions (from the inferred type of a lambda expression or method group to
Delegate,Expressionor a base type) are not considered for user-defined conversions.Fixes #56407
Relates to test plan #52192