From 5c8da032f0799a9c12e905aa8e095beab2f4e919 Mon Sep 17 00:00:00 2001 From: Julien Date: Tue, 19 Jul 2016 19:09:17 -0700 Subject: [PATCH] Implement type merging in deconstruction with tuple literal (#12526) --- .../Portable/Binder/Binder_Deconstruct.cs | 94 ++++++-- .../Portable/CSharpResources.Designer.cs | 9 + .../CSharp/Portable/CSharpResources.resx | 3 + .../CSharp/Portable/Errors/ErrorCode.cs | 1 + .../Emit/CodeGen/CodeGenDeconstructTests.cs | 204 +++++++++++++++++- 5 files changed, 284 insertions(+), 27 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs index d7b34dda92b0d..f06fc276358e1 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs @@ -56,16 +56,25 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CShar if ((object)boundRHS.Type == null) { - if (boundRHS.Kind == BoundKind.TupleLiteral && !isDeclaration) + if (boundRHS.Kind == BoundKind.TupleLiteral) { - // tuple literal without type such as `(null, null)`, let's fix it up by peeking at the LHS - TypeSymbol lhsAsTuple = MakeTupleTypeFromDeconstructionLHS(checkedVariables, diagnostics, Compilation); - boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); + // 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, Compilation, diagnostics); + if (lhsAsTuple != null) + { + boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); + } } else { - // expression without type such as `null` Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, right); + } + + if ((object)boundRHS.Type == null) + { + // we could still not infer a type for the RHS FailRemainingInferences(checkedVariables, diagnostics); return new BoundDeconstructionAssignmentOperator( @@ -74,7 +83,6 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CShar voidType, hasErrors: true); } } - var deconstructionSteps = ArrayBuilder.GetInstance(1); var assignmentSteps = ArrayBuilder.GetInstance(1); bool hasErrors = !DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(boundRHS.Syntax, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps); @@ -188,7 +196,7 @@ private static void SetInferredTypes(ArrayBuilder variab if (!variable.HasNestedVariables && variable.Single.Kind == BoundKind.DeconstructionLocalPendingInference) { BoundLocal local = ((DeconstructionLocalPendingInference)variable.Single).SetInferredType(foundTypes[i], success: true); - variables[i] = new DeconstructionVariable(local); + variables[i] = new DeconstructionVariable(local, local.Syntax); } } } @@ -211,7 +219,7 @@ private void FailRemainingInferences(ArrayBuilder variab if (variable.Single.Kind == BoundKind.DeconstructionLocalPendingInference) { var local = ((DeconstructionLocalPendingInference)variable.Single).FailInference(this); - variables[i] = new DeconstructionVariable(local); + variables[i] = new DeconstructionVariable(local, local.Syntax); } } } @@ -226,11 +234,11 @@ private class DeconstructionVariable public readonly ArrayBuilder NestedVariables; public readonly CSharpSyntaxNode Syntax; - public DeconstructionVariable(BoundExpression variable) + public DeconstructionVariable(BoundExpression variable, CSharpSyntaxNode syntax) { Single = variable; NestedVariables = null; - Syntax = variable.Syntax; + Syntax = syntax; } public DeconstructionVariable(ArrayBuilder variables, CSharpSyntaxNode syntax) @@ -282,21 +290,67 @@ private bool DeconstructOrAssignOutputs( } /// - /// For cases where the RHS of a deconstruction-assignment has no type (TupleLiteral), we squint and look at the LHS as a tuple type to give the RHS a type. + /// For cases where the RHS of a deconstruction-declaration is a tuple literal, we merge type information from both the LHS and RHS. + /// 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. + /// Returns null if a merged tuple type could not be fabricated. /// - private static TypeSymbol MakeTupleTypeFromDeconstructionLHS(ArrayBuilder topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) + private static TypeSymbol MakeMergedTupleType(ArrayBuilder lhsVariables, BoundTupleLiteral rhsLiteral, CSharpSyntaxNode syntax, CSharpCompilation compilation, DiagnosticBag diagnostics) { - var typesBuilder = ArrayBuilder.GetInstance(topLevelCheckedVariables.Count); - foreach (var variable in topLevelCheckedVariables) + int leftLength = lhsVariables.Count; + int rightLength = rhsLiteral.Arguments.Length; + + var typesBuilder = ArrayBuilder.GetInstance(lhsVariables.Count); + for (int i = 0; i < rightLength; i++) { - if (variable.HasNestedVariables) + BoundExpression element = rhsLiteral.Arguments[i]; + TypeSymbol mergedType = element.Type; + + if (i < leftLength) { - typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(variable.NestedVariables, diagnostics, compilation)); + var variable = lhsVariables[i]; + if (variable.HasNestedVariables) + { + if (element.Kind == BoundKind.TupleLiteral) + { + // (variables) on the left and (elements) on the right + mergedType = MakeMergedTupleType(variable.NestedVariables, (BoundTupleLiteral)element, syntax, compilation, diagnostics); + } + else if ((object)mergedType == null) + { + // (variables) on the left and null on the right + Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, element.Syntax); + } + } + else + { + if ((object)variable.Single.Type != null) + { + // typed-variable on the left + mergedType = variable.Single.Type; + } + 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 { - typesBuilder.Add(variable.Single.Type); + if ((object)mergedType == null) + { + // a typeless element on the right, matching no variable on the left + Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, element.Syntax); + } } + + typesBuilder.Add(mergedType); + } + + if (typesBuilder.Any(t => t == null)) + { + typesBuilder.Free(); + return null; } return TupleTypeSymbol.Create(locationOpt: null, elementTypes: typesBuilder.ToImmutableAndFree(), elementLocations: default(ImmutableArray), elementNames: default(ImmutableArray), compilation: compilation, diagnostics: diagnostics); @@ -327,7 +381,7 @@ private ArrayBuilder BindDeconstructionAssignmentVariabl var boundVariable = BindExpression(argument.Expression, diagnostics, invoked: false, indexed: false); var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignment, diagnostics); - checkedVariablesBuilder.Add(new DeconstructionVariable(checkedVariable)); + checkedVariablesBuilder.Add(new DeconstructionVariable(checkedVariable, argument)); } } @@ -527,11 +581,11 @@ private ArrayBuilder BindDeconstructionDeclarationLocals DeconstructionVariable local; if (variable.IsDeconstructionDeclaration) { - local = new DeconstructionVariable(BindDeconstructionDeclarationLocals(variable, typeSyntax, diagnostics), node.Deconstruction); + local = new DeconstructionVariable(BindDeconstructionDeclarationLocals(variable, typeSyntax, diagnostics), variable.Deconstruction); } else { - local = new DeconstructionVariable(BindDeconstructionDeclarationLocal(variable, typeSyntax, diagnostics)); + local = new DeconstructionVariable(BindDeconstructionDeclarationLocal(variable, typeSyntax, diagnostics), variable); } localsBuilder.Add(local); diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs index 81a5ece35745d..273f80877a784 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs +++ b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs @@ -3094,6 +3094,15 @@ internal static string ERR_DecConstError { } } + /// + /// Looks up a localized string similar to The type information on the left-hand-side '{0}' and right-hand-side '{1}' of the deconstruction was insufficient to infer a merged type.. + /// + internal static string ERR_DeconstructCouldNotInferMergedType { + get { + return ResourceManager.GetString("ERR_DeconstructCouldNotInferMergedType", resourceCulture); + } + } + /// /// Looks up a localized string similar to Deconstruction `var (...)` form disallows a specific type for 'var'.. /// diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index b4ce5842d0dc6..c5dac5849b82e 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -4917,4 +4917,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ Cannot reference 'System.Runtime.CompilerServices.TupleElementNamesAttribute' explicitly. Use the tuple syntax to define tuple names. + + The type information on the left-hand-side '{0}' and right-hand-side '{1}' of the deconstruction was insufficient to infer a merged type. + diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index 414a706c00eee..b3751439021ae 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1365,6 +1365,7 @@ internal enum ErrorCode ERR_PredefinedTypeMemberNotFoundInAssembly = 8205, ERR_MissingDeconstruct = 8206, + ERR_DeconstructCouldNotInferMergedType = 8209, ERR_DeconstructRequiresExpression = 8210, ERR_DeconstructWrongCardinality = 8211, ERR_CannotDeconstructDynamic = 8212, diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index a06269d80243c..a68ae58547236 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -2974,14 +2974,14 @@ static void Main() "; var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); comp.VerifyDiagnostics( - // (6,24): error CS8210: Deconstruct assignment requires an expression with a type on the right-hand-side. + // (6,9): error CS8209: The type information on the left-hand-side 'x2' and right-hand-side 'null' of the deconstruction was insufficient to infer a merged type. // var (x1, x2) = (1, null); - Diagnostic(ErrorCode.ERR_DeconstructRequiresExpression, "(1, null)").WithLocation(6, 24) + Diagnostic(ErrorCode.ERR_DeconstructCouldNotInferMergedType, "var (x1, x2) = (1, null);").WithArguments("x2", "null").WithLocation(6, 9) ); } [Fact] - public void InferTypeOfTypelessDeclaration() + public void TypeMergingSuccess1() { string source = @" class C @@ -2992,12 +2992,202 @@ static void Main() System.Console.WriteLine(x1 + "" "" + x2 + "" "" + x3); } } +"; + var comp = CompileAndVerify(source, expectedOutput: " 1 2", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); + comp.VerifyDiagnostics(); + } + + [Fact] + public void TypeMergingSuccess2() + { + string source = @" +class C +{ + static void Main() + { + (string x1, byte x2, var x3) = (null, 2, 3); + System.Console.WriteLine(x1 + "" "" + x2 + "" "" + x3); + } +} +"; + + Action validator = (ModuleSymbol module) => + { + var sourceModule = (SourceModuleSymbol)module; + var compilation = sourceModule.DeclaringCompilation; + var tree = compilation.SyntaxTrees.First(); + var model = compilation.GetSemanticModel(tree); + + var literal = tree.GetRoot().DescendantNodes().OfType().Single(); + Assert.Equal(@"(null, 2, 3)", literal.ToString()); + Assert.Null(model.GetTypeInfo(literal).Type); + Assert.Equal("(System.String, System.Byte, System.Int32)", model.GetTypeInfo(literal).ConvertedType.ToTestDisplayString()); + Assert.Equal(ConversionKind.ImplicitTupleLiteral, model.GetConversion(literal).Kind); + }; + + var comp = CompileAndVerify(source, expectedOutput: " 2 3", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, sourceSymbolValidator: validator); + comp.VerifyDiagnostics(); + } + + [Fact] + public void TypeMergingSuccess3() + { + string source = @" +class C +{ + static void Main() + { + (string x1, var x2) = (null, (1, 2)); + System.Console.WriteLine(x1 + "" "" + x2); + } +} +"; + + Action validator = (ModuleSymbol module) => + { + var sourceModule = (SourceModuleSymbol)module; + var compilation = sourceModule.DeclaringCompilation; + var tree = compilation.SyntaxTrees.First(); + var model = compilation.GetSemanticModel(tree); + + var literal = tree.GetRoot().DescendantNodes().OfType().First(); + Assert.Equal(@"(null, (1, 2))", literal.ToString()); + Assert.Null(model.GetTypeInfo(literal).Type); + Assert.Equal("(System.String, (System.Int32, System.Int32))", model.GetTypeInfo(literal).ConvertedType.ToTestDisplayString()); + Assert.Equal(ConversionKind.ImplicitTupleLiteral, model.GetConversion(literal).Kind); + + var nestedLiteral = literal.Arguments[1]; + Assert.Equal(@"(1, 2)", nestedLiteral.ToString()); + Assert.Null(model.GetTypeInfo(nestedLiteral).Type); + Assert.Null(model.GetTypeInfo(nestedLiteral).ConvertedType); + }; + + var comp = CompileAndVerify(source, expectedOutput: " (1, 2)", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, sourceSymbolValidator: validator); + comp.VerifyDiagnostics(); + } + + [Fact] + public void TypeMergingSuccess4() + { + string source = @" +class C +{ + static void Main() + { + ((string x1, byte x2, var x3), int x4) = (M(), 4); + System.Console.WriteLine(x1 + "" "" + x2 + "" "" + x3 + "" "" + x4); + } + static (string, byte, int) M() { return (null, 2, 3); } +} +"; + var comp = CompileAndVerify(source, expectedOutput: " 2 3 4", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); + comp.VerifyDiagnostics(); + } + + [Fact] + public void TypeMergingWithMultipleAmbiguousVars() + { + string source = @" +class C +{ + static void Main() + { + (string x1, (byte x2, var x3), var x4) = (null, (2, null), null); + } +} "; var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); comp.VerifyDiagnostics( - // (6,37): error CS8210: Deconstruct assignment requires an expression with a type on the right-hand-side. - // (var (x1, x2), string x3) = ((1, 2), null); - Diagnostic(ErrorCode.ERR_DeconstructRequiresExpression, "((1, 2), null)").WithLocation(6, 37) + // (6,9): error CS8209: The type information on the left-hand-side 'var x3' and right-hand-side 'null' of the deconstruction was insufficient to infer a merged type. + // (string x1, (byte x2, var x3), var x4) = (null, (2, null), null); + Diagnostic(ErrorCode.ERR_DeconstructCouldNotInferMergedType, "(string x1, (byte x2, var x3), var x4) = (null, (2, null), null);").WithArguments("var x3", "null").WithLocation(6, 9), + // (6,9): error CS8209: The type information on the left-hand-side 'var x4' and right-hand-side 'null' of the deconstruction was insufficient to infer a merged type. + // (string x1, (byte x2, var x3), var x4) = (null, (2, null), null); + Diagnostic(ErrorCode.ERR_DeconstructCouldNotInferMergedType, "(string x1, (byte x2, var x3), var x4) = (null, (2, null), null);").WithArguments("var x4", "null").WithLocation(6, 9) + ); + } + + [Fact] + public void TypeMergingWithTooManyLeftNestings() + { + string source = @" +class C +{ + static void Main() + { + ((string x1, byte x2, var x3), int x4) = (null, 4); + } +} +"; + var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); + comp.VerifyDiagnostics( + // (6,51): error CS8210: Deconstruct assignment requires an expression with a type on the right-hand-side. + // ((string x1, byte x2, var x3), int x4) = (null, 4); + Diagnostic(ErrorCode.ERR_DeconstructRequiresExpression, "null").WithLocation(6, 51) + ); + } + + [Fact] + public void TypeMergingWithTooManyRightNestings() + { + string source = @" +class C +{ + static void Main() + { + (string x1, var x2) = (null, (null, 2)); + } +} +"; + var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); + comp.VerifyDiagnostics( + // (6,9): error CS8209: The type information on the left-hand-side 'var x2' and right-hand-side '(null, 2)' of the deconstruction was insufficient to infer a merged type. + // (string x1, var x2) = (null, (null, 2)); + Diagnostic(ErrorCode.ERR_DeconstructCouldNotInferMergedType, "(string x1, var x2) = (null, (null, 2));").WithArguments("var x2", "(null, 2)").WithLocation(6, 9) + ); + } + + [Fact] + public void TypeMergingWithTooManyLeftVariables() + { + string source = @" +class C +{ + static void Main() + { + (string x1, var x2, int x3) = (null, ""hello""); + } +} +"; + var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); + comp.VerifyDiagnostics( + // (6,9): error CS8211: Cannot deconstruct a tuple of '2' elements into '3' variables. + // (string x1, var x2, int x3) = (null, "hello"); + Diagnostic(ErrorCode.ERR_DeconstructWrongCardinality, @"(string x1, var x2, int x3) = (null, ""hello"");").WithArguments("2", "3").WithLocation(6, 9) + ); + } + + [Fact] + public void TypeMergingWithTooManyRightElements() + { + string source = @" +class C +{ + static void Main() + { + (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 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) ); } @@ -3808,7 +3998,7 @@ static void Main() Assert.Equal("(int, int)", model.GetTypeInfo(literal2).Type.ToDisplayString()); }; - var verifier = CompileAndVerify(source, additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, sourceSymbolValidator: validator); + var verifier = CompileAndVerify(source, additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, sourceSymbolValidator: validator); verifier.VerifyDiagnostics(); }