From c71be153335af74ea8c819a4dac6f5fc7df316ec Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 31 May 2016 18:41:29 -0700 Subject: [PATCH 01/13] Deconstruction-assignment: nested scenario --- .../Portable/Binder/Binder_Statements.cs | 260 +++++++-- .../Portable/BoundTree/BoundExpression.cs | 13 +- .../CSharp/Portable/BoundTree/BoundNodes.xml | 16 +- .../CSharp/Portable/BoundTree/Expression.cs | 32 +- .../Portable/FlowAnalysis/DataFlowPass.cs | 34 +- .../FlowAnalysis/PreciseAbstractFlowPass.cs | 26 +- .../Lowering/LocalRewriter/LocalRewriter.cs | 18 +- ...writer_DeconstructionAssignmentOperator.cs | 113 ++-- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 504 +++++++++++++++--- .../Portable/InternalUtilities/OneOrMany.cs | 35 ++ 10 files changed, 838 insertions(+), 213 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 1e5f56dbcae9c..a9167edb1b3be 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1724,63 +1724,130 @@ private BoundExpression BindAssignment(AssignmentExpressionSyntax node, Diagnost /// private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax node, DiagnosticBag diagnostics) { - SeparatedSyntaxList arguments = ((TupleExpressionSyntax)node.Left).Arguments; - - // receiver for Deconstruct + // receiver for first Deconstruct step var boundRHS = BindValue(node.Right, diagnostics, BindValueKind.RValue); - int numElements = arguments.Count; - Debug.Assert(numElements >= 2); // this should not have parsed as a tuple. + SeparatedSyntaxList arguments = ((TupleExpressionSyntax)node.Left).Arguments; + ImmutableArray checkedVariables = BindDeconstructionVariables(arguments, diagnostics); - // bind the variables and check they can be assigned to - var checkedVariablesBuilder = ArrayBuilder.GetInstance(numElements); - foreach (var argument in arguments) + var deconstructionSteps = ArrayBuilder.GetInstance(1); + var assignmentSteps = ArrayBuilder.GetInstance(1); + try { - var boundVariable = BindExpression(argument.Expression, diagnostics, invoked: false, indexed: false); - var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignment, diagnostics); + if ((object)boundRHS.Type == null) + { + 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); + boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); + } + else + { + // expression without type such as `null` + Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, node); + return BadExpression(node, checkedVariables.Concat(boundRHS).ToArray()); + } + } - checkedVariablesBuilder.Add(checkedVariable); - } + if (!DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(node.Right, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps)) + { + return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, OneOrMany.Create(ImmutableArray.Empty), + ImmutableArray.Empty, + ErrorTypeSymbol.UnknownResultType, + hasErrors: true); + } - var checkedVariables = checkedVariablesBuilder.ToImmutableAndFree(); + var deconstructions = OneOrMany.Create(deconstructionSteps); + var assignments = assignmentSteps.ToImmutable(); - // tuple literal such as `(1, 2)`, `(null, null)`, `(x.P, y.M())` - if (boundRHS.Kind == BoundKind.TupleLiteral || ((object)boundRHS.Type != null && boundRHS.Type.IsTupleType)) + TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); + return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, deconstructions, assignments, voidType); + } + finally { - return BindDeconstructWithTuple(node, diagnostics, boundRHS, checkedVariables); + deconstructionSteps.Free(); + assignmentSteps.Free(); } + } + + /// + /// Whether the target is a tuple or a type that requires Deconstruction, this will generate and stack appropriate deconstruction and assignment steps. + /// Note that the variables may either be plain or nested variables. + /// Returns false if there was an error. + /// + private bool DeconstructIntoSteps( + BoundDeconstructValuePlaceholder targetPlaceholder, + AssignmentExpressionSyntax syntax, + DiagnosticBag diagnostics, + ImmutableArray variables, + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) + { + Debug.Assert(targetPlaceholder.Type != null); - // expression without type such as `null` - if (boundRHS.Type == null) + if (targetPlaceholder.Type.IsTupleType) { - Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, node); - return BadExpression(node, checkedVariables.Concat(boundRHS).ToArray()); + // tuple literal such as `(1, 2)`, `(null, null)`, `(x.P, y.M())` + return DeconstructTuple(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); + } + else + { + return DeconstructWithDeconstruct(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); } - - return BindDeconstructWithDeconstruct(node, diagnostics, boundRHS, checkedVariables); } - private BoundExpression BindDeconstructWithTuple(AssignmentExpressionSyntax node, DiagnosticBag diagnostics, BoundExpression boundRHS, ImmutableArray checkedVariables) + /// + /// This will generate and stack appropriate deconstruction and assignment steps for a tuple type. + /// The produced deconstruction step has no Deconstruct method since the tuple already has distinct elements. + /// Returns false if there was an error. + /// + private bool DeconstructTuple( + BoundDeconstructValuePlaceholder targetPlaceholder, + AssignmentExpressionSyntax syntax, + DiagnosticBag diagnostics, + ImmutableArray variables, + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { - // make a conversion for the tuple based on the LHS information (this also takes care of tuple literals without a natural type) - var lhsTypes = checkedVariables.SelectAsArray(v => v.Type); - TypeSymbol lhsAsTuple = TupleTypeSymbol.Create(locationOpt: null, elementTypes: lhsTypes, elementLocations: default(ImmutableArray), elementNames: default(ImmutableArray), compilation: Compilation, diagnostics: diagnostics); - var typedRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); + Debug.Assert(targetPlaceholder.Type.IsTupleType); - ImmutableArray tupleTypes = typedRHS.Type.TupleElementTypes; + var tupleTypes = targetPlaceholder.Type.TupleElementTypes; - // figure out the pairwise conversions - var assignments = checkedVariables.SelectAsArray((variable, index, types) => MakeAssignmentInfo(variable, types[index], node, diagnostics), tupleTypes); + // Add step for deconstructing this target + var deconstructionStep = new BoundDeconstructionAssignmentOperator.DeconstructStep() + { + DeconstructMemberOpt = null, + TargetPlaceholder = targetPlaceholder, + OutputPlaceholders = tupleTypes.SelectAsArray(type => new BoundDeconstructValuePlaceholder(syntax, type)) + }; - TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); - return new BoundDeconstructionAssignmentOperator(node, checkedVariables, typedRHS, deconstructMemberOpt: null, assignments: assignments, type: voidType); + deconstructionSteps.Add(deconstructionStep); + + // outputs will either need a conversion step and assignment step, or if they are nested variables, they will need further deconstruction + if (!DeconstructOrAssignOutputs(deconstructionStep, variables, syntax, diagnostics, deconstructionSteps, assignmentSteps)) + { + return false; + } + + return true; } - private BoundExpression BindDeconstructWithDeconstruct(AssignmentExpressionSyntax node, DiagnosticBag diagnostics, BoundExpression boundRHS, ImmutableArray checkedVariables) + /// + /// This will generate and stack appropriate deconstruction and assignment steps for a non-tuple type. + /// Returns false if there was an error (if a suitable Deconstruct method was not found). + /// + private bool DeconstructWithDeconstruct( + BoundDeconstructValuePlaceholder targetPlaceholder, + AssignmentExpressionSyntax syntax, + DiagnosticBag diagnostics, + ImmutableArray variables, + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { // symbol and parameters for Deconstruct DiagnosticBag bag = new DiagnosticBag(); - MethodSymbol deconstructMethod = FindDeconstruct(checkedVariables, boundRHS, node, bag); + MethodSymbol deconstructMethod = FindDeconstruct(variables.Length, targetPlaceholder, syntax, bag); if (!diagnostics.HasAnyErrors()) { diagnostics.AddRange(bag); @@ -1788,32 +1855,127 @@ private BoundExpression BindDeconstructWithDeconstruct(AssignmentExpressionSynta if ((object)deconstructMethod == null) { - return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, ErrorMethodSymbol.UnknownMethod, - ImmutableArray.Empty, - ErrorTypeSymbol.UnknownResultType, - hasErrors: true); + return false; } - // figure out the pairwise conversions var deconstructParameters = deconstructMethod.Parameters; - var assignments = checkedVariables.SelectAsArray((variable, index, parameters) => MakeAssignmentInfo(variable, parameters[index].Type, node, diagnostics), deconstructParameters); - TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); - return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, deconstructMethod, assignments, voidType); + // add step for deconstructing this target + var deconstructionStep = new BoundDeconstructionAssignmentOperator.DeconstructStep() + { + DeconstructMemberOpt = deconstructMethod, + TargetPlaceholder = targetPlaceholder, + OutputPlaceholders = deconstructParameters.SelectAsArray(parameter => new BoundDeconstructValuePlaceholder(syntax, parameter.Type)) + }; + deconstructionSteps.Add(deconstructionStep); + + // outputs will either need a conversion step and assignment step, or if they are nested variables, they will need further deconstruction + if (!DeconstructOrAssignOutputs(deconstructionStep, variables, syntax, diagnostics, deconstructionSteps, assignmentSteps)) + { + return false; + } + + return true; + } + + /// + /// Takes the outputs from the previous deconstructionStep and depending on the structure of variables, will generate further deconstructions, or simply assignments. + /// + public bool DeconstructOrAssignOutputs( + BoundDeconstructionAssignmentOperator.DeconstructStep deconstructionStep, + ImmutableArray variables, + AssignmentExpressionSyntax syntax, + DiagnosticBag diagnostics, + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) + { + for (int i = 0; i < variables.Length; i++) + { + var variable = variables[i]; + + if (variable.Kind == BoundKind.DeconstructionVariables) + { + var nestedVariables = ((BoundDeconstructionVariables)variables[i]).Variables; + + if (!DeconstructIntoSteps(deconstructionStep.OutputPlaceholders[i], syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) + { + return false; + } + } + else + { + var assignment = MakeAssignmentInfo(variable, deconstructionStep.OutputPlaceholders[i].Type, deconstructionStep.OutputPlaceholders[i], syntax, diagnostics); + assignmentSteps.Add(assignment); + } + } + + return true; + } + + /// + /// For cases where the RHS of a deconstruction-assignment has not type (TupleLiteral), we squint and look at the LHS as a tuple type to give the RHS a type. + /// + private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray topLevelCheckedVariables, DiagnosticBag diagnostics) + { + var typesBuilder = ArrayBuilder.GetInstance(topLevelCheckedVariables.Length); + foreach (var variable in topLevelCheckedVariables) + { + if (variable.Kind == BoundKind.DeconstructionVariables) + { + typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(((BoundDeconstructionVariables)variable).Variables, diagnostics)); + } + else + { + typesBuilder.Add(variable.Type); + } + } + + return TupleTypeSymbol.Create(locationOpt: null, elementTypes: typesBuilder.ToImmutableAndFree(), elementLocations: default(ImmutableArray), elementNames: default(ImmutableArray), compilation: Compilation, diagnostics: diagnostics); + } + + /// + /// Returns an array of variables, where some may be nested variables (BoundDeconstructionVariables). + /// Checks that all the variables are assignable to. + /// + private ImmutableArray BindDeconstructionVariables(SeparatedSyntaxList arguments, DiagnosticBag diagnostics) + { + int numElements = arguments.Count; + Debug.Assert(numElements >= 2); // this should not have parsed as a tuple. + + // bind the variables and check they can be assigned to + var checkedVariablesBuilder = ArrayBuilder.GetInstance(numElements); + + foreach (var argument in arguments) + { + if (argument.Expression.Kind() == SyntaxKind.TupleExpression) // nested tuple case + { + var nestedArguments = ((TupleExpressionSyntax)argument.Expression).Arguments; + checkedVariablesBuilder.Add(new BoundDeconstructionVariables(argument, BindDeconstructionVariables(nestedArguments, diagnostics))); + } + else + { + var boundVariable = BindExpression(argument.Expression, diagnostics, invoked: false, indexed: false); + var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignment, diagnostics); + + checkedVariablesBuilder.Add(checkedVariable); + } + } + + var checkedVariables = checkedVariablesBuilder.ToImmutableAndFree(); + return checkedVariables; } /// /// Figures out how to assign from sourceType into receivingVariable and bundles the information (leaving holes for the actual source and receiver) into an AssignmentInfo. /// - private BoundDeconstructionAssignmentOperator.AssignmentInfo MakeAssignmentInfo(BoundExpression receivingVariable, TypeSymbol sourceType, AssignmentExpressionSyntax node, DiagnosticBag diagnostics) + private BoundDeconstructionAssignmentOperator.AssignmentStep MakeAssignmentInfo(BoundExpression receivingVariable, TypeSymbol sourceType, BoundDeconstructValuePlaceholder rightPlaceholder, AssignmentExpressionSyntax node, DiagnosticBag diagnostics) { - var leftPlaceholder = new BoundLValuePlaceholder(receivingVariable.Syntax, receivingVariable.Type) { WasCompilerGenerated = true }; - var rightPlaceholder = new BoundRValuePlaceholder(node.Right, sourceType) { WasCompilerGenerated = true }; + var leftPlaceholder = new BoundDeconstructValuePlaceholder(receivingVariable.Syntax, receivingVariable.Type) { WasCompilerGenerated = true }; // each assignment has a placeholder for a receiver and another for the source BoundAssignmentOperator op = BindAssignment(node, leftPlaceholder, rightPlaceholder, diagnostics); - return new BoundDeconstructionAssignmentOperator.AssignmentInfo() { Assignment = op, LValuePlaceholder = leftPlaceholder, RValuePlaceholder = rightPlaceholder }; + return new BoundDeconstructionAssignmentOperator.AssignmentStep() { Assignment = op, OutputPlaceholder = leftPlaceholder, TargetPlaceholder = rightPlaceholder }; } /// @@ -1821,7 +1983,7 @@ private BoundDeconstructionAssignmentOperator.AssignmentInfo MakeAssignmentInfo( /// Returns true if the Deconstruct method is found. /// If so, it outputs the method. /// - private static MethodSymbol FindDeconstruct(ImmutableArray checkedVariables, BoundExpression boundRHS, SyntaxNode node, DiagnosticBag diagnostics) + private static MethodSymbol FindDeconstruct(int numCheckedVariables, BoundExpression boundRHS, SyntaxNode node, DiagnosticBag diagnostics) { // find symbol for Deconstruct member ImmutableArray candidates = boundRHS.Type.GetMembers("Deconstruct"); @@ -1853,9 +2015,9 @@ private static MethodSymbol FindDeconstruct(ImmutableArray chec return null; } - if (deconstructMethod.ParameterCount != checkedVariables.Length) + if (deconstructMethod.ParameterCount != numCheckedVariables) { - Error(diagnostics, ErrorCode.ERR_DeconstructWrongParams, boundRHS.Syntax, deconstructMethod, checkedVariables.Length); + Error(diagnostics, ErrorCode.ERR_DeconstructWrongParams, boundRHS.Syntax, deconstructMethod, numCheckedVariables); return null; } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs index 9ebbdc8f0d059..24c1d2e54c60b 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs @@ -280,11 +280,18 @@ public override Symbol ExpressionSymbol internal sealed partial class BoundDeconstructionAssignmentOperator : BoundExpression { - internal class AssignmentInfo + internal class AssignmentStep { public BoundAssignmentOperator Assignment; - public BoundLValuePlaceholder LValuePlaceholder; - public BoundRValuePlaceholder RValuePlaceholder; + public BoundDeconstructValuePlaceholder OutputPlaceholder; + public BoundDeconstructValuePlaceholder TargetPlaceholder; + } + + internal class DeconstructStep + { + public MethodSymbol DeconstructMemberOpt; // the deconstruct member, or null if tuple deconstruction + public BoundDeconstructValuePlaceholder TargetPlaceholder; + public ImmutableArray OutputPlaceholders; // placeholders for the outputs produced by this deconstruction } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index 80a65cbf18e35..48e197e0a4152 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -84,9 +84,7 @@ This node is used to represent an expression returning value of a certain type. It is used to perform intermediate binding, and will not survive the local rewriting. --> - - - + @@ -394,13 +392,21 @@ + - + + - + + + + + + + diff --git a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs index d70a651b8d65f..711918aabe581 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs @@ -33,22 +33,7 @@ Optional IOperation.ConstantValue public abstract TResult Accept(OperationVisitor visitor, TArgument argument); } - internal sealed partial class BoundLValuePlaceholder : BoundValuePlaceholderBase, IPlaceholderExpression - { - protected override OperationKind ExpressionKind => OperationKind.PlaceholderExpression; - - public override void Accept(OperationVisitor visitor) - { - visitor.VisitPlaceholderExpression(this); - } - - public override TResult Accept(OperationVisitor visitor, TArgument argument) - { - return visitor.VisitPlaceholderExpression(this, argument); - } - } - - internal sealed partial class BoundRValuePlaceholder : BoundValuePlaceholderBase, IPlaceholderExpression + internal sealed partial class BoundDeconstructValuePlaceholder : BoundValuePlaceholderBase, IPlaceholderExpression { protected override OperationKind ExpressionKind => OperationKind.PlaceholderExpression; @@ -1049,6 +1034,21 @@ public override void Accept(OperationVisitor visitor) } } + internal sealed partial class BoundDeconstructionVariables : BoundExpression + { + protected override OperationKind ExpressionKind => OperationKind.None; + + public override void Accept(OperationVisitor visitor) + { + visitor.VisitNoneOperation(this); + } + + public override TResult Accept(OperationVisitor visitor, TArgument argument) + { + return visitor.VisitNoneOperation(this, argument); + } + } + internal sealed partial class BoundVoid : BoundExpression { protected override OperationKind ExpressionKind => OperationKind.None; diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs index 4b7eb0c8c4bf4..d63fdf44e2fb6 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs @@ -1485,7 +1485,7 @@ public override BoundNode VisitFixedStatement(BoundFixedStatement node) { foreach (LocalSymbol local in node.Locals) { - switch(local.DeclarationKind) + switch (local.DeclarationKind) { case LocalDeclarationKind.RegularVariable: case LocalDeclarationKind.PatternVariable: @@ -1721,14 +1721,40 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) Assign(node.Left, node.Right, refKind: node.RefKind); return null; } - + public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstructionAssignmentOperator node) { base.VisitDeconstructionAssignmentOperator(node); - foreach(BoundExpression variable in node.LeftVariables) + foreach (BoundExpression variable in node.LeftVariables) + { + if (variable.Kind == BoundKind.DeconstructionVariables) + { + VisitDeconstructionVariables((BoundDeconstructionVariables)variable); + } + else + { + Assign(variable, null, refKind: RefKind.None); + } + } + + return null; + } + + public override BoundNode VisitDeconstructionVariables(BoundDeconstructionVariables node) + { + base.VisitDeconstructionVariables(node); + + foreach (BoundExpression variable in node.Variables) { - Assign(variable, null, refKind: RefKind.None); + if (variable.Kind == BoundKind.DeconstructionVariables) + { + VisitDeconstructionVariables((BoundDeconstructionVariables)variable); + } + else + { + Assign(variable, null, refKind: RefKind.None); + } } return null; diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs index aab14c7e3ffdc..08f003580c3d8 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs @@ -1518,7 +1518,14 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct { foreach (BoundExpression variable in node.LeftVariables) { - VisitLvalue(variable); + if (variable.Kind == BoundKind.DeconstructionVariables) + { + VisitDeconstructionVariables((BoundDeconstructionVariables)variable); + } + else + { + VisitLvalue(variable); + } } VisitRvalue(node.Right); @@ -1526,6 +1533,23 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct return null; } + public override BoundNode VisitDeconstructionVariables(BoundDeconstructionVariables node) + { + foreach (BoundExpression variable in node.Variables) + { + if (variable.Kind == BoundKind.DeconstructionVariables) + { + VisitDeconstructionVariables((BoundDeconstructionVariables)variable); + } + else + { + VisitLvalue(variable); + } + } + + return null; + } + public override BoundNode VisitCompoundAssignmentOperator(BoundCompoundAssignmentOperator node) { // TODO: should events be handled specially too? diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs index a3011854e813a..02d1ce7da323d 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs @@ -230,12 +230,7 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen } } - public override BoundNode VisitLValuePlaceholder(BoundLValuePlaceholder node) - { - return PlaceholderReplacement(node); - } - - public override BoundNode VisitRValuePlaceholder(BoundRValuePlaceholder node) + public override BoundNode VisitDeconstructValuePlaceholder(BoundDeconstructValuePlaceholder node) { return PlaceholderReplacement(node); } @@ -287,6 +282,17 @@ private void RemovePlaceholderReplacement(BoundValuePlaceholderBase placeholder) Debug.Assert(removed); } + /// + /// Remove all the listed placeholders. + /// + private void RemovePlaceholderReplacements(IEnumerable placeholders) + { + foreach (var placeholder in placeholders) + { + RemovePlaceholderReplacement(placeholder); + } + } + public override BoundNode VisitBadExpression(BoundBadExpression node) { // Cannot recurse into BadExpression children since the BadExpression diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index ee3bf313d4278..d19c24578981e 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -14,59 +14,87 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct { var temps = ArrayBuilder.GetInstance(); var stores = ArrayBuilder.GetInstance(); + var placeholders = ArrayBuilder.GetInstance(); - var lhsReceivers = ArrayBuilder.GetInstance(); - foreach (var variable in node.LeftVariables) - { - // This will be filled in with the LHS that uses temporaries to prevent - // double-evaluation of side effects. - lhsReceivers.Add(TransformCompoundAssignmentLHS(variable, stores, temps, isDynamicAssignment: false)); - } - - BoundExpression loweredRight = VisitExpression(node.Right); - ImmutableArray rhsValues; + // evaluate left-hand-side side-effects + var lhsTemps = LhsSideEffects(node.LeftVariables, temps, stores); // get or make right-hand-side values - if (node.Right.Type.IsTupleType) - { - rhsValues = AccessTupleFields(node, loweredRight, temps, stores); - } - else + BoundExpression loweredRight = VisitExpression(node.Right); + AddPlaceholderReplacement(node.DeconstructSteps[0].TargetPlaceholder, loweredRight); + placeholders.Add(node.DeconstructSteps[0].TargetPlaceholder); + foreach (var deconstruction in node.DeconstructSteps) { - rhsValues = CallDeconstruct(node, loweredRight, temps, stores); + if (deconstruction.DeconstructMemberOpt == null) + { + // tuple case + AccessTupleFields(node, deconstruction, temps, stores, placeholders); + } + else + { + CallDeconstruct(node, deconstruction, temps, stores, placeholders); + } } - // assign from rhs values to lhs receivers - int numAssignments = node.Assignments.Length; + int numAssignments = node.AssignmentSteps.Length; for (int i = 0; i < numAssignments; i++) { // lower the assignment and replace the placeholders for source and target in the process - var assignmentInfo = node.Assignments[i]; - - AddPlaceholderReplacement(assignmentInfo.LValuePlaceholder, lhsReceivers[i]); - AddPlaceholderReplacement(assignmentInfo.RValuePlaceholder, rhsValues[i]); + var assignmentInfo = node.AssignmentSteps[i]; + AddPlaceholderReplacement(assignmentInfo.OutputPlaceholder, lhsTemps[i]); var assignment = VisitExpression(assignmentInfo.Assignment); - RemovePlaceholderReplacement(assignmentInfo.LValuePlaceholder); - RemovePlaceholderReplacement(assignmentInfo.RValuePlaceholder); + RemovePlaceholderReplacement(assignmentInfo.OutputPlaceholder); stores.Add(assignment); } - stores.Add(new BoundVoid(node.Syntax, node.Type)); - var result = _factory.Sequence(temps.ToImmutable(), stores.ToArray()); + var result = _factory.Sequence(temps.ToImmutable(), stores.ToImmutable(), new BoundVoid(node.Syntax, node.Type)); + + RemovePlaceholderReplacements(placeholders); + placeholders.Free(); temps.Free(); stores.Free(); - lhsReceivers.Free(); return result; } - private ImmutableArray AccessTupleFields(BoundDeconstructionAssignmentOperator node, BoundExpression loweredRight, ArrayBuilder temps, ArrayBuilder stores) + /// + /// Adds the side effects to stores and returns temporaries (as a flat list) to access them. + /// + private ImmutableArray LhsSideEffects(ImmutableArray variables, ArrayBuilder temps, ArrayBuilder stores) { - var tupleType = loweredRight.Type.IsTupleType ? loweredRight.Type : TupleTypeSymbol.Create((NamedTypeSymbol)loweredRight.Type); + var lhsReceivers = ArrayBuilder.GetInstance(); + + LhsSideEffects(variables, temps, stores, lhsReceivers); + + return lhsReceivers.ToImmutableAndFree(); + } + + /// + /// Adds the side effects to stores and adds temporaries to lhsReceivers. + /// + private void LhsSideEffects(ImmutableArray variables, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder lhsReceivers) + { + foreach (var variable in variables) + { + if (variable.Kind == BoundKind.DeconstructionVariables) + { + LhsSideEffects(((BoundDeconstructionVariables)variable).Variables, temps, stores, lhsReceivers); + } + else + { + lhsReceivers.Add(TransformCompoundAssignmentLHS(variable, stores, temps, isDynamicAssignment: false)); + } + } + } + + private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, BoundDeconstructionAssignmentOperator.DeconstructStep deconstruction, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder placeholders) + { + var target = PlaceholderReplacement(deconstruction.TargetPlaceholder); + var tupleType = target.Type.IsTupleType ? target.Type : TupleTypeSymbol.Create((NamedTypeSymbol)target.Type); var tupleElementTypes = tupleType.TupleElementTypes; var numElements = tupleElementTypes.Length; @@ -74,14 +102,13 @@ private ImmutableArray AccessTupleFields(BoundDeconstructionAss CSharpSyntaxNode syntax = node.Syntax; - // save the loweredRight as we need to access it multiple times + // save the loweredRight as we need to access it multiple times BoundAssignmentOperator assignmentToTemp; - var savedTuple = _factory.StoreToTemp(loweredRight, out assignmentToTemp); + var savedTuple = _factory.StoreToTemp(target, out assignmentToTemp); stores.Add(assignmentToTemp); temps.Add(savedTuple.LocalSymbol); // list the tuple fields accessors - var fieldAccessorsBuilder = ArrayBuilder.GetInstance(numElements); var fields = tupleType.TupleElementFields; for (int i = 0; i < numElements; i++) @@ -94,10 +121,10 @@ private ImmutableArray AccessTupleFields(BoundDeconstructionAss Symbol.ReportUseSiteDiagnostic(useSiteInfo, _diagnostics, syntax.Location); } var fieldAccess = MakeTupleFieldAccess(syntax, field, savedTuple, null, LookupResultKind.Empty, tupleElementTypes[i]); - fieldAccessorsBuilder.Add(fieldAccess); - } - return fieldAccessorsBuilder.ToImmutableAndFree(); + AddPlaceholderReplacement(deconstruction.OutputPlaceholders[i], fieldAccess); + placeholders.Add(deconstruction.OutputPlaceholders[i]); + } } /// @@ -105,19 +132,20 @@ private ImmutableArray AccessTupleFields(BoundDeconstructionAss /// Adds a invocation of Deconstruct with those as out parameters onto the 'stores' sequence /// Returns the expressions for those out parameters /// - private ImmutableArray CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundExpression loweredRight, ArrayBuilder temps, ArrayBuilder stores) + private void CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundDeconstructionAssignmentOperator.DeconstructStep deconstruction, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder placeholders) { - Debug.Assert((object)node.DeconstructMemberOpt != null); + Debug.Assert((object)deconstruction.DeconstructMemberOpt != null); CSharpSyntaxNode syntax = node.Syntax; // prepare out parameters for Deconstruct - var deconstructParameters = node.DeconstructMemberOpt.Parameters; + var deconstructParameters = deconstruction.DeconstructMemberOpt.Parameters; var outParametersBuilder = ArrayBuilder.GetInstance(deconstructParameters.Length); Debug.Assert(deconstructParameters.Length == node.LeftVariables.Length); - foreach (var deconstructParameter in deconstructParameters) + for (var i = 0; i < deconstructParameters.Length; i++) { + var deconstructParameter = deconstructParameters[i]; var localSymbol = new SynthesizedLocal(_factory.CurrentMethod, deconstructParameter.Type, SynthesizedLocalKind.LoweringTemp); var localBound = new BoundLocal(syntax, @@ -129,15 +157,16 @@ private ImmutableArray CallDeconstruct(BoundDeconstructionAssig temps.Add(localSymbol); outParametersBuilder.Add(localBound); + + AddPlaceholderReplacement(deconstruction.OutputPlaceholders[i], localBound); + placeholders.Add(deconstruction.OutputPlaceholders[i]); } var outParameters = outParametersBuilder.ToImmutableAndFree(); // invoke Deconstruct - var invokeDeconstruct = MakeCall(syntax, loweredRight, node.DeconstructMemberOpt, outParameters, node.DeconstructMemberOpt.ReturnType); + var invokeDeconstruct = MakeCall(syntax, PlaceholderReplacement(deconstruction.TargetPlaceholder), deconstruction.DeconstructMemberOpt, outParameters, deconstruction.DeconstructMemberOpt.ReturnType); stores.Add(invokeDeconstruct); - - return outParameters; } } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index ec91475cfb936..a2162c046b0c4 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -10,6 +10,48 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen [CompilerTrait(CompilerFeature.Tuples)] public class CodeGenDeconstructTests : CSharpTestBase { + const string commonSource = +@"public class Pair +{ + T1 item1; + T2 item2; + + public Pair(T1 item1, T2 item2) + { + this.item1 = item1; + this.item2 = item2; + } + + public void Deconstruct(out T1 item1, out T2 item2) + { + System.Console.WriteLine($""Deconstructing {ToString()}""); + item1 = this.item1; + item2 = this.item2; + } + + public override string ToString() { return $""({item1.ToString()}, {item2.ToString()})""; } +} + +public static class Pair +{ + public static Pair Create(T1 item1, T2 item2) { return new Pair(item1, item2); } +} + +public class Integer +{ + public int state; + public override string ToString() { return state.ToString(); } + public Integer(int i) { state = i; } + public static implicit operator LongInteger(Integer i) { System.Console.WriteLine($""Converting {i}""); return new LongInteger(i.state); } +} + +public class LongInteger +{ + long state; + public LongInteger(long l) { state = l; } + public override string ToString() { return state.ToString(); } +}"; + [Fact] public void SimpleAssign() { @@ -280,10 +322,9 @@ class C static void Main() { C c = new C(); - int z; // PROTOTYPE(tuples) this should be removed once the return-type issue is fixed - (c.getHolderForX().x, c.getHolderForY().y, z) = c.getDeconstructReceiver(); + (c.getHolderForX().x, c.getHolderForY().y) = c.getDeconstructReceiver(); } - public void Deconstruct(out D1 x, out D2 y, out int z) { x = new D1(); y = new D2(); z = 3; Console.WriteLine(""Deconstruct""); } + public void Deconstruct(out D1 x, out D2 y) { x = new D1(); y = new D2(); Console.WriteLine(""Deconstruct""); } } class D1 { @@ -454,47 +495,15 @@ public void Deconstruct(out int a, out int b) "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.Regular.WithTuplesFeature()); comp.VerifyDiagnostics( - // (8,18): error CS0266: Cannot implicitly convert type 'int' to 'byte'. An explicit conversion exists (are you missing a cast?) + // (8,9): error CS0266: Cannot implicitly convert type 'int' to 'byte'. An explicit conversion exists (are you missing a cast?) // (x, y) = new C(); - Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "new C()").WithArguments("int", "byte").WithLocation(8, 18), - // (8,18): error CS0029: Cannot implicitly convert type 'int' to 'string' + Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "(x, y) = new C()").WithArguments("int", "byte").WithLocation(8, 9), + // (8,9): error CS0029: Cannot implicitly convert type 'int' to 'string' // (x, y) = new C(); - Diagnostic(ErrorCode.ERR_NoImplicitConv, "new C()").WithArguments("int", "string").WithLocation(8, 18) + Diagnostic(ErrorCode.ERR_NoImplicitConv, "(x, y) = new C()").WithArguments("int", "string").WithLocation(8, 9) ); } - [Fact(Skip = "PROTOTYPE(tuples)")] - public void Nesting() - { - string source = @" -class C -{ - static void Main() - { - int x, y, z; - (x, (y, z)) = new C(); - } - - public void Deconstruct(out int a, out D d) - { - a = 1; - d = new D(); - } -} -class D -{ - public void Deconstruct(out string b, out string c) - { - b = ""hello""; - c = ""world""; - } -} -"; - var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.Regular.WithTuplesFeature()); - comp.VerifyDiagnostics(); - // expect a console output - } - [Fact] public void ExpressionType() { @@ -959,57 +968,57 @@ .maxstack 10 .locals init (long V_0, //x int V_1) //y IL_0000: ldc.i4.1 - IL_0001: conv.i8 + IL_0001: ldc.i4.1 IL_0002: ldc.i4.1 - IL_0003: conv.i8 + IL_0003: ldc.i4.1 IL_0004: ldc.i4.1 - IL_0005: conv.i8 + IL_0005: ldc.i4.1 IL_0006: ldc.i4.1 - IL_0007: conv.i8 - IL_0008: ldc.i4.1 - IL_0009: conv.i8 - IL_000a: ldc.i4.1 - IL_000b: conv.i8 - IL_000c: ldc.i4.1 - IL_000d: conv.i8 - IL_000e: ldc.i4.1 - IL_000f: conv.i8 - IL_0010: ldc.i4.4 - IL_0011: conv.i8 - IL_0012: ldc.i4.2 - IL_0013: newobj ""System.ValueTuple..ctor(long, long, int)"" - IL_0018: newobj ""System.ValueTuple..ctor(long, long, long, long, long, long, long, (long, long, int))"" - IL_001d: dup - IL_001e: ldfld ""long System.ValueTuple.Item1"" + IL_0007: ldc.i4.1 + IL_0008: ldc.i4.4 + IL_0009: ldc.i4.2 + IL_000a: newobj ""System.ValueTuple..ctor(int, int, int)"" + IL_000f: newobj ""System.ValueTuple..ctor(int, int, int, int, int, int, int, (int, int, int))"" + IL_0014: dup + IL_0015: ldfld ""int System.ValueTuple.Item1"" + IL_001a: conv.i8 + IL_001b: stloc.0 + IL_001c: dup + IL_001d: ldfld ""int System.ValueTuple.Item2"" + IL_0022: conv.i8 IL_0023: stloc.0 IL_0024: dup - IL_0025: ldfld ""long System.ValueTuple.Item2"" - IL_002a: stloc.0 - IL_002b: dup - IL_002c: ldfld ""long System.ValueTuple.Item3"" - IL_0031: stloc.0 - IL_0032: dup - IL_0033: ldfld ""long System.ValueTuple.Item4"" - IL_0038: stloc.0 - IL_0039: dup - IL_003a: ldfld ""long System.ValueTuple.Item5"" - IL_003f: stloc.0 - IL_0040: dup - IL_0041: ldfld ""long System.ValueTuple.Item6"" - IL_0046: stloc.0 - IL_0047: dup - IL_0048: ldfld ""long System.ValueTuple.Item7"" - IL_004d: stloc.0 - IL_004e: dup - IL_004f: ldfld ""(long, long, int) System.ValueTuple.Rest"" - IL_0054: ldfld ""long System.ValueTuple.Item1"" - IL_0059: stloc.0 - IL_005a: dup - IL_005b: ldfld ""(long, long, int) System.ValueTuple.Rest"" - IL_0060: ldfld ""long System.ValueTuple.Item2"" + IL_0025: ldfld ""int System.ValueTuple.Item3"" + IL_002a: conv.i8 + IL_002b: stloc.0 + IL_002c: dup + IL_002d: ldfld ""int System.ValueTuple.Item4"" + IL_0032: conv.i8 + IL_0033: stloc.0 + IL_0034: dup + IL_0035: ldfld ""int System.ValueTuple.Item5"" + IL_003a: conv.i8 + IL_003b: stloc.0 + IL_003c: dup + IL_003d: ldfld ""int System.ValueTuple.Item6"" + IL_0042: conv.i8 + IL_0043: stloc.0 + IL_0044: dup + IL_0045: ldfld ""int System.ValueTuple.Item7"" + IL_004a: conv.i8 + IL_004b: stloc.0 + IL_004c: dup + IL_004d: ldfld ""(int, int, int) System.ValueTuple.Rest"" + IL_0052: ldfld ""int System.ValueTuple.Item1"" + IL_0057: conv.i8 + IL_0058: stloc.0 + IL_0059: dup + IL_005a: ldfld ""(int, int, int) System.ValueTuple.Rest"" + IL_005f: ldfld ""int System.ValueTuple.Item2"" + IL_0064: conv.i8 IL_0065: stloc.0 - IL_0066: ldfld ""(long, long, int) System.ValueTuple.Rest"" - IL_006b: ldfld ""int System.ValueTuple.Item3"" + IL_0066: ldfld ""(int, int, int) System.ValueTuple.Rest"" + IL_006b: ldfld ""int System.ValueTuple.Item3"" IL_0070: stloc.1 IL_0071: ldloc.0 IL_0072: box ""long"" @@ -1125,12 +1134,14 @@ static void Main() } } "; - // PROTOTYPE(tuples) This error message is misleading var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); comp.VerifyDiagnostics( - // (9,18): error CS0029: Cannot implicitly convert type '(int, int)' to '(byte, string)' + // (9,9): error CS0266: Cannot implicitly convert type 'int' to 'byte'. An explicit conversion exists (are you missing a cast?) // (x, y) = (1, 2); - Diagnostic(ErrorCode.ERR_NoImplicitConv, "(1, 2)").WithArguments("(int, int)", "(byte, string)").WithLocation(9, 18) + Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "(x, y) = (1, 2)").WithArguments("int", "byte").WithLocation(9, 9), + // (9,9): error CS0029: Cannot implicitly convert type 'int' to 'string' + // (x, y) = (1, 2); + Diagnostic(ErrorCode.ERR_NoImplicitConv, "(x, y) = (1, 2)").WithArguments("int", "string").WithLocation(9, 9) ); } @@ -1343,5 +1354,324 @@ public void Deconstruct(out int a, out int b) Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, "z = ((x, y) = new C())").WithArguments("void").WithLocation(10, 13) ); } + + [Fact] + public void NestedTupleAssignment() + { + string source = @" +class C +{ + static void Main() + { + int x; + string y, z; + + (x, (y, z)) = (1, (""a"", ""b"")); + System.Console.WriteLine(x + "" "" + y + "" "" + z); + } +} +"; + var comp = CompileAndVerify(source, expectedOutput: "1 a b", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void NestedTypelessTupleAssignment() + { + string source = @" +class C +{ + static void Main() + { + string x, y, z; + + (x, (y, z)) = (null, (null, null)); + System.Console.WriteLine(""nothing"" + x + y + z); + } +} +"; + var comp = CompileAndVerify(source, expectedOutput: "nothing", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact(Skip = "PROTOTYPE(tuples)")] + public void NestedTypelessTupleAssignment2() + { + string source = @" +class C +{ + static void Main() + { + int x, y, z; // int cannot be null + + (x, (y, z)) = (null, (null, null)); + System.Console.WriteLine(""nothing"" + x + y + z); + } +} +"; + var comp = CompileAndVerify(source, expectedOutput: "nothing", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void NestedDeconstructAssignment() + { + string source = @" +class C +{ + static void Main() + { + int x; + string y, z; + + (x, (y, z)) = new D1(); + System.Console.WriteLine(x + "" "" + y + "" "" + z); + } +} +class D1 +{ + public void Deconstruct(out int item1, out D2 item2) + { + item1 = 1; + item2 = new D2(); + } +} +class D2 +{ + public void Deconstruct(out string item1, out string item2) + { + item1 = ""a""; + item2 = ""b""; + } +} +"; + var comp = CompileAndVerify(source, expectedOutput: "1 a b", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void NestedMixedAssignment1() + { + string source = @" +class C +{ + static void Main() + { + int x, y, z; + + (x, (y, z)) = (1, new D1()); + System.Console.WriteLine(x + "" "" + y + "" "" + z); + } +} +class D1 +{ + public void Deconstruct(out int item1, out int item2) + { + item1 = 2; + item2 = 3; + } +} +"; + var comp = CompileAndVerify(source, expectedOutput: "1 2 3", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void NestedMixedAssignment2() + { + string source = @" +class C +{ + static void Main() + { + int x; + string y, z; + + (x, (y, z)) = new D1(); + System.Console.WriteLine(x + "" "" + y + "" "" + z); + } +} +class D1 +{ + public void Deconstruct(out int item1, out (string, string) item2) + { + item1 = 1; + item2 = (""a"", ""b""); + } +} +"; + var comp = CompileAndVerify(source, expectedOutput: "1 a b", additionalRefs: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void VerifyNestedExecutionOrder() + { + string source = @" +using System; +class C +{ + int x { set { Console.WriteLine($""setX""); } } + int y { set { Console.WriteLine($""setY""); } } + int z { set { Console.WriteLine($""setZ""); } } + + C getHolderForX() { Console.WriteLine(""getHolderforX""); return this; } + C getHolderForY() { Console.WriteLine(""getHolderforY""); return this; } + C getHolderForZ() { Console.WriteLine(""getHolderforZ""); return this; } + C getDeconstructReceiver() { Console.WriteLine(""getDeconstructReceiver""); return this; } + + static void Main() + { + C c = new C(); + (c.getHolderForX().x, (c.getHolderForY().y, c.getHolderForZ().z)) = c.getDeconstructReceiver(); + } + public void Deconstruct(out D1 x, out C1 t) { x = new D1(); t = new C1(); Console.WriteLine(""Deconstruct1""); } +} +class C1 +{ + public void Deconstruct(out D2 y, out D3 z) { y = new D2(); z = new D3(); Console.WriteLine(""Deconstruct2""); } +} +class D1 +{ + public static implicit operator int(D1 d) { Console.WriteLine(""Conversion1""); return 1; } +} +class D2 +{ + public static implicit operator int(D2 d) { Console.WriteLine(""Conversion2""); return 2; } +} +class D3 +{ + public static implicit operator int(D3 d) { Console.WriteLine(""Conversion3""); return 3; } +} +"; + + string expected = +@"getHolderforX +getHolderforY +getHolderforZ +getDeconstructReceiver +Deconstruct1 +Deconstruct2 +Conversion1 +setX +Conversion2 +setY +Conversion3 +setZ +"; + var comp = CompileAndVerify(source, expectedOutput: expected, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void VerifyNestedExecutionOrder2() + { + string source = @" +using System; +class C +{ + static LongInteger x1 { set { Console.WriteLine($""setX1 {value}""); } } + static LongInteger x2 { set { Console.WriteLine($""setX2 {value}""); } } + static LongInteger x3 { set { Console.WriteLine($""setX3 {value}""); } } + static LongInteger x4 { set { Console.WriteLine($""setX4 {value}""); } } + static LongInteger x5 { set { Console.WriteLine($""setX5 {value}""); } } + static LongInteger x6 { set { Console.WriteLine($""setX6 {value}""); } } + static LongInteger x7 { set { Console.WriteLine($""setX7 {value}""); } } + + static void Main() + { + ((x1, (x2, x3)), ((x4, x5), (x6, x7))) = Pair.Create(Pair.Create(new Integer(1), Pair.Create(new Integer(2), new Integer(3))), + Pair.Create(Pair.Create(new Integer(4), new Integer(5)), Pair.Create(new Integer(6), new Integer(7)))); + } +} +" + commonSource; + + string expected = +@"Deconstructing ((1, (2, 3)), ((4, 5), (6, 7))) +Deconstructing (1, (2, 3)) +Deconstructing (2, 3) +Deconstructing ((4, 5), (6, 7)) +Deconstructing (4, 5) +Deconstructing (6, 7) +Converting 1 +setX1 1 +Converting 2 +setX2 2 +Converting 3 +setX3 3 +Converting 4 +setX4 4 +Converting 5 +setX5 5 +Converting 6 +setX6 6 +Converting 7 +setX7 7"; + var comp = CompileAndVerify(source, expectedOutput: expected, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void MixOfAssignments() + { + string source = @" +class C +{ + static void Main() + { + long x; + string y; + + C a, b, c; + c = new C(); + (x, y) = a = b = c; + System.Console.WriteLine(x + "" "" + y); + } + + public void Deconstruct(out int a, out string b) + { + a = 1; + b = ""hello""; + } +} +"; + + var comp = CompileAndVerify(source, expectedOutput: "1 hello", parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } + + [Fact] + public void AssignWithPostfixOperator() + { + string source = @" +class C +{ + int state = 1; + + static void Main() + { + long x; + string y; + C c = new C(); + (x, y) = c++; + System.Console.WriteLine(x + "" "" + y); + } + + public void Deconstruct(out int a, out string b) + { + a = state; + b = ""hello""; + } + + public static C operator ++(C c1) + { + return new C() { state = 2 }; + } +} +"; + + var comp = CompileAndVerify(source, expectedOutput: "1 hello", parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics(); + } } } \ No newline at end of file diff --git a/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs b/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs index f76cee436a307..6863c9ea14de0 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs @@ -105,6 +105,29 @@ public T Current get { return _collection[_index]; } } } + + public static bool operator ==(OneOrMany first, OneOrMany second) + { + if (first.Count != second.Count) + { + return false; + } + + for (int i = 0; i < first.Count; i++) + { + if (first[i].Equals(second[i])) + { + return false; + } + } + + return true; + } + + public static bool operator !=(OneOrMany first, OneOrMany second) + { + return !(first == second); + } } internal static class OneOrMany @@ -118,5 +141,17 @@ public static OneOrMany Create(ImmutableArray many) { return new OneOrMany(many); } + + public static OneOrMany Create(ArrayBuilder some) + { + if (some.Count == 1) + { + return new OneOrMany(some[0]); + } + else + { + return new OneOrMany(some.ToImmutable()); + } + } } } From 86ca7d3b575ed57870be6e6ddc5fe18d2aa2870d Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 2 Jun 2016 10:08:00 -0700 Subject: [PATCH 02/13] Flattening variables in bound node --- .../Portable/Binder/Binder_Statements.cs | 36 ++++++++++++++++++- ...writer_DeconstructionAssignmentOperator.cs | 25 +++---------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index a9167edb1b3be..a2289c242c250 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1762,7 +1762,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax var assignments = assignmentSteps.ToImmutable(); TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); - return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, deconstructions, assignments, voidType); + return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, deconstructions, assignments, voidType); } finally { @@ -1978,6 +1978,40 @@ private BoundDeconstructionAssignmentOperator.AssignmentStep MakeAssignmentInfo( return new BoundDeconstructionAssignmentOperator.AssignmentStep() { Assignment = op, OutputPlaceholder = leftPlaceholder, TargetPlaceholder = rightPlaceholder }; } + private ImmutableArray FlattenDeconstructVariables(ImmutableArray variables) + { + bool isFlat = true; + foreach (var variable in variables) + { + if (variable.Kind == BoundKind.DeconstructionVariables) { isFlat = false; break; } + } + + if (isFlat) + { + return variables; + } + + var builder = ArrayBuilder.GetInstance(variables.Length); + FlattenDeconstructVariables(variables, builder); + + return builder.ToImmutableAndFree(); + } + + private void FlattenDeconstructVariables(ImmutableArray variables, ArrayBuilder builder) + { + foreach (var variable in variables) + { + if (variable.Kind == BoundKind.DeconstructionVariables) + { + FlattenDeconstructVariables(((BoundDeconstructionVariables)variable).Variables, builder); + } + else + { + builder.Add(variable); + } + } + } + /// /// Find the Deconstruct method for the expression on the right, that will fit the assignable bound expressions on the left. /// Returns true if the Deconstruct method is found. diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index d19c24578981e..7bb004e4405a1 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -66,29 +66,14 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct /// private ImmutableArray LhsSideEffects(ImmutableArray variables, ArrayBuilder temps, ArrayBuilder stores) { - var lhsReceivers = ArrayBuilder.GetInstance(); + var lhsReceivers = ArrayBuilder.GetInstance(variables.Length); - LhsSideEffects(variables, temps, stores, lhsReceivers); - - return lhsReceivers.ToImmutableAndFree(); - } - - /// - /// Adds the side effects to stores and adds temporaries to lhsReceivers. - /// - private void LhsSideEffects(ImmutableArray variables, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder lhsReceivers) - { foreach (var variable in variables) { - if (variable.Kind == BoundKind.DeconstructionVariables) - { - LhsSideEffects(((BoundDeconstructionVariables)variable).Variables, temps, stores, lhsReceivers); - } - else - { - lhsReceivers.Add(TransformCompoundAssignmentLHS(variable, stores, temps, isDynamicAssignment: false)); - } + lhsReceivers.Add(TransformCompoundAssignmentLHS(variable, stores, temps, isDynamicAssignment: false)); } + + return lhsReceivers.ToImmutableAndFree(); } private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, BoundDeconstructionAssignmentOperator.DeconstructStep deconstruction, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder placeholders) @@ -98,7 +83,6 @@ private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, Bound var tupleElementTypes = tupleType.TupleElementTypes; var numElements = tupleElementTypes.Length; - Debug.Assert(numElements == node.LeftVariables.Length); CSharpSyntaxNode syntax = node.Syntax; @@ -141,7 +125,6 @@ private void CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundDe // prepare out parameters for Deconstruct var deconstructParameters = deconstruction.DeconstructMemberOpt.Parameters; var outParametersBuilder = ArrayBuilder.GetInstance(deconstructParameters.Length); - Debug.Assert(deconstructParameters.Length == node.LeftVariables.Length); for (var i = 0; i < deconstructParameters.Length; i++) { From dd3b674d34cdcd4bef1ff0bca11f1b84da8350d8 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 2 Jun 2016 10:37:02 -0700 Subject: [PATCH 03/13] Fixing warning with OneOrMany --- ...writer_DeconstructionAssignmentOperator.cs | 9 ++++--- .../Portable/InternalUtilities/OneOrMany.cs | 25 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index 7bb004e4405a1..c59ff3b68f530 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -1,6 +1,5 @@ // 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 System; using System.Collections.Immutable; using System.Diagnostics; using System.Linq; @@ -12,6 +11,9 @@ internal sealed partial class LocalRewriter { public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstructionAssignmentOperator node) { + Debug.Assert(node.DeconstructSteps != null); + Debug.Assert(node.AssignmentSteps != null); + var temps = ArrayBuilder.GetInstance(); var stores = ArrayBuilder.GetInstance(); var placeholders = ArrayBuilder.GetInstance(); @@ -23,6 +25,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct BoundExpression loweredRight = VisitExpression(node.Right); AddPlaceholderReplacement(node.DeconstructSteps[0].TargetPlaceholder, loweredRight); placeholders.Add(node.DeconstructSteps[0].TargetPlaceholder); + foreach (var deconstruction in node.DeconstructSteps) { if (deconstruction.DeconstructMemberOpt == null) @@ -39,7 +42,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct int numAssignments = node.AssignmentSteps.Length; for (int i = 0; i < numAssignments; i++) { - // lower the assignment and replace the placeholders for source and target in the process + // lower the assignment and replace the placeholders for its outputs in the process var assignmentInfo = node.AssignmentSteps[i]; AddPlaceholderReplacement(assignmentInfo.OutputPlaceholder, lhsTemps[i]); @@ -86,7 +89,7 @@ private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, Bound CSharpSyntaxNode syntax = node.Syntax; - // save the loweredRight as we need to access it multiple times + // save the target as we need to access it multiple times BoundAssignmentOperator assignmentToTemp; var savedTuple = _factory.StoreToTemp(target, out assignmentToTemp); stores.Add(assignmentToTemp); diff --git a/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs b/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs index 6863c9ea14de0..16c6fda098273 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs @@ -108,14 +108,24 @@ public T Current public static bool operator ==(OneOrMany first, OneOrMany second) { - if (first.Count != second.Count) + return first.Equals(second); + } + + public static bool operator !=(OneOrMany first, OneOrMany second) + { + return !(first == second); + } + + public bool Equals(OneOrMany other) + { + if (Count != other.Count) { return false; } - for (int i = 0; i < first.Count; i++) + for (int i = 0; i < Count; i++) { - if (first[i].Equals(second[i])) + if (this[i].Equals(other[i])) { return false; } @@ -124,9 +134,14 @@ public T Current return true; } - public static bool operator !=(OneOrMany first, OneOrMany second) + public override bool Equals(object obj) { - return !(first == second); + return obj is OneOrMany && Equals((OneOrMany)obj); + } + + public override int GetHashCode() + { + return base.GetHashCode(); } } From 199c6ada15cfff08f463a98fc1ebe2afd4cc2092 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 2 Jun 2016 11:08:28 -0700 Subject: [PATCH 04/13] PR feedback (1) --- .../Portable/Binder/Binder_Statements.cs | 138 ++++++++---------- .../CSharp/Portable/BoundTree/BoundNodes.xml | 2 +- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 4 +- .../Portable/InternalUtilities/OneOrMany.cs | 50 ------- 4 files changed, 67 insertions(+), 127 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index a2289c242c250..03b9b35ff7f35 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -8,6 +8,8 @@ using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslyn.Utilities; +using DeconstructStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.DeconstructStep; +using AssignmentStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.AssignmentStep; namespace Microsoft.CodeAnalysis.CSharp { @@ -1730,35 +1732,35 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax SeparatedSyntaxList arguments = ((TupleExpressionSyntax)node.Left).Arguments; ImmutableArray checkedVariables = BindDeconstructionVariables(arguments, diagnostics); - var deconstructionSteps = ArrayBuilder.GetInstance(1); - var assignmentSteps = ArrayBuilder.GetInstance(1); - try + if ((object)boundRHS.Type == null) { - if ((object)boundRHS.Type == null) + if (boundRHS.Kind == BoundKind.TupleLiteral) { - 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); - boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); - } - else - { - // expression without type such as `null` - Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, node); - return BadExpression(node, checkedVariables.Concat(boundRHS).ToArray()); - } + // 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); } + else + { + // expression without type such as `null` + Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, node); + return BadExpression(node, checkedVariables.Concat(boundRHS).ToArray()); + } + } + var deconstructionSteps = ArrayBuilder.GetInstance(1); + var assignmentSteps = ArrayBuilder.GetInstance(1); + try + { if (!DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(node.Right, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps)) { - return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, OneOrMany.Create(ImmutableArray.Empty), - ImmutableArray.Empty, + return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, ImmutableArray.Empty, + ImmutableArray.Empty, ErrorTypeSymbol.UnknownResultType, hasErrors: true); } - var deconstructions = OneOrMany.Create(deconstructionSteps); + var deconstructions = deconstructionSteps.ToImmutable(); var assignments = assignmentSteps.ToImmutable(); TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); @@ -1781,72 +1783,74 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { Debug.Assert(targetPlaceholder.Type != null); + DeconstructStep step; + if (targetPlaceholder.Type.IsTupleType) { // tuple literal such as `(1, 2)`, `(null, null)`, `(x.P, y.M())` - return DeconstructTuple(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); + step = MakeTupleDeconstructStep(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); } else { - return DeconstructWithDeconstruct(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); + step = MakeNonTupleDeconstructStep(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); + + if (step == null) + { + return false; + } } + + deconstructionSteps.Add(step); + + // outputs will either need a conversion step and assignment step, or if they are nested variables, they will need further deconstruction + return DeconstructOrAssignOutputs(step, variables, syntax, diagnostics, deconstructionSteps, assignmentSteps); } /// /// This will generate and stack appropriate deconstruction and assignment steps for a tuple type. /// The produced deconstruction step has no Deconstruct method since the tuple already has distinct elements. - /// Returns false if there was an error. /// - private bool DeconstructTuple( + static private DeconstructStep MakeTupleDeconstructStep( BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { Debug.Assert(targetPlaceholder.Type.IsTupleType); var tupleTypes = targetPlaceholder.Type.TupleElementTypes; - // Add step for deconstructing this target - var deconstructionStep = new BoundDeconstructionAssignmentOperator.DeconstructStep() + var deconstructionStep = new DeconstructStep() { DeconstructMemberOpt = null, TargetPlaceholder = targetPlaceholder, - OutputPlaceholders = tupleTypes.SelectAsArray(type => new BoundDeconstructValuePlaceholder(syntax, type)) + OutputPlaceholders = tupleTypes.SelectAsArray((t, s) => new BoundDeconstructValuePlaceholder(s, t), syntax) }; - deconstructionSteps.Add(deconstructionStep); - - // outputs will either need a conversion step and assignment step, or if they are nested variables, they will need further deconstruction - if (!DeconstructOrAssignOutputs(deconstructionStep, variables, syntax, diagnostics, deconstructionSteps, assignmentSteps)) - { - return false; - } - - return true; + return deconstructionStep; } /// /// This will generate and stack appropriate deconstruction and assignment steps for a non-tuple type. - /// Returns false if there was an error (if a suitable Deconstruct method was not found). + /// Returns null if there was an error (if a suitable Deconstruct method was not found). /// - private bool DeconstructWithDeconstruct( + static private DeconstructStep MakeNonTupleDeconstructStep( BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { // symbol and parameters for Deconstruct - DiagnosticBag bag = new DiagnosticBag(); + var bag = DiagnosticBag.GetInstance(); MethodSymbol deconstructMethod = FindDeconstruct(variables.Length, targetPlaceholder, syntax, bag); if (!diagnostics.HasAnyErrors()) { @@ -1855,39 +1859,31 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax if ((object)deconstructMethod == null) { - return false; + return null; } var deconstructParameters = deconstructMethod.Parameters; - // add step for deconstructing this target - var deconstructionStep = new BoundDeconstructionAssignmentOperator.DeconstructStep() + var deconstructionStep = new DeconstructStep() { DeconstructMemberOpt = deconstructMethod, TargetPlaceholder = targetPlaceholder, - OutputPlaceholders = deconstructParameters.SelectAsArray(parameter => new BoundDeconstructValuePlaceholder(syntax, parameter.Type)) + OutputPlaceholders = deconstructParameters.SelectAsArray((p, s) => new BoundDeconstructValuePlaceholder(s, p.Type), syntax) }; - deconstructionSteps.Add(deconstructionStep); - // outputs will either need a conversion step and assignment step, or if they are nested variables, they will need further deconstruction - if (!DeconstructOrAssignOutputs(deconstructionStep, variables, syntax, diagnostics, deconstructionSteps, assignmentSteps)) - { - return false; - } - - return true; + return deconstructionStep; } /// /// Takes the outputs from the previous deconstructionStep and depending on the structure of variables, will generate further deconstructions, or simply assignments. /// public bool DeconstructOrAssignOutputs( - BoundDeconstructionAssignmentOperator.DeconstructStep deconstructionStep, + DeconstructStep deconstructionStep, ImmutableArray variables, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { for (int i = 0; i < variables.Length; i++) { @@ -1895,7 +1891,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax if (variable.Kind == BoundKind.DeconstructionVariables) { - var nestedVariables = ((BoundDeconstructionVariables)variables[i]).Variables; + var nestedVariables = ((BoundDeconstructionVariables)variable).Variables; if (!DeconstructIntoSteps(deconstructionStep.OutputPlaceholders[i], syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) { @@ -1915,14 +1911,14 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax /// /// For cases where the RHS of a deconstruction-assignment has not type (TupleLiteral), we squint and look at the LHS as a tuple type to give the RHS a type. /// - private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray topLevelCheckedVariables, DiagnosticBag diagnostics) + static private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) { var typesBuilder = ArrayBuilder.GetInstance(topLevelCheckedVariables.Length); foreach (var variable in topLevelCheckedVariables) { if (variable.Kind == BoundKind.DeconstructionVariables) { - typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(((BoundDeconstructionVariables)variable).Variables, diagnostics)); + typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(((BoundDeconstructionVariables)variable).Variables, diagnostics, compilation)); } else { @@ -1930,7 +1926,7 @@ private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray), elementNames: default(ImmutableArray), compilation: Compilation, diagnostics: diagnostics); + return TupleTypeSymbol.Create(locationOpt: null, elementTypes: typesBuilder.ToImmutableAndFree(), elementLocations: default(ImmutableArray), elementNames: default(ImmutableArray), compilation: compilation, diagnostics: diagnostics); } /// @@ -1968,25 +1964,19 @@ private ImmutableArray BindDeconstructionVariables(SeparatedSyn /// /// Figures out how to assign from sourceType into receivingVariable and bundles the information (leaving holes for the actual source and receiver) into an AssignmentInfo. /// - private BoundDeconstructionAssignmentOperator.AssignmentStep MakeAssignmentInfo(BoundExpression receivingVariable, TypeSymbol sourceType, BoundDeconstructValuePlaceholder rightPlaceholder, AssignmentExpressionSyntax node, DiagnosticBag diagnostics) + private AssignmentStep MakeAssignmentInfo(BoundExpression receivingVariable, TypeSymbol sourceType, BoundDeconstructValuePlaceholder rightPlaceholder, AssignmentExpressionSyntax node, DiagnosticBag diagnostics) { var leftPlaceholder = new BoundDeconstructValuePlaceholder(receivingVariable.Syntax, receivingVariable.Type) { WasCompilerGenerated = true }; // each assignment has a placeholder for a receiver and another for the source BoundAssignmentOperator op = BindAssignment(node, leftPlaceholder, rightPlaceholder, diagnostics); - return new BoundDeconstructionAssignmentOperator.AssignmentStep() { Assignment = op, OutputPlaceholder = leftPlaceholder, TargetPlaceholder = rightPlaceholder }; + return new AssignmentStep() { Assignment = op, OutputPlaceholder = leftPlaceholder, TargetPlaceholder = rightPlaceholder }; } - private ImmutableArray FlattenDeconstructVariables(ImmutableArray variables) + static private ImmutableArray FlattenDeconstructVariables(ImmutableArray variables) { - bool isFlat = true; - foreach (var variable in variables) - { - if (variable.Kind == BoundKind.DeconstructionVariables) { isFlat = false; break; } - } - - if (isFlat) + if (variables.All(v => v.Kind != BoundKind.DeconstructionVariables)) { return variables; } @@ -1997,7 +1987,7 @@ private ImmutableArray FlattenDeconstructVariables(ImmutableArr return builder.ToImmutableAndFree(); } - private void FlattenDeconstructVariables(ImmutableArray variables, ArrayBuilder builder) + static private void FlattenDeconstructVariables(ImmutableArray variables, ArrayBuilder builder) { foreach (var variable in variables) { diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index 48e197e0a4152..f46f9c248159e 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -397,7 +397,7 @@ - + diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index a2162c046b0c4..ba8a17ea7cbc5 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -1640,7 +1640,7 @@ public void Deconstruct(out int a, out string b) comp.VerifyDiagnostics(); } - [Fact] + [Fact(Skip = "PROTOTYPE(tuples)")] public void AssignWithPostfixOperator() { string source = @" @@ -1669,7 +1669,7 @@ public void Deconstruct(out int a, out string b) } } "; - + // PROTOTYPE(tuples) we expect "2 hello" instead var comp = CompileAndVerify(source, expectedOutput: "1 hello", parseOptions: TestOptions.Regular.WithTuplesFeature()); comp.VerifyDiagnostics(); } diff --git a/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs b/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs index 16c6fda098273..f76cee436a307 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/OneOrMany.cs @@ -105,44 +105,6 @@ public T Current get { return _collection[_index]; } } } - - public static bool operator ==(OneOrMany first, OneOrMany second) - { - return first.Equals(second); - } - - public static bool operator !=(OneOrMany first, OneOrMany second) - { - return !(first == second); - } - - public bool Equals(OneOrMany other) - { - if (Count != other.Count) - { - return false; - } - - for (int i = 0; i < Count; i++) - { - if (this[i].Equals(other[i])) - { - return false; - } - } - - return true; - } - - public override bool Equals(object obj) - { - return obj is OneOrMany && Equals((OneOrMany)obj); - } - - public override int GetHashCode() - { - return base.GetHashCode(); - } } internal static class OneOrMany @@ -156,17 +118,5 @@ public static OneOrMany Create(ImmutableArray many) { return new OneOrMany(many); } - - public static OneOrMany Create(ArrayBuilder some) - { - if (some.Count == 1) - { - return new OneOrMany(some[0]); - } - else - { - return new OneOrMany(some.ToImmutable()); - } - } } } From 461b4196899098d1e0a6f78b3c8af16db8ce0145 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 09:03:34 -0700 Subject: [PATCH 05/13] Adding error for tuple cardinality mismatch --- .../Portable/Binder/Binder_Statements.cs | 15 ++++-- .../Portable/CSharpResources.Designer.cs | 9 ++++ .../CSharp/Portable/CSharpResources.resx | 5 +- .../CSharp/Portable/Errors/ErrorCode.cs | 1 + .../Emit/CodeGen/CodeGenDeconstructTests.cs | 50 +++++++++++++++++++ 5 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 03b9b35ff7f35..2631763c72a93 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -8,8 +8,8 @@ using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslyn.Utilities; -using DeconstructStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.DeconstructStep; using AssignmentStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.AssignmentStep; +using DeconstructStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.DeconstructStep; namespace Microsoft.CodeAnalysis.CSharp { @@ -1798,11 +1798,11 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax else { step = MakeNonTupleDeconstructStep(targetPlaceholder, syntax, diagnostics, variables, deconstructionSteps, assignmentSteps); + } - if (step == null) - { - return false; - } + if (step == null) + { + return false; } deconstructionSteps.Add(step); @@ -1826,6 +1826,11 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax Debug.Assert(targetPlaceholder.Type.IsTupleType); var tupleTypes = targetPlaceholder.Type.TupleElementTypes; + if (variables.Length != tupleTypes.Length) + { + Error(diagnostics, ErrorCode.ERR_DeconstructWrongCardinality, syntax, tupleTypes.Length, variables.Length); + return null; + } var deconstructionStep = new DeconstructStep() { diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs index 1b53d6a1af540..49c5e930aca20 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs +++ b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs @@ -3112,6 +3112,15 @@ internal class CSharpResources { } } + /// + /// Looks up a localized string similar to Cannot deconstruct a tuple of '{0}' elements into '{1}' variables.. + /// + internal static string ERR_DeconstructWrongCardinality { + get { + return ResourceManager.GetString("ERR_DeconstructWrongCardinality", resourceCulture); + } + } + /// /// Looks up a localized string similar to The Deconstruct method for type '{0}' doesn't have the number of parameters ({1}) needed for this deconstruction.. /// diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index aafe2102d7d64..cce4a2d44fe35 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -4899,4 +4899,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ Option '{0}' must be an absolute path. - + + Cannot deconstruct a tuple of '{0}' elements into '{1}' variables. + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index 300bb23f27bec..cd9f590566a3e 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1399,5 +1399,6 @@ internal enum ErrorCode ERR_DeconstructRequiresOutParams = 8208, ERR_DeconstructWrongParams = 8209, ERR_DeconstructRequiresExpression = 8210, + ERR_DeconstructWrongCardinality = 8211, } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index ba8a17ea7cbc5..6aad171a4713b 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -1673,5 +1673,55 @@ public void Deconstruct(out int a, out string b) var comp = CompileAndVerify(source, expectedOutput: "1 hello", parseOptions: TestOptions.Regular.WithTuplesFeature()); comp.VerifyDiagnostics(); } + + [Fact] + public void TupleWithWrongCardinality() + { + string source = @" +class C +{ + static void Main() + { + int x, y, z; + + (x, y, z) = MakePair(); + } + + public static (int, int) MakePair() + { + return (42, 42); + } +} +"; + var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics( + // (8,9): error CS8211: Cannot deconstruct a tuple of '2' elements into '3' variables. + // (x, y, z) = MakePair(); + Diagnostic(ErrorCode.ERR_DeconstructWrongCardinality, "(x, y, z) = MakePair()").WithArguments("2", "3").WithLocation(8, 9) + ); + } + + [Fact] + public void NestedTupleWithWrongCardinality() + { + string source = @" +class C +{ + static void Main() + { + int x, y, z, w; + + (x, (y, z, w)) = Pair.Create(42, (43, 44)); + } +} +" + commonSource; + + var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }, parseOptions: TestOptions.Regular.WithTuplesFeature()); + comp.VerifyDiagnostics( + // (8,9): error CS8211: Cannot deconstruct a tuple of '2' elements into '3' variables. + // (x, (y, z, w)) = Pair.Create(42, (43, 44)); + Diagnostic(ErrorCode.ERR_DeconstructWrongCardinality, "(x, (y, z, w)) = Pair.Create(42, (43, 44))").WithArguments("2", "3").WithLocation(8, 9) + ); + } } } \ No newline at end of file From 6e68b8d18393ec957d1504ba090bf4cd859a376d Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 12:13:43 -0700 Subject: [PATCH 06/13] PR feedback (2) --- .../CSharp/Portable/Binder/Binder_Statements.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 2631763c72a93..4cae731d084ed 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1882,7 +1882,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax /// /// Takes the outputs from the previous deconstructionStep and depending on the structure of variables, will generate further deconstructions, or simply assignments. /// - public bool DeconstructOrAssignOutputs( + private bool DeconstructOrAssignOutputs( DeconstructStep deconstructionStep, ImmutableArray variables, AssignmentExpressionSyntax syntax, @@ -1893,19 +1893,20 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax for (int i = 0; i < variables.Length; i++) { var variable = variables[i]; + var valuePlaceholder = deconstructionStep.OutputPlaceholders[i]; if (variable.Kind == BoundKind.DeconstructionVariables) { var nestedVariables = ((BoundDeconstructionVariables)variable).Variables; - if (!DeconstructIntoSteps(deconstructionStep.OutputPlaceholders[i], syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) + if (!DeconstructIntoSteps(valuePlaceholder, syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) { return false; } } else { - var assignment = MakeAssignmentInfo(variable, deconstructionStep.OutputPlaceholders[i].Type, deconstructionStep.OutputPlaceholders[i], syntax, diagnostics); + var assignment = MakeAssignmentInfo(variable, valuePlaceholder.Type, valuePlaceholder, syntax, diagnostics); assignmentSteps.Add(assignment); } } @@ -1914,7 +1915,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax } /// - /// For cases where the RHS of a deconstruction-assignment has not 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-assignment has no type (TupleLiteral), we squint and look at the LHS as a tuple type to give the RHS a type. /// static private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) { From 3b6d01254557d1705ed5e6bfe55f3698ac1f613f Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 12:31:08 -0700 Subject: [PATCH 07/13] Removing nested variables from data flow analysis, since now flattened --- .../Portable/Binder/Binder_Statements.cs | 2 +- .../Portable/FlowAnalysis/DataFlowPass.cs | 28 +------------------ .../FlowAnalysis/PreciseAbstractFlowPass.cs | 22 ++------------- 3 files changed, 4 insertions(+), 48 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 4cae731d084ed..d88f154649bcc 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1754,7 +1754,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax { if (!DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(node.Right, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps)) { - return new BoundDeconstructionAssignmentOperator(node, checkedVariables, boundRHS, ImmutableArray.Empty, + return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, ImmutableArray.Empty, ImmutableArray.Empty, ErrorTypeSymbol.UnknownResultType, hasErrors: true); diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs index d63fdf44e2fb6..b314f0a94b584 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs @@ -1728,33 +1728,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct foreach (BoundExpression variable in node.LeftVariables) { - if (variable.Kind == BoundKind.DeconstructionVariables) - { - VisitDeconstructionVariables((BoundDeconstructionVariables)variable); - } - else - { - Assign(variable, null, refKind: RefKind.None); - } - } - - return null; - } - - public override BoundNode VisitDeconstructionVariables(BoundDeconstructionVariables node) - { - base.VisitDeconstructionVariables(node); - - foreach (BoundExpression variable in node.Variables) - { - if (variable.Kind == BoundKind.DeconstructionVariables) - { - VisitDeconstructionVariables((BoundDeconstructionVariables)variable); - } - else - { - Assign(variable, null, refKind: RefKind.None); - } + Assign(variable, null, refKind: RefKind.None); } return null; diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs index 08f003580c3d8..e881b4ecf3b60 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs @@ -1518,14 +1518,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct { foreach (BoundExpression variable in node.LeftVariables) { - if (variable.Kind == BoundKind.DeconstructionVariables) - { - VisitDeconstructionVariables((BoundDeconstructionVariables)variable); - } - else - { - VisitLvalue(variable); - } + VisitLvalue(variable); } VisitRvalue(node.Right); @@ -1535,18 +1528,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct public override BoundNode VisitDeconstructionVariables(BoundDeconstructionVariables node) { - foreach (BoundExpression variable in node.Variables) - { - if (variable.Kind == BoundKind.DeconstructionVariables) - { - VisitDeconstructionVariables((BoundDeconstructionVariables)variable); - } - else - { - VisitLvalue(variable); - } - } - + Debug.Assert(false, "BoundDeconstructionVariables should not make it past binding stage."); return null; } From 6f354a1a256ccbeb93193c467e8450d6605819a7 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 18:13:25 -0700 Subject: [PATCH 08/13] PR feedback (3) --- .../Portable/Binder/Binder_Statements.cs | 71 +++++++------------ .../Portable/BoundTree/BoundExpression.cs | 17 ----- .../CSharp/Portable/BoundTree/BoundNodes.xml | 18 +++-- ...writer_DeconstructionAssignmentOperator.cs | 8 +-- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 5 +- 5 files changed, 46 insertions(+), 73 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index d88f154649bcc..ceb5cd1e4199a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -8,8 +8,6 @@ using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslyn.Utilities; -using AssignmentStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.AssignmentStep; -using DeconstructStep = Microsoft.CodeAnalysis.CSharp.BoundDeconstructionAssignmentOperator.DeconstructStep; namespace Microsoft.CodeAnalysis.CSharp { @@ -1744,18 +1742,18 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax { // expression without type such as `null` Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, node); - return BadExpression(node, checkedVariables.Concat(boundRHS).ToArray()); + return BadExpression(node, FlattenDeconstructVariables(checkedVariables).Concat(boundRHS).ToArray()); } } - var deconstructionSteps = ArrayBuilder.GetInstance(1); - var assignmentSteps = ArrayBuilder.GetInstance(1); + var deconstructionSteps = ArrayBuilder.GetInstance(1); + var assignmentSteps = ArrayBuilder.GetInstance(1); try { if (!DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(node.Right, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps)) { - return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, ImmutableArray.Empty, - ImmutableArray.Empty, + return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, ImmutableArray.Empty, + ImmutableArray.Empty, ErrorTypeSymbol.UnknownResultType, hasErrors: true); } @@ -1783,12 +1781,12 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { Debug.Assert(targetPlaceholder.Type != null); - DeconstructStep step; + BoundDeconstructionDeconstructStep step; if (targetPlaceholder.Type.IsTupleType) { @@ -1815,13 +1813,13 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax /// This will generate and stack appropriate deconstruction and assignment steps for a tuple type. /// The produced deconstruction step has no Deconstruct method since the tuple already has distinct elements. /// - static private DeconstructStep MakeTupleDeconstructStep( + static private BoundDeconstructionDeconstructStep MakeTupleDeconstructStep( BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { Debug.Assert(targetPlaceholder.Type.IsTupleType); @@ -1832,63 +1830,42 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax return null; } - var deconstructionStep = new DeconstructStep() - { - DeconstructMemberOpt = null, - TargetPlaceholder = targetPlaceholder, - OutputPlaceholders = tupleTypes.SelectAsArray((t, s) => new BoundDeconstructValuePlaceholder(s, t), syntax) - }; - - return deconstructionStep; + return new BoundDeconstructionDeconstructStep(syntax, null, targetPlaceholder, tupleTypes.SelectAsArray((t, s) => new BoundDeconstructValuePlaceholder(s, t), syntax)); } /// /// This will generate and stack appropriate deconstruction and assignment steps for a non-tuple type. /// Returns null if there was an error (if a suitable Deconstruct method was not found). /// - static private DeconstructStep MakeNonTupleDeconstructStep( + static private BoundDeconstructionDeconstructStep MakeNonTupleDeconstructStep( BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { // symbol and parameters for Deconstruct - var bag = DiagnosticBag.GetInstance(); - MethodSymbol deconstructMethod = FindDeconstruct(variables.Length, targetPlaceholder, syntax, bag); - if (!diagnostics.HasAnyErrors()) - { - diagnostics.AddRange(bag); - } + MethodSymbol deconstructMethod = FindDeconstruct(variables.Length, targetPlaceholder, syntax, diagnostics); if ((object)deconstructMethod == null) { return null; } - var deconstructParameters = deconstructMethod.Parameters; - - var deconstructionStep = new DeconstructStep() - { - DeconstructMemberOpt = deconstructMethod, - TargetPlaceholder = targetPlaceholder, - OutputPlaceholders = deconstructParameters.SelectAsArray((p, s) => new BoundDeconstructValuePlaceholder(s, p.Type), syntax) - }; - - return deconstructionStep; + return new BoundDeconstructionDeconstructStep(syntax, deconstructMethod, targetPlaceholder, deconstructMethod.Parameters.SelectAsArray((p, s) => new BoundDeconstructValuePlaceholder(s, p.Type), syntax)); } /// /// Takes the outputs from the previous deconstructionStep and depending on the structure of variables, will generate further deconstructions, or simply assignments. /// private bool DeconstructOrAssignOutputs( - DeconstructStep deconstructionStep, + BoundDeconstructionDeconstructStep deconstructionStep, ImmutableArray variables, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { for (int i = 0; i < variables.Length; i++) { @@ -1970,14 +1947,14 @@ private ImmutableArray BindDeconstructionVariables(SeparatedSyn /// /// Figures out how to assign from sourceType into receivingVariable and bundles the information (leaving holes for the actual source and receiver) into an AssignmentInfo. /// - private AssignmentStep MakeAssignmentInfo(BoundExpression receivingVariable, TypeSymbol sourceType, BoundDeconstructValuePlaceholder rightPlaceholder, AssignmentExpressionSyntax node, DiagnosticBag diagnostics) + private BoundDeconstructionAssignmentStep MakeAssignmentInfo(BoundExpression receivingVariable, TypeSymbol sourceType, BoundDeconstructValuePlaceholder inputPlaceholder, AssignmentExpressionSyntax node, DiagnosticBag diagnostics) { - var leftPlaceholder = new BoundDeconstructValuePlaceholder(receivingVariable.Syntax, receivingVariable.Type) { WasCompilerGenerated = true }; + var outputPlaceholder = new BoundDeconstructValuePlaceholder(receivingVariable.Syntax, receivingVariable.Type) { WasCompilerGenerated = true }; // each assignment has a placeholder for a receiver and another for the source - BoundAssignmentOperator op = BindAssignment(node, leftPlaceholder, rightPlaceholder, diagnostics); + BoundAssignmentOperator op = BindAssignment(node, outputPlaceholder, inputPlaceholder, diagnostics); - return new AssignmentStep() { Assignment = op, OutputPlaceholder = leftPlaceholder, TargetPlaceholder = rightPlaceholder }; + return new BoundDeconstructionAssignmentStep(node, op, inputPlaceholder, outputPlaceholder); } static private ImmutableArray FlattenDeconstructVariables(ImmutableArray variables) diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs index 24c1d2e54c60b..b0243a2ea198a 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs @@ -278,23 +278,6 @@ public override Symbol ExpressionSymbol public ImmutableArray OriginalUserDefinedOperatorsOpt { get; } } - internal sealed partial class BoundDeconstructionAssignmentOperator : BoundExpression - { - internal class AssignmentStep - { - public BoundAssignmentOperator Assignment; - public BoundDeconstructValuePlaceholder OutputPlaceholder; - public BoundDeconstructValuePlaceholder TargetPlaceholder; - } - - internal class DeconstructStep - { - public MethodSymbol DeconstructMemberOpt; // the deconstruct member, or null if tuple deconstruction - public BoundDeconstructValuePlaceholder TargetPlaceholder; - public ImmutableArray OutputPlaceholders; // placeholders for the outputs produced by this deconstruction - } - } - internal partial class BoundCompoundAssignmentOperator { public override Symbol ExpressionSymbol diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index f46f9c248159e..24ca99f05dfeb 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -397,15 +397,25 @@ - + - - + + + + + + + + + + + + + - diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index c59ff3b68f530..8170e63c4c37b 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -19,7 +19,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct var placeholders = ArrayBuilder.GetInstance(); // evaluate left-hand-side side-effects - var lhsTemps = LhsSideEffects(node.LeftVariables, temps, stores); + var lhsTemps = LeftHandSideSideEffects(node.LeftVariables, temps, stores); // get or make right-hand-side values BoundExpression loweredRight = VisitExpression(node.Right); @@ -67,7 +67,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct /// /// Adds the side effects to stores and returns temporaries (as a flat list) to access them. /// - private ImmutableArray LhsSideEffects(ImmutableArray variables, ArrayBuilder temps, ArrayBuilder stores) + private ImmutableArray LeftHandSideSideEffects(ImmutableArray variables, ArrayBuilder temps, ArrayBuilder stores) { var lhsReceivers = ArrayBuilder.GetInstance(variables.Length); @@ -79,7 +79,7 @@ private ImmutableArray LhsSideEffects(ImmutableArray temps, ArrayBuilder stores, ArrayBuilder placeholders) + private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, BoundDeconstructionDeconstructStep deconstruction, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder placeholders) { var target = PlaceholderReplacement(deconstruction.TargetPlaceholder); var tupleType = target.Type.IsTupleType ? target.Type : TupleTypeSymbol.Create((NamedTypeSymbol)target.Type); @@ -119,7 +119,7 @@ private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, Bound /// Adds a invocation of Deconstruct with those as out parameters onto the 'stores' sequence /// Returns the expressions for those out parameters /// - private void CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundDeconstructionAssignmentOperator.DeconstructStep deconstruction, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder placeholders) + private void CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundDeconstructionDeconstructStep deconstruction, ArrayBuilder temps, ArrayBuilder stores, ArrayBuilder placeholders) { Debug.Assert((object)deconstruction.DeconstructMemberOpt != null); diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index 6aad171a4713b..460bfb47f52a1 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -203,7 +203,10 @@ static void Main() Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "f").WithArguments("long", "f").WithLocation(8, 12), // (8,17): error CS1061: 'string' does not contain a definition for 'g' and no extension method 'g' accepting a first argument of type 'string' could be found (are you missing a using directive or an assembly reference?) // (x.f, y.g) = new C(); - Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "g").WithArguments("string", "g").WithLocation(8, 17) + Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "g").WithArguments("string", "g").WithLocation(8, 17), + // (8,22): error CS8209: The Deconstruct method for type 'C.Deconstruct()' doesn't have the number of parameters (2) needed for this deconstruction. + // (x.f, y.g) = new C(); + Diagnostic(ErrorCode.ERR_DeconstructWrongParams, "new C()").WithArguments("C.Deconstruct()", "2").WithLocation(8, 22) ); } From f205d5f98d0de3b92b342194bd0d7e6f6555f112 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 19:01:54 -0700 Subject: [PATCH 09/13] Don't use a bound node for nested deconstruction variables --- .../Portable/Binder/Binder_Statements.cs | 65 +++++++++++-------- .../CSharp/Portable/BoundTree/BoundNodes.xml | 5 -- .../CSharp/Portable/BoundTree/Expression.cs | 15 ----- .../FlowAnalysis/PreciseAbstractFlowPass.cs | 6 -- 4 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index ceb5cd1e4199a..9b93349222325 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1728,7 +1728,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax var boundRHS = BindValue(node.Right, diagnostics, BindValueKind.RValue); SeparatedSyntaxList arguments = ((TupleExpressionSyntax)node.Left).Arguments; - ImmutableArray checkedVariables = BindDeconstructionVariables(arguments, diagnostics); + ImmutableArray checkedVariables = BindDeconstructionVariables(arguments, diagnostics); if ((object)boundRHS.Type == null) { @@ -1780,7 +1780,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, - ImmutableArray variables, + ImmutableArray variables, ArrayBuilder deconstructionSteps, ArrayBuilder assignmentSteps) { @@ -1817,7 +1817,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, - ImmutableArray variables, + ImmutableArray variables, ArrayBuilder deconstructionSteps, ArrayBuilder assignmentSteps) { @@ -1841,7 +1841,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax BoundDeconstructValuePlaceholder targetPlaceholder, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, - ImmutableArray variables, + ImmutableArray variables, ArrayBuilder deconstructionSteps, ArrayBuilder assignmentSteps) { @@ -1856,12 +1856,30 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax return new BoundDeconstructionDeconstructStep(syntax, deconstructMethod, targetPlaceholder, deconstructMethod.Parameters.SelectAsArray((p, s) => new BoundDeconstructValuePlaceholder(s, p.Type), syntax)); } + private class DeconstructionVariable + { + public BoundExpression Single; + public ImmutableArray? Nested; + + public DeconstructionVariable(BoundExpression variable) + { + Single = variable; + Nested = null; + } + + public DeconstructionVariable(ImmutableArray variables) + { + Single = null; + Nested = variables; + } + } + /// /// Takes the outputs from the previous deconstructionStep and depending on the structure of variables, will generate further deconstructions, or simply assignments. /// private bool DeconstructOrAssignOutputs( BoundDeconstructionDeconstructStep deconstructionStep, - ImmutableArray variables, + ImmutableArray variables, AssignmentExpressionSyntax syntax, DiagnosticBag diagnostics, ArrayBuilder deconstructionSteps, @@ -1872,9 +1890,9 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax var variable = variables[i]; var valuePlaceholder = deconstructionStep.OutputPlaceholders[i]; - if (variable.Kind == BoundKind.DeconstructionVariables) + if (variable.Nested.HasValue) { - var nestedVariables = ((BoundDeconstructionVariables)variable).Variables; + var nestedVariables = variable.Nested.Value; if (!DeconstructIntoSteps(valuePlaceholder, syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) { @@ -1883,7 +1901,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax } else { - var assignment = MakeAssignmentInfo(variable, valuePlaceholder.Type, valuePlaceholder, syntax, diagnostics); + var assignment = MakeAssignmentInfo(variable.Single, valuePlaceholder.Type, valuePlaceholder, syntax, diagnostics); assignmentSteps.Add(assignment); } } @@ -1894,18 +1912,18 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax /// /// 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. /// - static private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) + static private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) { var typesBuilder = ArrayBuilder.GetInstance(topLevelCheckedVariables.Length); foreach (var variable in topLevelCheckedVariables) { - if (variable.Kind == BoundKind.DeconstructionVariables) + if (variable.Nested.HasValue) { - typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(((BoundDeconstructionVariables)variable).Variables, diagnostics, compilation)); + typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(variable.Nested.Value, diagnostics, compilation)); } else { - typesBuilder.Add(variable.Type); + typesBuilder.Add(variable.Single.Type); } } @@ -1916,27 +1934,27 @@ static private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray - private ImmutableArray BindDeconstructionVariables(SeparatedSyntaxList arguments, DiagnosticBag diagnostics) + private ImmutableArray BindDeconstructionVariables(SeparatedSyntaxList arguments, DiagnosticBag diagnostics) { int numElements = arguments.Count; Debug.Assert(numElements >= 2); // this should not have parsed as a tuple. // bind the variables and check they can be assigned to - var checkedVariablesBuilder = ArrayBuilder.GetInstance(numElements); + var checkedVariablesBuilder = ArrayBuilder.GetInstance(numElements); foreach (var argument in arguments) { if (argument.Expression.Kind() == SyntaxKind.TupleExpression) // nested tuple case { var nestedArguments = ((TupleExpressionSyntax)argument.Expression).Arguments; - checkedVariablesBuilder.Add(new BoundDeconstructionVariables(argument, BindDeconstructionVariables(nestedArguments, diagnostics))); + checkedVariablesBuilder.Add(new DeconstructionVariable(BindDeconstructionVariables(nestedArguments, diagnostics))); } else { var boundVariable = BindExpression(argument.Expression, diagnostics, invoked: false, indexed: false); var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignment, diagnostics); - checkedVariablesBuilder.Add(checkedVariable); + checkedVariablesBuilder.Add(new DeconstructionVariable(checkedVariable)); } } @@ -1957,30 +1975,25 @@ private BoundDeconstructionAssignmentStep MakeAssignmentInfo(BoundExpression rec return new BoundDeconstructionAssignmentStep(node, op, inputPlaceholder, outputPlaceholder); } - static private ImmutableArray FlattenDeconstructVariables(ImmutableArray variables) + static private ImmutableArray FlattenDeconstructVariables(ImmutableArray variables) { - if (variables.All(v => v.Kind != BoundKind.DeconstructionVariables)) - { - return variables; - } - var builder = ArrayBuilder.GetInstance(variables.Length); FlattenDeconstructVariables(variables, builder); return builder.ToImmutableAndFree(); } - static private void FlattenDeconstructVariables(ImmutableArray variables, ArrayBuilder builder) + static private void FlattenDeconstructVariables(ImmutableArray variables, ArrayBuilder builder) { foreach (var variable in variables) { - if (variable.Kind == BoundKind.DeconstructionVariables) + if (variable.Nested.HasValue) { - FlattenDeconstructVariables(((BoundDeconstructionVariables)variable).Variables, builder); + FlattenDeconstructVariables(variable.Nested.Value, builder); } else { - builder.Add(variable); + builder.Add(variable.Single); } } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index 24ca99f05dfeb..677404f63ca20 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -414,11 +414,6 @@ - - - - - diff --git a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs index 711918aabe581..19fc6c1aa4211 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs @@ -1034,21 +1034,6 @@ public override void Accept(OperationVisitor visitor) } } - internal sealed partial class BoundDeconstructionVariables : BoundExpression - { - protected override OperationKind ExpressionKind => OperationKind.None; - - public override void Accept(OperationVisitor visitor) - { - visitor.VisitNoneOperation(this); - } - - public override TResult Accept(OperationVisitor visitor, TArgument argument) - { - return visitor.VisitNoneOperation(this, argument); - } - } - internal sealed partial class BoundVoid : BoundExpression { protected override OperationKind ExpressionKind => OperationKind.None; diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs index e881b4ecf3b60..aab14c7e3ffdc 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs @@ -1526,12 +1526,6 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct return null; } - public override BoundNode VisitDeconstructionVariables(BoundDeconstructionVariables node) - { - Debug.Assert(false, "BoundDeconstructionVariables should not make it past binding stage."); - return null; - } - public override BoundNode VisitCompoundAssignmentOperator(BoundCompoundAssignmentOperator node) { // TODO: should events be handled specially too? From ba975f7cb9ea6ce071ead81f33787cf0dac4304a Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 19:09:55 -0700 Subject: [PATCH 10/13] Capture as many deconstruction and assignment steps as possible when error --- .../Portable/Binder/Binder_Statements.cs | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 9b93349222325..f7a515b33e7e1 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1750,19 +1750,13 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax var assignmentSteps = ArrayBuilder.GetInstance(1); try { - if (!DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(node.Right, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps)) - { - return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, ImmutableArray.Empty, - ImmutableArray.Empty, - ErrorTypeSymbol.UnknownResultType, - hasErrors: true); - } + bool hasErrors = !DeconstructIntoSteps(new BoundDeconstructValuePlaceholder(node.Right, boundRHS.Type), node, diagnostics, checkedVariables, deconstructionSteps, assignmentSteps); var deconstructions = deconstructionSteps.ToImmutable(); var assignments = assignmentSteps.ToImmutable(); TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); - return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, deconstructions, assignments, voidType); + return new BoundDeconstructionAssignmentOperator(node, FlattenDeconstructVariables(checkedVariables), boundRHS, deconstructions, assignments, voidType, hasErrors: hasErrors); } finally { @@ -1814,12 +1808,12 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax /// The produced deconstruction step has no Deconstruct method since the tuple already has distinct elements. /// static private BoundDeconstructionDeconstructStep MakeTupleDeconstructStep( - BoundDeconstructValuePlaceholder targetPlaceholder, - AssignmentExpressionSyntax syntax, - DiagnosticBag diagnostics, - ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + BoundDeconstructValuePlaceholder targetPlaceholder, + AssignmentExpressionSyntax syntax, + DiagnosticBag diagnostics, + ImmutableArray variables, + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { Debug.Assert(targetPlaceholder.Type.IsTupleType); @@ -1838,12 +1832,12 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax /// Returns null if there was an error (if a suitable Deconstruct method was not found). /// static private BoundDeconstructionDeconstructStep MakeNonTupleDeconstructStep( - BoundDeconstructValuePlaceholder targetPlaceholder, - AssignmentExpressionSyntax syntax, - DiagnosticBag diagnostics, - ImmutableArray variables, - ArrayBuilder deconstructionSteps, - ArrayBuilder assignmentSteps) + BoundDeconstructValuePlaceholder targetPlaceholder, + AssignmentExpressionSyntax syntax, + DiagnosticBag diagnostics, + ImmutableArray variables, + ArrayBuilder deconstructionSteps, + ArrayBuilder assignmentSteps) { // symbol and parameters for Deconstruct MethodSymbol deconstructMethod = FindDeconstruct(variables.Length, targetPlaceholder, syntax, diagnostics); @@ -1876,6 +1870,7 @@ public DeconstructionVariable(ImmutableArray variables) /// /// Takes the outputs from the previous deconstructionStep and depending on the structure of variables, will generate further deconstructions, or simply assignments. + /// Returns true for success, but false if has errors. /// private bool DeconstructOrAssignOutputs( BoundDeconstructionDeconstructStep deconstructionStep, @@ -1885,6 +1880,8 @@ public DeconstructionVariable(ImmutableArray variables) ArrayBuilder deconstructionSteps, ArrayBuilder assignmentSteps) { + bool hasErrors = false; + for (int i = 0; i < variables.Length; i++) { var variable = variables[i]; @@ -1896,7 +1893,7 @@ public DeconstructionVariable(ImmutableArray variables) if (!DeconstructIntoSteps(valuePlaceholder, syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) { - return false; + hasErrors = true; } } else @@ -1906,7 +1903,7 @@ public DeconstructionVariable(ImmutableArray variables) } } - return true; + return !hasErrors; } /// From 1748df10488e8edc556e7e42f78776a263f92fb7 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 3 Jun 2016 20:01:10 -0700 Subject: [PATCH 11/13] Adding PROTOTYPE markers --- src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs | 3 ++- .../LocalRewriter_DeconstructionAssignmentOperator.cs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs index b314f0a94b584..ef66ff6fc8d7e 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs @@ -1728,7 +1728,8 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct foreach (BoundExpression variable in node.LeftVariables) { - Assign(variable, null, refKind: RefKind.None); + // PROTOTYPE(tuples) value should not be set to null + Assign(variable, value: null, refKind: RefKind.None); } return null; diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index 8170e63c4c37b..8953f400037c5 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -73,6 +73,7 @@ private ImmutableArray LeftHandSideSideEffects(ImmutableArray Date: Mon, 6 Jun 2016 11:19:10 -0700 Subject: [PATCH 12/13] PR feedback (4) --- .../Portable/Binder/Binder_Statements.cs | 20 +++++++++---------- .../Lowering/LocalRewriter/LocalRewriter.cs | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index f7a515b33e7e1..893aa28e8449d 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1853,12 +1853,12 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax private class DeconstructionVariable { public BoundExpression Single; - public ImmutableArray? Nested; + public ImmutableArray Nested; public DeconstructionVariable(BoundExpression variable) { Single = variable; - Nested = null; + Nested = default(ImmutableArray); } public DeconstructionVariable(ImmutableArray variables) @@ -1866,6 +1866,8 @@ public DeconstructionVariable(ImmutableArray variables) Single = null; Nested = variables; } + + public bool IsNested => Nested != default(ImmutableArray); } /// @@ -1887,11 +1889,9 @@ public DeconstructionVariable(ImmutableArray variables) var variable = variables[i]; var valuePlaceholder = deconstructionStep.OutputPlaceholders[i]; - if (variable.Nested.HasValue) + if (variable.IsNested) { - var nestedVariables = variable.Nested.Value; - - if (!DeconstructIntoSteps(valuePlaceholder, syntax, diagnostics, nestedVariables, deconstructionSteps, assignmentSteps)) + if (!DeconstructIntoSteps(valuePlaceholder, syntax, diagnostics, variable.Nested, deconstructionSteps, assignmentSteps)) { hasErrors = true; } @@ -1914,9 +1914,9 @@ static private TypeSymbol MakeTupleTypeFromDeconstructionLHS(ImmutableArray.GetInstance(topLevelCheckedVariables.Length); foreach (var variable in topLevelCheckedVariables) { - if (variable.Nested.HasValue) + if (variable.IsNested) { - typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(variable.Nested.Value, diagnostics, compilation)); + typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(variable.Nested, diagnostics, compilation)); } else { @@ -1984,9 +1984,9 @@ static private void FlattenDeconstructVariables(ImmutableArray /// Remove all the listed placeholders. /// - private void RemovePlaceholderReplacements(IEnumerable placeholders) + private void RemovePlaceholderReplacements(ArrayBuilder placeholders) { foreach (var placeholder in placeholders) { From 87ee4f41d17066c900042187af645d6d3dc83b50 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 6 Jun 2016 14:40:13 -0700 Subject: [PATCH 13/13] PR feedback (5) --- src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs | 6 +++--- .../LocalRewriter_DeconstructionAssignmentOperator.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 893aa28e8449d..86ee7db2c0368 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1852,8 +1852,8 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax private class DeconstructionVariable { - public BoundExpression Single; - public ImmutableArray Nested; + public readonly BoundExpression Single; + public readonly ImmutableArray Nested; public DeconstructionVariable(BoundExpression variable) { @@ -1867,7 +1867,7 @@ public DeconstructionVariable(ImmutableArray variables) Nested = variables; } - public bool IsNested => Nested != default(ImmutableArray); + public bool IsNested => !Nested.IsDefault; } /// diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index 8953f400037c5..94e3dc6ccc484 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -19,7 +19,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct var placeholders = ArrayBuilder.GetInstance(); // evaluate left-hand-side side-effects - var lhsTemps = LeftHandSideSideEffects(node.LeftVariables, temps, stores); + var lhsTargets = LeftHandSideSideEffects(node.LeftVariables, temps, stores); // get or make right-hand-side values BoundExpression loweredRight = VisitExpression(node.Right); @@ -44,7 +44,7 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct { // lower the assignment and replace the placeholders for its outputs in the process var assignmentInfo = node.AssignmentSteps[i]; - AddPlaceholderReplacement(assignmentInfo.OutputPlaceholder, lhsTemps[i]); + AddPlaceholderReplacement(assignmentInfo.OutputPlaceholder, lhsTargets[i]); var assignment = VisitExpression(assignmentInfo.Assignment);