Added initial support for explicit tuple conversions #12068
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1716,60 +1716,9 @@ private BoundExpression BindCastCore(ExpressionSyntax node, BoundExpression oper | |
| HashSet<DiagnosticInfo> useSiteDiagnostics = null; | ||
| Conversion conversion = this.Conversions.ClassifyConversionForCast(operand, targetType, ref useSiteDiagnostics); | ||
| diagnostics.Add(node, useSiteDiagnostics); | ||
| if (operand.HasAnyErrors || targetType.IsErrorType() || !conversion.IsValid || targetType.IsStatic) | ||
| { | ||
| GenerateExplicitConversionErrors(diagnostics, node, conversion, operand, targetType); | ||
|
|
||
| return new BoundConversion( | ||
| node, | ||
|
|
@@ -1785,6 +1734,128 @@ private BoundExpression BindCastCore(ExpressionSyntax node, BoundExpression oper | |
| return CreateConversion(node, operand, conversion, isCast: true, wasCompilerGenerated: wasCompilerGenerated, destination: targetType, diagnostics: diagnostics); | ||
| } | ||
|
|
||
| private void GenerateExplicitConversionErrors( | ||
| DiagnosticBag diagnostics, | ||
| CSharpSyntaxNode syntax, | ||
| Conversion conversion, | ||
| BoundExpression operand, | ||
| TypeSymbol targetType) | ||
| { | ||
| // Make sure that errors within the unbound lambda don't get lost. | ||
| if (operand.Kind == BoundKind.UnboundLambda) | ||
| { | ||
| GenerateAnonymousFunctionConversionError(diagnostics, operand.Syntax, (UnboundLambda)operand, targetType); | ||
| return; | ||
| } | ||
|
|
||
| if (operand.HasAnyErrors || targetType.IsErrorType()) | ||
| { | ||
| // an error has already been reported elsewhere | ||
| return; | ||
| } | ||
|
|
||
| if (targetType.IsStatic) | ||
| { | ||
| // The specification states in the section titled "Referencing Static | ||
|
There was a problem hiding this comment. If you really wanted to channel Eric you could quote it here :) There was a problem hiding this comment. These comments are actually preexisting ones. I have just moved this code into a helper so that I could call it recursively if needed. Yes, sounds like Eric. :-) |
||
| // Class Types" that it is always illegal to have a static class in a | ||
| // cast operator. | ||
| diagnostics.Add(ErrorCode.ERR_ConvertToStaticClass, syntax.Location, targetType); | ||
| return; | ||
| } | ||
|
|
||
| if (!targetType.IsReferenceType && !targetType.IsNullableType() && operand.IsLiteralNull()) | ||
|
There was a problem hiding this comment. What happens to There was a problem hiding this comment. IsReferenceType returns true for dynamic. In this case dynamic can be null so a cast (dynamic)null will succeed. In fact dynamic is implicitly convertable to and from any type so it will always work in this context. There was a problem hiding this comment. Interestingly "dynamic" is contextual and is not recognized as a type in a context of a tuple. So that blocks this scenario for now. |
||
| { | ||
| diagnostics.Add(ErrorCode.ERR_ValueCantBeNull, syntax.Location, targetType); | ||
| return; | ||
| } | ||
|
|
||
| if (conversion.ResultKind == LookupResultKind.OverloadResolutionFailure) | ||
| { | ||
| Debug.Assert(conversion.IsUserDefined); | ||
|
|
||
| ImmutableArray<MethodSymbol> originalUserDefinedConversions = conversion.OriginalUserDefinedConversions; | ||
| if (originalUserDefinedConversions.Length > 1) | ||
| { | ||
| diagnostics.Add(ErrorCode.ERR_AmbigUDConv, syntax.Location, originalUserDefinedConversions[0], originalUserDefinedConversions[1], operand.Type, targetType); | ||
| } | ||
| else | ||
| { | ||
| Debug.Assert(originalUserDefinedConversions.Length == 0, | ||
| "How can there be exactly one applicable user-defined conversion if the conversion doesn't exist?"); | ||
| SymbolDistinguisher distinguisher1 = new SymbolDistinguisher(this.Compilation, operand.Type, targetType); | ||
| diagnostics.Add(ErrorCode.ERR_NoExplicitConv, syntax.Location, distinguisher1.First, distinguisher1.Second); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // TODO: report more specific diagnostics here for failed method group conversions | ||
| if (operand.Kind == BoundKind.MethodGroup) | ||
| { | ||
| diagnostics.Add(ErrorCode.ERR_NoExplicitConv, syntax.Location, MessageID.IDS_SK_METHOD.Localize(), targetType); | ||
| return; | ||
| } | ||
|
|
||
| if (operand.Kind == BoundKind.TupleLiteral) | ||
| { | ||
| var tuple = (BoundTupleLiteral)operand; | ||
| var targetElementTypes = default(ImmutableArray<TypeSymbol>); | ||
|
|
||
| // source does not have a type. | ||
| // Such conversions could only happen via explicit tuple literal conversions | ||
| // which are currently not supported. | ||
| // See: https://github.com/dotnet/roslyn/issues/11804 | ||
| if ((object)tuple.Type == null) | ||
| { | ||
| Error(diagnostics, ErrorCode.ERR_NoExplicitConv, syntax, tuple.Display, targetType); | ||
| return; | ||
| } | ||
|
|
||
| // If target is a tuple or compatible type with the same number of elements, | ||
| // report errors for tuple arguments that failed to convert, which would be more useful. | ||
| if (targetType.TryGetElementTypesIfTupleOrCompatible(out targetElementTypes) && | ||
| targetElementTypes.Length == tuple.Arguments.Length) | ||
| { | ||
| GenerateExplicitConversionErrorsForTupleLiteralArguments(diagnostics, syntax, tuple.Arguments, targetElementTypes); | ||
| return; | ||
| } | ||
|
|
||
| // Otherwise it is just a regular conversion failure from T1 to T2. | ||
| } | ||
|
|
||
| Debug.Assert((object)operand.Type != null); | ||
| SymbolDistinguisher distinguisher = new SymbolDistinguisher(this.Compilation, operand.Type, targetType); | ||
| diagnostics.Add(ErrorCode.ERR_NoExplicitConv, syntax.Location, distinguisher.First, distinguisher.Second); | ||
| } | ||
|
|
||
| private void GenerateExplicitConversionErrorsForTupleLiteralArguments( | ||
| DiagnosticBag diagnostics, | ||
| CSharpSyntaxNode syntax, | ||
| ImmutableArray<BoundExpression> tupleArguments, | ||
| ImmutableArray<TypeSymbol> targetElementTypes) | ||
| { | ||
| var argLength = tupleArguments.Length; | ||
|
|
||
| // report all leaf elements of the tuple literal that failed to convert | ||
| // NOTE: we are not responsible for reporting use site errors here, just the failed leaf conversions. | ||
| // By the time we get here we have done analysis and know we have failed the cast in general, and diagnostics collected in the process is already in the bag. | ||
| // The only thing left is to form a diagnostics about the actually failing conversion(s). | ||
| // This whole method does not itself collect any usesite diagnostics. Its only purpose is to produce an error better than "conversion failed here" | ||
| HashSet<DiagnosticInfo> usDiagsUnused = null; | ||
|
|
||
| for (int i = 0; i < targetElementTypes.Length; i++) | ||
| { | ||
| var argument = tupleArguments[i]; | ||
| var targetElementType = targetElementTypes[i]; | ||
|
|
||
| var elementConversion = Conversions.ClassifyConversionFromType(argument.Type, targetElementType, ref usDiagsUnused); | ||
| if (!elementConversion.IsValid) | ||
| { | ||
| GenerateExplicitConversionErrors(diagnostics, argument.Syntax, elementConversion, argument, targetElementType); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private BoundExpression BindOriginal(OriginalExpressionSyntax syntax, DiagnosticBag diagnostics) | ||
| { | ||
| var containingMethod = (MethodSymbol)this.ContainingMember(); | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,10 @@ | ||
| // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using Microsoft.CodeAnalysis.CSharp.Symbols; | ||
| using Roslyn.Utilities; | ||
| using System; | ||
| using System.Collections.Immutable; | ||
| using System.Diagnostics; | ||
|
There was a problem hiding this comment. I think the system ones are supposed to go at the top. |
||
|
|
||
| namespace Microsoft.CodeAnalysis.CSharp | ||
| { | ||
|
|
@@ -53,7 +51,10 @@ public struct Conversion : IEquatable<Conversion> | |
| private const byte IsExtensionMethodMask = 1 << 0; | ||
| private const byte IsArrayIndexMask = 1 << 1; | ||
|
|
||
| private readonly Conversion[] _nestedConversionsOpt; | ||
|
There was a problem hiding this comment. Could not make this ImmutableArray because of a false-circularity bug in CLR loader. |
||
|
|
||
| private Conversion(ConversionKind kind, bool isExtensionMethod, bool isArrayIndex, UserDefinedConversionResult conversionResult, MethodSymbol methodGroupConversionMethod) | ||
| : this() | ||
| { | ||
| _kind = kind; | ||
| _conversionResult = conversionResult; | ||
|
|
@@ -81,6 +82,13 @@ internal Conversion(ConversionKind kind) | |
| this._kind = kind; | ||
| } | ||
|
|
||
| internal Conversion(ConversionKind kind, Conversion[] nestedConversions) | ||
| : this() | ||
| { | ||
| this._kind = kind; | ||
| this._nestedConversionsOpt = nestedConversions; | ||
| } | ||
|
|
||
| internal ConversionKind Kind | ||
| { | ||
| get | ||
|
|
@@ -128,7 +136,28 @@ internal bool IsValid | |
| { | ||
| get | ||
| { | ||
| if (!this.Exists) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (_nestedConversionsOpt != null) | ||
| { | ||
| foreach (var conv in _nestedConversionsOpt) | ||
| { | ||
| if (!conv.IsValid) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| Debug.Assert(!this.IsUserDefined); | ||
| return true; | ||
| } | ||
|
|
||
| return !this.IsUserDefined || | ||
| (object)this.Method != null || | ||
| _conversionResult.Kind == UserDefinedConversionResultKind.Valid; | ||
| } | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get conversion classification for elements from nestedConversions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. That is the next change that I mentioned in the description. We might not need to be classifying anything at all when lowering conversions.