Deconstruction-assignment: nested scenario (#11715)

src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs

@@ -278,16 +278,6 @@ public override Symbol ExpressionSymbol
         public ImmutableArray<MethodSymbol> OriginalUserDefinedOperatorsOpt { get; }
     }
 
-    internal sealed partial class BoundDeconstructionAssignmentOperator : BoundExpression
-    {
-        internal class AssignmentInfo
-        {
-            public BoundAssignmentOperator Assignment;
-            public BoundLValuePlaceholder LValuePlaceholder;
-            public BoundRValuePlaceholder RValuePlaceholder;
-        }
-    }
-
     internal partial class BoundCompoundAssignmentOperator
     {
         public override Symbol ExpressionSymbol

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.
   -->
-  <Node Name="BoundLValuePlaceholder" Base="BoundValuePlaceholderBase" HasValidate="true">
-  </Node>
-  <Node Name="BoundRValuePlaceholder" Base="BoundValuePlaceholderBase" HasValidate="true">
+  <Node Name="BoundDeconstructValuePlaceholder" Base="BoundValuePlaceholderBase" HasValidate="true">
   </Node>
 
   <!-- only used by codegen -->
@@ -394,13 +392,26 @@
     <!-- Non-null type is required for this node kind -->
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
 
+    <!-- Various assignable expressions, including some BoundDeconstructionVariables for nested variables -->
     <Field Name="LeftVariables" Type="ImmutableArray&lt;BoundExpression&gt;"/>
 
     <Field Name="Right" Type="BoundExpression"/>
+
+    <Field Name="DeconstructSteps" Type="ImmutableArray&lt;BoundDeconstructionDeconstructStep&gt;" Null="NotApplicable" SkipInVisitor="true" />
+
+    <Field Name="AssignmentSteps" Type="ImmutableArray&lt;BoundDeconstructionAssignmentStep&gt;" Null="NotApplicable" SkipInVisitor="true"/>
+  </Node>
+
+  <Node Name="BoundDeconstructionDeconstructStep" Base="BoundNode">
     <Field Name="DeconstructMemberOpt" Type="MethodSymbol" Null="allow"/>
+    <Field Name="TargetPlaceholder" Type="BoundDeconstructValuePlaceholder" Null="disallow"/>
+    <Field Name="OutputPlaceholders" Type="ImmutableArray&lt;BoundDeconstructValuePlaceholder&gt;"/>
+  </Node>
 
-    <!-- The assignments have placeholders for the left and right part of the assignment -->
-    <Field Name="Assignments" Type="ImmutableArray&lt;BoundDeconstructionAssignmentOperator.AssignmentInfo&gt;" Null="NotApplicable" SkipInVisitor="true"/>
+  <Node Name="BoundDeconstructionAssignmentStep" Base="BoundNode">
+    <Field Name="Assignment" Type="BoundAssignmentOperator" Null="disallow"/>
+    <Field Name="TargetPlaceholder" Type="BoundDeconstructValuePlaceholder" Null="disallow"/>
+    <Field Name="OutputPlaceholder" Type="BoundDeconstructValuePlaceholder" Null="disallow"/>
   </Node>
 
   <Node Name="BoundVoid" Base="BoundExpression">

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -33,22 +33,7 @@ Optional<object> IOperation.ConstantValue
         public abstract TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, TResult> 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<TArgument, TResult>(OperationVisitor<TArgument, TResult> 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;
 

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -4899,4 +4899,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_OptionMustBeAbsolutePath" xml:space="preserve">
     <value>Option '{0}' must be an absolute path.</value>
   </data>
-</root>
+  <data name="ERR_DeconstructWrongCardinality" xml:space="preserve">
+    <value>Cannot deconstruct a tuple of '{0}' elements into '{1}' variables.</value>
+  </data>
+</root>

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,
     }
 }

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,15 @@ 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)
             {
-                Assign(variable, null, refKind: RefKind.None);
+                // PROTOTYPE(tuples) value should not be set to null
+                Assign(variable, value: null, refKind: RefKind.None);
             }
 
             return null;

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);
         }
 
