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.

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

Implement type merging in deconstruction with tuple literal #12526

Merged
merged 5 commits into from
Jul 20, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
20 changes: 14 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CShar
// Let's fix the literal up by figuring out its type
// For declarations, that means merging type information from the LHS and RHS
// For assignments, only the LHS side matters since it is necessarily typed
TypeSymbol lhsAsTuple = MakeMergedTupleType(checkedVariables, (BoundTupleLiteral)boundRHS, node, diagnostics, Compilation);
TypeSymbol lhsAsTuple = MakeMergedTupleType(checkedVariables, (BoundTupleLiteral)boundRHS, node, Compilation, diagnostics);
if (lhsAsTuple != null)
Copy link
Member

Choose a reason for hiding this comment

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

(object)lhsAsTuple != null

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. I'm still not quite sure when that is useful. Is it for certain types only (such as Symbols)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll queue that and your other suggestion in next PR.

Copy link
Member

Choose a reason for hiding this comment

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

Only for types that define operator==.

{
boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics);
Expand Down Expand Up @@ -294,7 +294,7 @@ private bool DeconstructOrAssignOutputs(
/// For cases where the RHS of a deconstruction-assignment is a tuple literal, the type information from the LHS determines the merged type, since all variables have a type.
Copy link
Member

Choose a reason for hiding this comment

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

Where in the method do we check for declaration vs. assignment?

Copy link
Member Author

Choose a reason for hiding this comment

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

We never have to check explicitly. The LHS in an assignment is always completely typed (no vars).

/// Returns null if a merged tuple type could not be fabricated.
/// </summary>
private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariable> lhsVariables, BoundTupleLiteral rhsLiteral, CSharpSyntaxNode syntax, DiagnosticBag diagnostics, CSharpCompilation compilation)
private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariable> lhsVariables, BoundTupleLiteral rhsLiteral, CSharpSyntaxNode syntax, CSharpCompilation compilation, DiagnosticBag diagnostics)
{
int leftLength = lhsVariables.Count;
int rightLength = rhsLiteral.Arguments.Length;
Expand All @@ -313,28 +313,36 @@ private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariabl
if (element.Kind == BoundKind.TupleLiteral)
{
// (variables) on the left and (elements) on the right
mergedType = MakeMergedTupleType(variable.NestedVariables, (BoundTupleLiteral)element, syntax, diagnostics, compilation);
mergedType = MakeMergedTupleType(variable.NestedVariables, (BoundTupleLiteral)element, syntax, compilation, diagnostics);
}
else if (element.Type == null)
else if ((object)mergedType == null)
{
// (variables) on the left and null on the right
Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, element.Syntax);
}
}
else
{
if (variable.Single.Type != null)
if ((object)variable.Single.Type != null)
{
// typed-variable on the left
mergedType = variable.Single.Type;
}
else if (element.Type == null)
else if ((object)mergedType == null)
{
// typeless-variable on the left and typeless-element on the right
Error(diagnostics, ErrorCode.ERR_DeconstructCouldNotInferMergedType, syntax, variable.Syntax, element.Syntax);
}
}
}
else
{
if ((object)element.Type == null)
Copy link
Member

Choose a reason for hiding this comment

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

mergedType

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. Fixed

{
// a typeless element on the right, matching no variable on the left
Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, element.Syntax);
}
}

typesBuilder.Add(mergedType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3168,22 +3168,26 @@ static void Main()
}

[Fact]
public void TypeMergingWithTooManyRightVariables()
public void TypeMergingWithTooManyRightElements()
{
string source = @"
class C
{
static void Main()
{
(string x1, var x2) = (null, ""hello"", 3);
(string x1, var y1) = (null, ""hello"", 3);
(string x2, var y2) = (null, ""hello"", null);
}
}
";
var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef });
comp.VerifyDiagnostics(
// (6,9): error CS8211: Cannot deconstruct a tuple of '3' elements into '2' variables.
// (string x1, var x2) = (null, "hello", 3);
Diagnostic(ErrorCode.ERR_DeconstructWrongCardinality, @"(string x1, var x2) = (null, ""hello"", 3);").WithArguments("3", "2").WithLocation(6, 9)
// (string x1, var y1) = (null, "hello", 3);
Diagnostic(ErrorCode.ERR_DeconstructWrongCardinality, @"(string x1, var y1) = (null, ""hello"", 3);").WithArguments("3", "2").WithLocation(6, 9),
// (7,47): error CS8210: Deconstruct assignment requires an expression with a type on the right-hand-side.
// (string x2, var y2) = (null, "hello", null);
Diagnostic(ErrorCode.ERR_DeconstructRequiresExpression, "null").WithLocation(7, 47)
);
}

Expand Down