Skip to content

Commit

Permalink
Combine deconstruction assignment and declaration, and support discar…
Browse files Browse the repository at this point in the history
…ds. (#15548)

* Combine deconstruction assignment and declaration, and support discards.

- Combine deconstruction assignment and declaration, and support discards.
- Add wildcards.work.md to track outstanding work.
- Bind each type syntax once in a deconstruction.
- Because tuples may contain declarations, adjust lambda disambiguation
  and adjust parsing of argument lists.
- Diagnose tuple element names on the left of a deconstruction.
- Add relational operators to disambiguating tokens in 7.5.4.2

* Disallow deconstruction declarations except at the statement level.
This is now a semantic restriction (until we decide to remove it).

* Revise logic to detect `var` in a declaration expression.
Plus other changes per code review.

* Add a test that GetTypeInfo on a discard expression doesn't crash.
* Small changes per code review.
* Add (skipped) test for var invocation in parens.
* Rename "Discarded" to "Discard"
* Changes recommended via code review.
* Minor changes to the handling of declaration expressions per code review.
* Addressing blocking feedback while Neal is OOF

Fixes #14794
Fixes #14832
  • Loading branch information
gafter authored and jcouv committed Dec 12, 2016
1 parent 2ba627f commit 3806ef8
Show file tree
Hide file tree
Showing 81 changed files with 4,020 additions and 1,609 deletions.
32 changes: 17 additions & 15 deletions docs/features/wildcards.work.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,28 @@ work items remaining for wildcards
- [ ] Gather together a specification document
- [ ] Language behavior (e.g. [this](https://github.com/dotnet/roslyn/issues/14862) and [this](https://github.com/dotnet/roslyn/issues/14794) and [this](https://github.com/dotnet/roslyn/issues/14832))
- [ ] [SemanticModel behavior](https://gist.github.com/gafter/ab10e413efe3a066209cbf14cb874988) (see also [here](https://gist.github.com/gafter/37305d619bd04511f4f66b86f6f2d3a5))
- [ ] Warnings (for non-wildcard expression variables declared but not used)
- [ ] Debugging
- [ ] Warnings (for expression variables declared but not used)
- [x] Debugging (value discarded is not visible in debugger)

### Compiler
- [x] Syntax model changes
- [ ] Symbol changes
- [ ] Parsing for the short-form wildcard pattern `_`
- [ ] Implement binding of wildcards
- [ ] In a pattern
- [ ] In a deconstruction declaration
- [ ] In a deconstruction assignment expression
- [x] Symbol changes
- [x] Parsing for the short-form wildcard pattern `_`
- [x] Implement binding of wildcards
- [x] In a pattern
- [x] In a deconstruction declaration
- [x] In a deconstruction assignment expression
- [ ] In an out argument (in every argument context)
- [ ] Both the long form `var _` and the short form `_`
- [x] Both the long form `var _` and the short form `_`
- [x] Type inference for wildcards in each context
- [ ] Implement semantic model changes
- [ ] Type inference for wildcards in each context
- [ ] Implement lowering of wildcards
- [ ] In a pattern
- [ ] In a deconstruction declaration
- [ ] In a deconstruction assignment expression
- [ ] In an out argument (in every argument context)
- [ ] `GetTypeInfo` of a wildcard expression `_` should be the type of the discarded value
- [ ] `GetSymbolInfo` of a wildcard expression `_` should be an `IDiscardedSymbol`
- [x] Implement lowering of wildcards
- [x] In a pattern
- [x] In a deconstruction declaration
- [x] In a deconstruction assignment expression
- [ ] In an out argument (in each argument context)

### Testing
- [ ] Symbol tests
Expand Down
209 changes: 130 additions & 79 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

Large diffs are not rendered by default.

73 changes: 66 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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 =
Expand Down
16 changes: 8 additions & 8 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ internal BoundExpression MakeInvocationExpression(
var analyzedArguments = AnalyzedArguments.GetInstance();
Debug.Assert(!args.Any(e => e.Kind == BoundKind.OutVariablePendingInference ||
e.Kind == BoundKind.OutDeconstructVarPendingInference ||
e.Kind == BoundKind.DiscardedExpression && !e.HasExpressionType()));
e.Kind == BoundKind.DiscardExpression && !e.HasExpressionType()));
analyzedArguments.Arguments.AddRange(args);
BoundExpression result = BindInvocationExpression(
node, node, methodName, boundExpression, analyzedArguments, diagnostics, queryClause,
Expand Down Expand Up @@ -339,7 +339,7 @@ private ImmutableArray<BoundExpression> BuildArgumentsForDynamicInvocation(Analy
Debug.Assert(arguments.Arguments[i].Kind != BoundKind.OutDeconstructVarPendingInference);

if (arguments.Arguments[i].Kind == BoundKind.OutVariablePendingInference ||
arguments.Arguments[i].Kind == BoundKind.DiscardedExpression && !arguments.Arguments[i].HasExpressionType())
arguments.Arguments[i].Kind == BoundKind.DiscardExpression && !arguments.Arguments[i].HasExpressionType())
{
var builder = ArrayBuilder<BoundExpression>.GetInstance(arguments.Arguments.Count);
builder.AddRange(arguments.Arguments);
Expand All @@ -352,9 +352,9 @@ private ImmutableArray<BoundExpression> BuildArgumentsForDynamicInvocation(Analy
{
builder[i] = ((OutVariablePendingInference)argument).FailInference(this, diagnostics);
}
else if (argument.Kind == BoundKind.DiscardedExpression && !argument.HasExpressionType())
else if (argument.Kind == BoundKind.DiscardExpression && !argument.HasExpressionType())
{
builder[i] = ((BoundDiscardedExpression)argument).FailInference(this, diagnostics);
builder[i] = ((BoundDiscardExpression)argument).FailInference(this, diagnostics);
}

i++;
Expand Down Expand Up @@ -1192,7 +1192,7 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
break;
}
case BoundKind.OutVariablePendingInference:
case BoundKind.DiscardedExpression:
case BoundKind.DiscardExpression:
{
if (argument.HasExpressionType())
{
Expand Down Expand Up @@ -1230,15 +1230,15 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
newArguments[i] = ((OutVariablePendingInference)argument).SetInferredType(candidateType, null);
}
}
else if (argument.Kind == BoundKind.DiscardedExpression)
else if (argument.Kind == BoundKind.DiscardExpression)
{
if ((object)candidateType == null)
{
newArguments[i] = ((BoundDiscardedExpression)argument).FailInference(this, null);
newArguments[i] = ((BoundDiscardExpression)argument).FailInference(this, null);
}
else
{
newArguments[i] = ((BoundDiscardedExpression)argument).SetInferredType(candidateType);
newArguments[i] = ((BoundDiscardExpression)argument).SetInferredType(candidateType);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ private BoundPattern BindDeclarationPattern(
{
case SyntaxKind.SingleVariableDesignation:
break;
case SyntaxKind.DiscardedDesignation:
case SyntaxKind.DiscardDesignation:
return new BoundDeclarationPattern(node, null, boundDeclType, isVar, hasErrors);
default:
throw ExceptionUtilities.UnexpectedValue(node.Designation.Kind());
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ private TypeSymbol BindVariableType(CSharpSyntaxNode declarationNode, Diagnostic
declarationNode is VariableDesignationSyntax ||
declarationNode.Kind() == SyntaxKind.VariableDeclaration ||
declarationNode.Kind() == SyntaxKind.DeclarationExpression ||
declarationNode.Kind() == SyntaxKind.DiscardedDesignation);
declarationNode.Kind() == SyntaxKind.DiscardDesignation);

// If the type is "var" then suppress errors when binding it. "var" might be a legal type
// or it might not; if it is not then we do not want to report an error. If it is, then
Expand Down Expand Up @@ -1713,15 +1713,15 @@ private BoundExpression BindAssignment(AssignmentExpressionSyntax node, Diagnost
var op1 = BindValue(node.Left, diagnostics, BindValueKind.Assignment); // , BIND_MEMBERSET);
var op2 = BindValue(node.Right, diagnostics, BindValueKind.RValue); // , BIND_RVALUEREQUIRED);

if (op1.Kind == BoundKind.DiscardedExpression)
if (op1.Kind == BoundKind.DiscardExpression)
{
op1 = InferTypeForDiscard((BoundDiscardedExpression)op1, op2, diagnostics);
op1 = InferTypeForDiscard((BoundDiscardExpression)op1, op2, diagnostics);
}

return BindAssignment(node, op1, op2, diagnostics);
}

private BoundExpression InferTypeForDiscard(BoundDiscardedExpression op1, BoundExpression op2, DiagnosticBag diagnostics)
private BoundExpression InferTypeForDiscard(BoundDiscardExpression op1, BoundExpression op2, DiagnosticBag diagnostics)
{
if (op2.Type == null)
{
Expand Down Expand Up @@ -1865,7 +1865,7 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
Debug.Assert(valueKind == BindValueKind.RefOrOut);
return expr;

case BoundKind.DiscardedExpression:
case BoundKind.DiscardExpression:
Debug.Assert(valueKind == BindValueKind.Assignment || valueKind == BindValueKind.RefOrOut);
return expr;
}
Expand Down
Loading

0 comments on commit 3806ef8

Please sign in to comment.