-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
Combine deconstruction assignment and declaration, and support discards. #15548
Changes from all commits
1da1e53
1003470
887d224
ac9c1a3
cc42c17
711b6b0
f90f62b
c4d4991
c5e4534
3e0d9db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -647,6 +647,9 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, Diagnostic | |
CreateErrorType("ref")); | ||
} | ||
|
||
case SyntaxKind.DeclarationExpression: | ||
return BindDeclarationExpression((DeclarationExpressionSyntax)node, diagnostics); | ||
|
||
default: | ||
// NOTE: We could probably throw an exception here, but it's conceivable | ||
// that a non-parser syntax tree could reach this point with an unexpected | ||
|
@@ -703,6 +706,62 @@ private static bool IsThrowExpressionInProperContext(ThrowExpressionSyntax node) | |
} | ||
} | ||
|
||
// Bind a declaration expression where it isn't permitted. | ||
private BoundExpression BindDeclarationExpression(DeclarationExpressionSyntax node, DiagnosticBag diagnostics) | ||
{ | ||
// This is an error, as declaration expressions are handled specially in every context in which | ||
// they are permitted. So we have a context in which they are *not* permitted. Nevertheless, we | ||
// bind it and then give one nice message. | ||
|
||
bool isVar; | ||
bool isConst = false; | ||
AliasSymbol alias; | ||
TypeSymbol declType = BindVariableType(node.Designation, diagnostics, node.Type, ref isConst, out isVar, out alias); | ||
Error(diagnostics, ErrorCode.ERR_DeclarationExpressionNotPermitted, node); | ||
return BindDeclarationVariables(declType, node.Designation, diagnostics); | ||
} | ||
|
||
/// <summary> | ||
/// Bind a declaration variable where it isn't permitted. The caller is expected to produce a diagnostic. | ||
/// </summary> | ||
private BoundExpression BindDeclarationVariables(TypeSymbol declType, VariableDesignationSyntax node, DiagnosticBag diagnostics) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I am not sure why do we want to collect any additional errors besides the one already reported by the caller. It feels like that will only create more noise. #Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could drop the diagnostics from this call on the floor, but I believe they will be useful to the user when (1) the user refactors their code to use local declarations, or (2) we begin allowing this under more constrained circumstances. If we see complaints about cascaded diagnostics, we should consider refactoring. I'm fine with it as it stands. #ByDesign |
||
{ | ||
declType = declType ?? CreateErrorType("var"); | ||
switch (node.Kind()) | ||
{ | ||
case SyntaxKind.SingleVariableDesignation: | ||
{ | ||
var single = (SingleVariableDesignationSyntax)node; | ||
var result = BindDeconstructionVariable(declType, single, diagnostics); | ||
return result; | ||
} | ||
case SyntaxKind.DiscardDesignation: | ||
{ | ||
var discarded = (DiscardDesignationSyntax)node; | ||
return BindDiscardExpression(discarded, declType); | ||
} | ||
case SyntaxKind.ParenthesizedVariableDesignation: | ||
{ | ||
var tuple = (ParenthesizedVariableDesignationSyntax)node; | ||
var builder = ArrayBuilder<BoundExpression>.GetInstance(tuple.Variables.Count); | ||
foreach (var n in tuple.Variables) | ||
{ | ||
builder.Add(BindDeclarationVariables(declType, n, diagnostics)); | ||
} | ||
var subExpressions = builder.ToImmutableAndFree(); | ||
var tupleType = TupleTypeSymbol.Create( | ||
null, | ||
subExpressions.SelectAsArray(e => e.Type), | ||
default(ImmutableArray<Location>), | ||
default(ImmutableArray<string>), | ||
Compilation); | ||
return new BoundTupleLiteral(node, default(ImmutableArray<string>), subExpressions, tupleType); | ||
} | ||
default: | ||
throw ExceptionUtilities.UnexpectedValue(node.Kind()); | ||
} | ||
} | ||
|
||
private BoundExpression BindTupleExpression(TupleExpressionSyntax node, DiagnosticBag diagnostics) | ||
{ | ||
SeparatedSyntaxList<ArgumentSyntax> arguments = node.Arguments; | ||
|
@@ -1144,7 +1203,7 @@ private BoundExpression BindIdentifier( | |
{ | ||
if (node.IsKind(SyntaxKind.IdentifierName) && FallBackOnDiscard((IdentifierNameSyntax)node, diagnostics)) | ||
{ | ||
return new BoundDiscardedExpression(node, type: null); | ||
return new BoundDiscardExpression(node, type: null); | ||
} | ||
|
||
// Otherwise, the simple-name is undefined and a compile-time error occurs. | ||
|
@@ -2127,15 +2186,15 @@ private BoundExpression BindOutVariableArgument(DeclarationExpressionSyntax decl | |
VariableDesignationSyntax designation = declarationExpression.Designation; | ||
switch (designation.Kind()) | ||
{ | ||
case SyntaxKind.DiscardedDesignation: | ||
case SyntaxKind.DiscardDesignation: | ||
{ | ||
bool isVar; | ||
bool isConst = false; | ||
AliasSymbol alias; | ||
TypeSymbol declType = BindVariableType(designation, diagnostics, typeSyntax, ref isConst, out isVar, out alias); | ||
Debug.Assert(isVar == ((object)declType == null)); | ||
|
||
return new BoundDiscardedExpression((DiscardedDesignationSyntax)designation, declType); | ||
return new BoundDiscardExpression((DiscardDesignationSyntax)designation, declType); | ||
} | ||
case SyntaxKind.SingleVariableDesignation: | ||
return BindOutVariableDeclarationArgument(declarationExpression, diagnostics); | ||
|
@@ -2402,11 +2461,11 @@ private void CoerceArguments<TMember>( | |
TypeSymbol parameterType = GetCorrespondingParameterType(ref result, parameters, arg); | ||
arguments[arg] = ((OutDeconstructVarPendingInference)argument).SetInferredType(parameterType, success: true); | ||
} | ||
else if (argument.Kind == BoundKind.DiscardedExpression && !argument.HasExpressionType()) | ||
else if (argument.Kind == BoundKind.DiscardExpression && !argument.HasExpressionType()) | ||
{ | ||
TypeSymbol parameterType = GetCorrespondingParameterType(ref result, parameters, arg); | ||
Debug.Assert((object)parameterType != null); | ||
arguments[arg] = ((BoundDiscardedExpression)argument).SetInferredType(parameterType); | ||
arguments[arg] = ((BoundDiscardExpression)argument).SetInferredType(parameterType); | ||
} | ||
} | ||
} | ||
|
@@ -6234,9 +6293,9 @@ private BoundExpression ConvertToArrayIndex(BoundExpression index, SyntaxNode no | |
{ | ||
return ((OutVariablePendingInference)index).FailInference(this, diagnostics); | ||
} | ||
else if (index.Kind == BoundKind.DiscardedExpression && !index.HasExpressionType()) | ||
else if (index.Kind == BoundKind.DiscardExpression && !index.HasExpressionType()) | ||
{ | ||
return ((BoundDiscardedExpression)index).FailInference(this, diagnostics); | ||
return ((BoundDiscardExpression)index).FailInference(this, diagnostics); | ||
} | ||
|
||
var result = | ||
|
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.
Is this comment correct? From the body of the method, it's not clear that the declaration variable isn't permitted. #ByDesign
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.
While it is true that the code implements them, this is used only to bind them in contexts in which they are not currently permitted, via the method above which gives an error message. As you can see, the type inference (line 735 below) is short-circuited to produce an error type.
In reply to: 89843255 [](ancestors = 89843255)