+        /// <summary>
+        /// Remove all the listed placeholders.
+        /// </summary>
+        private void RemovePlaceholderReplacements(ArrayBuilder<BoundValuePlaceholderBase> placeholders)
+        {
+            foreach (var placeholder in placeholders)
+            {
+                RemovePlaceholderReplacement(placeholder);
+            }
+        }
+
         public override BoundNode VisitBadExpression(BoundBadExpression node)
         {
             // Cannot recurse into BadExpression children since the BadExpression

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,76 +11,92 @@ internal sealed partial class LocalRewriter
     {
         public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstructionAssignmentOperator node)
         {
+            Debug.Assert(node.DeconstructSteps != null);
+            Debug.Assert(node.AssignmentSteps != null);
+
             var temps = ArrayBuilder<LocalSymbol>.GetInstance();
             var stores = ArrayBuilder<BoundExpression>.GetInstance();
+            var placeholders = ArrayBuilder<BoundValuePlaceholderBase>.GetInstance();
 
-            var lhsReceivers = ArrayBuilder<BoundExpression>.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));
-            }
+            // evaluate left-hand-side side-effects
+            var lhsTargets = LeftHandSideSideEffects(node.LeftVariables, temps, stores);
 
+            // get or make right-hand-side values
             BoundExpression loweredRight = VisitExpression(node.Right);
-            ImmutableArray<BoundExpression> rhsValues;
+            AddPlaceholderReplacement(node.DeconstructSteps[0].TargetPlaceholder, loweredRight);
+            placeholders.Add(node.DeconstructSteps[0].TargetPlaceholder);
 
-            // get or make right-hand-side values
-            if (node.Right.Type.IsTupleType)
-            {
-                rhsValues = AccessTupleFields(node, loweredRight, temps, stores);
-            }
-            else
+            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]);
+                // lower the assignment and replace the placeholders for its outputs in the process
+                var assignmentInfo = node.AssignmentSteps[i];
+                AddPlaceholderReplacement(assignmentInfo.OutputPlaceholder, lhsTargets[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<BoundExpression> AccessTupleFields(BoundDeconstructionAssignmentOperator node, BoundExpression loweredRight, ArrayBuilder<LocalSymbol> temps, ArrayBuilder<BoundExpression> stores)
+        /// <summary>
+        /// Adds the side effects to stores and returns temporaries (as a flat list) to access them.
+        /// </summary>
+        private ImmutableArray<BoundExpression> LeftHandSideSideEffects(ImmutableArray<BoundExpression> variables, ArrayBuilder<LocalSymbol> temps, ArrayBuilder<BoundExpression> stores)
+        {
+            var lhsReceivers = ArrayBuilder<BoundExpression>.GetInstance(variables.Length);
+
+            foreach (var variable in variables)
+            {
+                // PROTOTYPE(tuples) should the dynamic flag always be false?
+                lhsReceivers.Add(TransformCompoundAssignmentLHS(variable, stores, temps, isDynamicAssignment: false));
+            }
+
+            return lhsReceivers.ToImmutableAndFree();
+        }
+
+        private void AccessTupleFields(BoundDeconstructionAssignmentOperator node, BoundDeconstructionDeconstructStep deconstruction, ArrayBuilder<LocalSymbol> temps, ArrayBuilder<BoundExpression> stores, ArrayBuilder<BoundValuePlaceholderBase> placeholders)
         {
-            var tupleType = loweredRight.Type.IsTupleType ? loweredRight.Type : TupleTypeSymbol.Create((NamedTypeSymbol)loweredRight.Type);
+            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;
-            Debug.Assert(numElements == node.LeftVariables.Length);
 
             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(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<BoundExpression>.GetInstance(numElements);
             var fields = tupleType.TupleElementFields;
 
             for (int i = 0; i < numElements; i++)
@@ -94,30 +109,30 @@ private ImmutableArray<BoundExpression> 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]);
+            }
         }
 
         /// <summary>
         /// Prepares local variables to be used in Deconstruct call
         /// Adds a invocation of Deconstruct with those as out parameters onto the 'stores' sequence
         /// Returns the expressions for those out parameters
         /// </summary>
-        private ImmutableArray<BoundExpression> CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundExpression loweredRight, ArrayBuilder<LocalSymbol> temps, ArrayBuilder<BoundExpression> stores)
+        private void CallDeconstruct(BoundDeconstructionAssignmentOperator node, BoundDeconstructionDeconstructStep deconstruction, ArrayBuilder<LocalSymbol> temps, ArrayBuilder<BoundExpression> stores, ArrayBuilder<BoundValuePlaceholderBase> 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<BoundExpression>.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 +144,16 @@ private ImmutableArray<BoundExpression> 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;
         }
     }
 }