Binding for deconstruction-declaration in 'foreach' statement (#12326)

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -1723,7 +1723,7 @@ private BoundExpression BindAssignment(AssignmentExpressionSyntax node, Diagnost
             return BindAssignment(node, op1, op2, diagnostics);
         }
 
-        private BoundAssignmentOperator BindAssignment(ExpressionSyntax node, BoundExpression op1, BoundExpression op2, DiagnosticBag diagnostics)
+        private BoundAssignmentOperator BindAssignment(CSharpSyntaxNode node, BoundExpression op1, BoundExpression op2, DiagnosticBag diagnostics)
         {
             Debug.Assert(op1 != null);
             Debug.Assert(op2 != null);
@@ -3086,6 +3086,14 @@ internal virtual BoundStatement BindForEachParts(DiagnosticBag diagnostics, Bind
             return this.Next.BindForEachParts(diagnostics, originalBinder);
         }
 
+        /// <summary>
+        /// Like BindForEachParts, but only bind the deconstruction part of the foreach, for purpose of inferring the types of the declared locals.
+        /// </summary>
+        internal virtual void BindForEachDeconstruction(DiagnosticBag diagnostics, Binder originalBinder)
+        {
+            this.Next.BindForEachDeconstruction(diagnostics, originalBinder);
+        }
+
         private BoundStatement BindBreak(BreakStatementSyntax node, DiagnosticBag diagnostics)
         {
             var target = this.BreakLabel;

src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs

@@ -182,6 +182,12 @@ internal override BoundStatement BindForEachParts(DiagnosticBag diagnostics, Bin
             throw ExceptionUtilities.Unreachable;
         }
 
+        internal override void BindForEachDeconstruction(DiagnosticBag diagnostics, Binder originalBinder)
+        {
+            // There's supposed to be a ForEachLoopBinder (or other overrider of this method) in the chain.
+            throw ExceptionUtilities.Unreachable;
+        }
+
         internal override BoundWhileStatement BindWhileParts(DiagnosticBag diagnostics, Binder originalBinder)
         {
             // There's supposed to be a WhileBinder (or other overrider of this method) in the chain.

src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs

@@ -27,7 +27,7 @@ private SourceLocalSymbol IterationVariable
         {
             get
             {
-                return (SourceLocalSymbol)this.Locals[0];
+                return _syntax.IsDeconstructionDeclaration ? null : (SourceLocalSymbol)this.Locals[0];
             }
         }
 
@@ -40,14 +40,29 @@ public ForEachLoopBinder(Binder enclosing, ForEachStatementSyntax syntax)
 
         protected override ImmutableArray<LocalSymbol> BuildLocals()
         {
-            var iterationVariable = SourceLocalSymbol.MakeForeachLocal(
-                (MethodSymbol)this.ContainingMemberOrLambda,
-                this,
-                _syntax.Type,
-                _syntax.Identifier,
-                _syntax.Expression);
-
-            return ImmutableArray.Create<LocalSymbol>(iterationVariable);
+            if (_syntax.IsDeconstructionDeclaration)
+            {
+                var locals = ArrayBuilder<LocalSymbol>.GetInstance();
+
+                CollectLocalsFromDeconstruction(
+                    _syntax.DeconstructionVariables,
+                    _syntax.DeconstructionVariables.Type,
+                    LocalDeclarationKind.ForEachIterationVariable,
+                    locals);
+
+                return locals.ToImmutableAndFree();
+            }
+            else
+            {
+                var iterationVariable = SourceLocalSymbol.MakeForeachLocal(
+                    (MethodSymbol)this.ContainingMemberOrLambda,
+                    this,
+                    _syntax.Type,
+                    _syntax.Identifier,
+                    _syntax.Expression);
+
+                return ImmutableArray.Create<LocalSymbol>(iterationVariable);
+            }
         }
 
         /// <summary>
@@ -59,10 +74,32 @@ internal override BoundStatement BindForEachParts(DiagnosticBag diagnostics, Bin
             return result;
         }
 
+        /// <summary>
+        /// Like BindForEachParts, but only bind the deconstruction part of the foreach, for purpose of inferring the types of the declared locals.
+        /// </summary>
+        internal override void BindForEachDeconstruction(DiagnosticBag diagnostics, Binder originalBinder)
+        {
+            // Use the right binder to avoid seeing iteration variable
+            BoundExpression collectionExpr = originalBinder.GetBinder(_syntax.Expression).BindValue(_syntax.Expression, diagnostics, BindValueKind.RValue);
+
+            ForEachEnumeratorInfo.Builder builder = new ForEachEnumeratorInfo.Builder();
+            TypeSymbol inferredType;
+            bool hasErrors = !GetEnumeratorInfoAndInferCollectionElementType(ref builder, ref collectionExpr, diagnostics, out inferredType);
+
+            VariableDeclarationSyntax variables = _syntax.DeconstructionVariables;
+            var valuePlaceholder = new BoundDeconstructValuePlaceholder(_syntax.Expression, inferredType ?? CreateErrorType("var"));
+            BoundDeconstructionAssignmentOperator deconstruction = BindDeconstructionDeclaration(
+                                                                    variables,
+                                                                    variables,
+                                                                    right: null,
+                                                                    diagnostics: diagnostics,
+                                                                    rightPlaceholder: valuePlaceholder);
+        }
+
         private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics, Binder originalBinder)
         {
             // Use the right binder to avoid seeing iteration variable
-            BoundExpression collectionExpr = originalBinder.GetBinder(_syntax.Expression).BindValue(_syntax.Expression, diagnostics, BindValueKind.RValue); 
+            BoundExpression collectionExpr = originalBinder.GetBinder(_syntax.Expression).BindValue(_syntax.Expression, diagnostics, BindValueKind.RValue);
 
             ForEachEnumeratorInfo.Builder builder = new ForEachEnumeratorInfo.Builder();
             TypeSymbol inferredType;
@@ -74,33 +111,54 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics,
                 (object)builder.MoveNextMethod == null ||
                 (object)builder.CurrentPropertyGetter == null;
 
-            // Check for local variable conflicts in the *enclosing* binder; obviously the *current*
-            // binder has a local that matches!
-            var hasNameConflicts = originalBinder.ValidateDeclarationNameConflictsInScope(IterationVariable, diagnostics);
-
-            // If the type in syntax is "var", then the type should be set explicitly so that the
-            // Type property doesn't fail.
-
-            TypeSyntax typeSyntax = _syntax.Type;
-
-            bool isVar;
-            AliasSymbol alias;
-            TypeSymbol declType = BindType(typeSyntax, diagnostics, out isVar, out alias);
-
             TypeSymbol iterationVariableType;
-            if (isVar)
+            BoundTypeExpression boundIterationVariableType;
+            bool hasNameConflicts = false;
+            BoundForEachDeconstructStep deconstructStep = null;
+            if (_syntax.IsDeconstructionDeclaration)
             {
                 iterationVariableType = inferredType ?? CreateErrorType("var");
+
+                VariableDeclarationSyntax variables = _syntax.DeconstructionVariables;
+                var valuePlaceholder = new BoundDeconstructValuePlaceholder(_syntax.Expression, iterationVariableType);
+                BoundDeconstructionAssignmentOperator deconstruction = BindDeconstructionDeclaration(
+                                                                        variables,
+                                                                        variables,
+                                                                        right: null,
+                                                                        diagnostics: diagnostics,
+                                                                        rightPlaceholder: valuePlaceholder);
+
+                deconstructStep = new BoundForEachDeconstructStep(_syntax.DeconstructionVariables, deconstruction, valuePlaceholder);
+                boundIterationVariableType = new BoundTypeExpression(variables, aliasOpt: null, type: iterationVariableType);
             }
             else
             {
-                Debug.Assert((object)declType != null);
-                iterationVariableType = declType;
-            }
+                // Check for local variable conflicts in the *enclosing* binder; obviously the *current*
+                // binder has a local that matches!
+                hasNameConflicts = originalBinder.ValidateDeclarationNameConflictsInScope(IterationVariable, diagnostics);
+
+                // If the type in syntax is "var", then the type should be set explicitly so that the
+                // Type property doesn't fail.
 
+                TypeSyntax typeSyntax = _syntax.Type;
 
-            BoundTypeExpression boundIterationVariableType = new BoundTypeExpression(typeSyntax, alias, iterationVariableType);
-            this.IterationVariable.SetTypeSymbol(iterationVariableType);
+                bool isVar;
+                AliasSymbol alias;
+                TypeSymbol declType = BindType(typeSyntax, diagnostics, out isVar, out alias);
+
+                if (isVar)
+                {
+                    iterationVariableType = inferredType ?? CreateErrorType("var");
+                }
+                else
+                {
+                    Debug.Assert((object)declType != null);
+                    iterationVariableType = declType;
+                }
+
+                boundIterationVariableType = new BoundTypeExpression(typeSyntax, alias, iterationVariableType);
+                this.IterationVariable.SetTypeSymbol(iterationVariableType);
+            }
 
             BoundStatement body = originalBinder.BindPossibleEmbeddedStatement(_syntax.Statement, diagnostics);
 
@@ -116,6 +174,7 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics,
                     boundIterationVariableType,
                     this.IterationVariable,
                     collectionExpr,
+                    deconstructStep,
                     body,
                     CheckOverflowAtRuntime,
                     this.BreakLabel,
@@ -207,6 +266,7 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics,
                 boundIterationVariableType,
                 this.IterationVariable,
                 convertedCollectionExpression,
+                deconstructStep,
                 body,
                 CheckOverflowAtRuntime,
                 this.BreakLabel,
@@ -475,7 +535,7 @@ private ForEachEnumeratorInfo.Builder GetDefaultEnumeratorInfo(ForEachEnumerator
             }
             else
             {
-                builder.ElementType = collectionExprType.SpecialType == SpecialType.System_String?
+                builder.ElementType = collectionExprType.SpecialType == SpecialType.System_String ?
                     GetSpecialType(SpecialType.System_Char, diagnostics, _syntax) :
                     ((ArrayTypeSymbol)collectionExprType).ElementType;
             }

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -816,19 +816,27 @@
 
     <!-- Pieces corresponding to the syntax -->
     <!-- This is so the binding API can find produce semantic info if the type is "var". -->
-    <Field Name="IterationVariableType" Type="BoundTypeExpression"/> 
-    <Field Name="IterationVariable" Type="LocalSymbol"/>
-    <!-- If this node does not have errors, then this is the foreach expression wrapped 
+    <!-- If there is a deconstruction, there will be no iteration variable (but we'll still have a type for it). -->
+    <Field Name="IterationVariableType" Type="BoundTypeExpression"/>
+    <Field Name="IterationVariableOpt" Type="LocalSymbol" Null="allow"/>
+    <!-- If this node does not have errors, then this is the foreach expression wrapped
          in a conversion to the collection type used by the foreach loop.  The conversion
          is here so that the binding API can return the correct ConvertedType in semantic 
          info.  It will be stripped off in the rewriter if it is redundant or causes extra
          boxing.  If this node has errors, then the conversion may not be present.-->
     <Field Name="Expression" Type="BoundExpression"/>
+    <Field Name="DeconstructionOpt" Type="BoundForEachDeconstructStep" Null="allow" SkipInVisitor="true"/>
     <Field Name="Body" Type="BoundStatement"/>
 
     <Field Name="Checked" Type="Boolean"/>
   </Node>
 
+  <!-- All the information need to apply a deconstruction at each iteration of a foreach loop involving a deconstruction-declaration. -->
+  <Node Name="BoundForEachDeconstructStep" Base="BoundNode">
+    <Field Name="DeconstructionAssignment" Type="BoundDeconstructionAssignmentOperator" Null="disallow"/>
+    <Field Name="TargetPlaceholder" Type="BoundDeconstructValuePlaceholder" Null="disallow"/>
+  </Node>
+
   <Node Name="BoundUsingStatement" Base="BoundStatement">
     <!-- DeclarationsOpt and ExpressionOpt cannot both be non-null. -->
     <Field Name="Locals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>

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

@@ -262,7 +262,7 @@ public override void Accept(OperationVisitor visitor)
 
     internal partial class BoundForEachStatement : IForEachLoopStatement
     {
-        ILocalSymbol IForEachLoopStatement.IterationVariable => this.IterationVariable;
+        ILocalSymbol IForEachLoopStatement.IterationVariable => this.IterationVariableOpt;
 
         IOperation IForEachLoopStatement.Collection => this.Expression;
 

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

@@ -251,6 +251,11 @@ private static Binder GetEnclosingBinder(CSharpSyntaxNode node, int position, Bi
                 {
                     binder = rootBinder.GetBinder(current);
                 }
+                else if (current.Kind() == SyntaxKind.VariableDeclaration &&
+                             (current.Parent as ForEachStatementSyntax)?.DeconstructionVariables == current)
+                {
+                    binder = rootBinder.GetBinder(current.Parent);
+                }
                 else
                 {
                     // If this ever breaks, make sure that all callers of

src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs

@@ -1075,7 +1075,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, RefKind
 
                 case BoundKind.ForEachStatement:
                     {
-                        var iterationVariable = ((BoundForEachStatement)node).IterationVariable;
+                        var iterationVariable = ((BoundForEachStatement)node).IterationVariableOpt;
                         Debug.Assert((object)iterationVariable != null);
                         int slot = GetOrCreateSlot(iterationVariable);
                         if (slot > 0) SetSlotState(slot, written);
@@ -1728,7 +1728,6 @@ public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstruct
 
             foreach (BoundExpression variable in node.LeftVariables)
             {
-                // PROTOTYPE(tuples) value should not be set to null
                 Assign(variable, value: null, refKind: RefKind.None);
             }
 
@@ -2055,7 +2054,7 @@ public override BoundNode VisitEventAccess(BoundEventAccess node)
 
         public override void VisitForEachIterationVariable(BoundForEachStatement node)
         {
-            var local = node.IterationVariable;
+            var local = node.IterationVariableOpt;
             if ((object)local != null)
             {
                 GetOrCreateSlot(local);

src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowsOutWalker.cs

@@ -149,7 +149,7 @@ private Symbol GetNodeSymbol(BoundNode node)
 
                     case BoundKind.ForEachStatement:
                         {
-                            return ((BoundForEachStatement)node).IterationVariable;
+                            return ((BoundForEachStatement)node).IterationVariableOpt;
                         }
 
                     case BoundKind.RangeVariable:

src/Compilers/CSharp/Portable/FlowAnalysis/ReadWriteWalker.cs

@@ -215,7 +215,7 @@ public override BoundNode VisitUnboundLambda(UnboundLambda node)
 
         public override void VisitForEachIterationVariable(BoundForEachStatement node)
         {
-            var local = node.IterationVariable;
+            var local = node.IterationVariableOpt;
             if ((object)local != null)
             {
                 GetOrCreateSlot(local);

src/Compilers/CSharp/Portable/FlowAnalysis/VariablesDeclaredWalker.cs

@@ -103,7 +103,7 @@ public override BoundNode VisitForEachStatement(BoundForEachStatement node)
         {
             if (IsInside)
             {
-                _variablesDeclared.Add(node.IterationVariable);
+                _variablesDeclared.Add(node.IterationVariableOpt);
             }
 
             return base.VisitForEachStatement(node);