Merge pull request #10483 from AlekseyTs/Issue8817_16

Clean-up PatternVariableBinder and PatternVariableFinder.

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

@@ -772,24 +772,10 @@ internal Binder[] GetAllBinders()
         }
 #endif
         
-        internal void BuildAndAddPatternVariables(
-            ArrayBuilder<LocalSymbol> builder,
-            CSharpSyntaxNode node = null,
-            ImmutableArray<CSharpSyntaxNode> nodes = default(ImmutableArray<CSharpSyntaxNode>))
-        {
-            var patterns = ArrayBuilder<DeclarationPatternSyntax>.GetInstance();
-            PatternVariableFinder.FindPatternVariables(patterns, node, nodes);
-            foreach (var pattern in patterns)
-            {
-                builder.Add(SourceLocalSymbol.MakeLocal(ContainingMemberOrLambda, this, RefKind.None, pattern.Type, pattern.Identifier, LocalDeclarationKind.PatternVariable));
-            }
-            patterns.Free();
-        }
-
         internal BoundExpression WrapWithVariablesIfAny(CSharpSyntaxNode scopeDesignator, BoundExpression expression)
         {
             var locals = this.GetDeclaredLocalsForScope(scopeDesignator);
-            return (locals.Length == 0)
+            return (locals.IsEmpty)
                 ? expression
                 : new BoundSequence(scopeDesignator, locals, ImmutableArray<BoundExpression>.Empty, expression, expression.Type) { WasCompilerGenerated = true };
         }

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

@@ -453,7 +453,6 @@ private BoundStatement BindLocalFunctionStatementParts(LocalFunctionStatementSyn
                 hasErrors |= this.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);
             }
 
-            // Binder could be null in error scenarios (as above)
             BoundBlock block;
             if (node.Body != null)
             {

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

@@ -31,7 +31,7 @@ override protected ImmutableArray<LocalSymbol> BuildLocals()
 
             if (_syntax.Filter != null)
             {
-                BuildAndAddPatternVariables(locals, _syntax.Filter.FilterExpression);
+                PatternVariableFinder.FindPatternVariables(this, locals, _syntax.Filter.FilterExpression);
             }
 
             return locals.ToImmutableAndFree();

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

@@ -32,7 +32,7 @@ override protected ImmutableArray<LocalSymbol> BuildLocals()
 
                     if (declarator.Initializer != null)
                     {
-                        BuildAndAddPatternVariables(locals, declarator.Initializer.Value);
+                        PatternVariableFinder.FindPatternVariables(this, locals, declarator.Initializer.Value);
                     }
                 }
 

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

@@ -39,21 +39,21 @@ override protected ImmutableArray<LocalSymbol> BuildLocals()
 
                     if (variable.Initializer != null)
                     {
-                        BuildAndAddPatternVariables(locals, variable.Initializer.Value);
+                        PatternVariableFinder.FindPatternVariables(this, locals, variable.Initializer.Value);
                     }
                 }
             }
             else
             {
-                BuildAndAddPatternVariables(locals, nodes:_syntax.Initializers.ToImmutableArray<CSharpSyntaxNode>());
+                PatternVariableFinder.FindPatternVariables(this, locals, _syntax.Initializers);
             }
 
             if (_syntax.Condition != null)
             {
-                BuildAndAddPatternVariables(locals, node: _syntax.Condition);
+                PatternVariableFinder.FindPatternVariables(this, locals, node: _syntax.Condition);
             }
 
-            BuildAndAddPatternVariables(locals, nodes: _syntax.Incrementors.ToImmutableArray<CSharpSyntaxNode>());
+            PatternVariableFinder.FindPatternVariables(this, locals, _syntax.Incrementors);
 
             return locals.ToImmutableAndFree();
         }

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

@@ -55,7 +55,7 @@ private void Visit(CSharpSyntaxNode syntax, Binder enclosing)
 
             if (syntax is ExpressionSyntax)
             {
-                var binder = new PatternVariableBinder(syntax, (ExpressionSyntax)syntax, enclosing);
+                var binder = new PatternVariableBinder(syntax, enclosing);
                 builder.AddToMap(syntax, binder);
                 builder.Visit(syntax, binder);
             }
@@ -145,7 +145,7 @@ private void VisitLambdaExpression(LambdaExpressionSyntax node)
             }
             else
             {
-                var binder = new PatternVariableBinder(body, (ExpressionSyntax)body, _enclosing);
+                var binder = new PatternVariableBinder(body, _enclosing);
                 AddToMap(body, binder);
                 Visit(body, binder);
             }
@@ -227,14 +227,14 @@ public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax no
 
         public override void VisitArrowExpressionClause(ArrowExpressionClauseSyntax node)
         {
-            var arrowBinder = new PatternVariableBinder(node, node.Expression, _enclosing);
+            var arrowBinder = new PatternVariableBinder(node, _enclosing);
             AddToMap(node, arrowBinder);
             Visit(node.Expression, arrowBinder);
         }
 
         public override void VisitEqualsValueClause(EqualsValueClauseSyntax node)
         {
-            var valueBinder = new PatternVariableBinder(node, node.Value, _enclosing);
+            var valueBinder = new PatternVariableBinder(node, _enclosing);
             AddToMap(node, valueBinder);
             Visit(node.Value, valueBinder);
         }
@@ -258,7 +258,8 @@ public override void VisitArgumentList(ArgumentListSyntax node)
             if (_root == node)
             {
                 // We are supposed to get here only for constructor initializers
-                var argBinder = new PatternVariableBinder(node, node.Arguments, _enclosing);
+                Debug.Assert(node.Parent is ConstructorInitializerSyntax);
+                var argBinder = new PatternVariableBinder(node, _enclosing);
                 AddToMap(node, argBinder);
 
                 foreach (var arg in node.Arguments)
@@ -337,7 +338,7 @@ public override void VisitUsingStatement(UsingStatementSyntax node)
         public override void VisitWhileStatement(WhileStatementSyntax node)
         {
             Debug.Assert((object)_containingMemberOrLambda == _enclosing.ContainingMemberOrLambda);
-            var patternBinder = new PatternVariableBinder(node, node.Condition, _enclosing);
+            var patternBinder = new PatternVariableBinder(node, _enclosing);
             var whileBinder = new WhileBinder(patternBinder, node);
             AddToMap(node, whileBinder);
 
@@ -348,7 +349,7 @@ public override void VisitWhileStatement(WhileStatementSyntax node)
         public override void VisitDoStatement(DoStatementSyntax node)
         {
             Debug.Assert((object)_containingMemberOrLambda == _enclosing.ContainingMemberOrLambda);
-            var patternBinder = new PatternVariableBinder(node, node.Condition, _enclosing);
+            var patternBinder = new PatternVariableBinder(node, _enclosing);
             var whileBinder = new WhileBinder(patternBinder, node);
             AddToMap(node, whileBinder);
 
@@ -397,7 +398,7 @@ public override void VisitForStatement(ForStatementSyntax node)
         public override void VisitForEachStatement(ForEachStatementSyntax node)
         {
             Debug.Assert((object)_containingMemberOrLambda == _enclosing.ContainingMemberOrLambda);
-            var patternBinder = new PatternVariableBinder(node.Expression, node.Expression, _enclosing);
+            var patternBinder = new PatternVariableBinder(node.Expression, _enclosing);
 
             AddToMap(node.Expression, patternBinder);
             Visit(node.Expression, patternBinder);
@@ -446,7 +447,7 @@ public override void VisitFixedStatement(FixedStatementSyntax node)
 
         public override void VisitLockStatement(LockStatementSyntax node)
         {
-            var patternBinder = new PatternVariableBinder(node, node.Expression, _enclosing);
+            var patternBinder = new PatternVariableBinder(node, _enclosing);
             var lockBinder = new LockBinder(patternBinder, node);
             AddToMap(node, lockBinder);
 
@@ -465,7 +466,7 @@ public override void VisitLockStatement(LockStatementSyntax node)
         public override void VisitSwitchStatement(SwitchStatementSyntax node)
         {
             Debug.Assert((object)_containingMemberOrLambda == _enclosing.ContainingMemberOrLambda);
-            var patternBinder = new PatternVariableBinder(node.Expression, node.Expression, _enclosing);
+            var patternBinder = new PatternVariableBinder(node.Expression, _enclosing);
             AddToMap(node.Expression, patternBinder);
 
             Visit(node.Expression, patternBinder);
@@ -505,7 +506,7 @@ public override void VisitSwitchSection(SwitchSectionSyntax node)
 
         public override void VisitIfStatement(IfStatementSyntax node)
         {
-            var ifBinder = new PatternVariableBinder(node.Condition, node.Condition, _enclosing);
+            var ifBinder = new PatternVariableBinder(node.Condition, _enclosing);
             AddToMap(node.Condition, ifBinder);
             Visit(node.Condition, ifBinder);
             VisitPossibleEmbeddedStatement(node.Statement, ifBinder);
@@ -519,7 +520,7 @@ public override void VisitLetStatement(LetStatementSyntax node)
         {
             // Note that we do *not* include variables defined in a let statement's pattern in the let statement's scope.
             // Those are instead included in the enclosing scope.
-            var letBinder = new PatternVariableBinder(node, ImmutableArray.Create(node.Expression, node.WhenClause?.Condition), _enclosing);
+            var letBinder = new PatternVariableBinder(node, _enclosing);
             Visit(node.Expression, letBinder);
 
             if (node.WhenClause != null)
@@ -616,7 +617,7 @@ public override void VisitYieldStatement(YieldStatementSyntax node)
         {
             if (node.Expression != null)
             {
-                var patternBinder = new PatternVariableBinder(node, node.Expression, _enclosing);
+                var patternBinder = new PatternVariableBinder(node, _enclosing);
                 AddToMap(node, patternBinder);
                 Visit(node.Expression, patternBinder);
             }
@@ -626,14 +627,14 @@ public override void VisitYieldStatement(YieldStatementSyntax node)
 
         public override void VisitExpressionStatement(ExpressionStatementSyntax node)
         {
-            var patternBinder = new PatternVariableBinder(node, node.Expression, _enclosing);
+            var patternBinder = new PatternVariableBinder(node, _enclosing);
             AddToMap(node, patternBinder);
             Visit(node.Expression, patternBinder);
         }
 
         public override void VisitLocalDeclarationStatement(LocalDeclarationStatementSyntax node)
         {
-            var patternBinder = new PatternVariableBinder(node, node.Declaration.Variables, _enclosing);
+            var patternBinder = new PatternVariableBinder(node, _enclosing);
             AddToMap(node, patternBinder);
 
             foreach (var decl in node.Declaration.Variables)
@@ -650,7 +651,7 @@ public override void VisitReturnStatement(ReturnStatementSyntax node)
         {
             if (node.Expression != null)
             {
-                var patternBinder = new PatternVariableBinder(node, node.Expression, _enclosing);
+                var patternBinder = new PatternVariableBinder(node, _enclosing);
                 AddToMap(node, patternBinder);
                 Visit(node.Expression, patternBinder);
             }
@@ -660,7 +661,7 @@ public override void VisitThrowStatement(ThrowStatementSyntax node)
         {
             if (node.Expression != null)
             {
-                var patternBinder = new PatternVariableBinder(node, node.Expression, _enclosing);
+                var patternBinder = new PatternVariableBinder(node, _enclosing);
                 AddToMap(node, patternBinder);
                 Visit(node.Expression, patternBinder);
             }

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

@@ -183,7 +183,7 @@ protected ImmutableArray<LocalSymbol> BuildLocals(SyntaxList<StatementSyntax> st
                             if (decl.Pattern != null)
                             {
                                 // Patterns from the let statement introduce bindings into the enclosing scope.
-                                BuildAndAddPatternVariables(locals, decl.Pattern);
+                                PatternVariableFinder.FindPatternVariables(this, locals, decl.Pattern);
                             }
                             else
                             {

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

@@ -11,96 +11,17 @@ namespace Microsoft.CodeAnalysis.CSharp
 {
     internal sealed class PatternVariableBinder : LocalScopeBinder
     {
-        private readonly CSharpSyntaxNode _node;
-        private readonly ImmutableArray<CSharpSyntaxNode> _nodes;
         public readonly CSharpSyntaxNode ScopeDesignator;
 
-        internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, ImmutableArray<ExpressionSyntax> expressions, Binder next) : base(next)
+        internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, Binder next) : base(next)
         {
             this.ScopeDesignator = scopeDesignator;
-            this._nodes = StaticCast<CSharpSyntaxNode>.From(expressions);
-        }
-
-        internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, IEnumerable<VariableDeclaratorSyntax> declarations, Binder next) : base(next)
-        {
-            this.ScopeDesignator = scopeDesignator;
-            var nodes = ArrayBuilder<CSharpSyntaxNode>.GetInstance();
-            foreach (var decl in declarations)
-            {
-                var value = decl.Initializer?.Value;
-                if (value != null) nodes.Add(value);
-            }
-            this._nodes = nodes.ToImmutableAndFree();
-        }
-
-        internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, IEnumerable<ArgumentSyntax> arguments, Binder next) : base(next)
-        {
-            this.ScopeDesignator = scopeDesignator;
-            var nodes = ArrayBuilder<CSharpSyntaxNode>.GetInstance();
-            foreach (var arg in arguments)
-            {
-                var value = arg.Expression;
-                if (value != null) nodes.Add(value);
-            }
-            this._nodes = nodes.ToImmutableAndFree();
-        }
-
-        internal PatternVariableBinder(SwitchSectionSyntax scopeDesignator, Binder next) : base(next)
-        {
-            this.ScopeDesignator = scopeDesignator;
-            var nodes = ArrayBuilder<CSharpSyntaxNode>.GetInstance();
-            foreach (var label in scopeDesignator.Labels)
-            {
-                var match = label as CasePatternSwitchLabelSyntax;
-                if (match != null)
-                {
-                    nodes.Add(match.Pattern);
-                    if (match.WhenClause != null)
-                    {
-                        nodes.Add(match.WhenClause.Condition);
-                    }
-                }
-            }
-
-            this._nodes = nodes.ToImmutableAndFree();
-        }
-
-        internal PatternVariableBinder(MatchSectionSyntax scopeDesignator, Binder next) : base(next)
-        {
-            this.ScopeDesignator = scopeDesignator;
-            this._nodes = scopeDesignator.WhenClause != null
-                ? ImmutableArray.Create<CSharpSyntaxNode>(scopeDesignator.Pattern, scopeDesignator.WhenClause.Condition, scopeDesignator.Expression)
-                : ImmutableArray.Create<CSharpSyntaxNode>(scopeDesignator.Pattern, scopeDesignator.Expression)
-                ;
-        }
-
-        internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, ExpressionSyntax expression, Binder next) : base(next)
-        {
-            this._node = expression;
-            this.ScopeDesignator = scopeDesignator;
-        }
-
-        internal PatternVariableBinder(AttributeSyntax scopeDesignator, Binder next) : base(next)
-        {
-            this.ScopeDesignator = scopeDesignator;
-
-            if (scopeDesignator.ArgumentList?.Arguments.Count > 0)
-            {
-                var expressions = ArrayBuilder<CSharpSyntaxNode>.GetInstance(scopeDesignator.ArgumentList.Arguments.Count);
-
-                foreach (var argument in scopeDesignator.ArgumentList.Arguments)
-                {
-                    expressions.Add(argument.Expression);
-                }
-
-                this._nodes = expressions.ToImmutableAndFree();
-            }
         }
 
         protected override ImmutableArray<LocalSymbol> BuildLocals()
         {
             var builder = ArrayBuilder<LocalSymbol>.GetInstance();
-            BuildAndAddPatternVariables(builder, _node, _nodes);
+            PatternVariableFinder.FindPatternVariables(this, builder, ScopeDesignator);
             return builder.ToImmutableAndFree();
         }
 

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

@@ -12,31 +12,102 @@ namespace Microsoft.CodeAnalysis.CSharp
 {
     internal class PatternVariableFinder : CSharpSyntaxWalker
     {
-        private ArrayBuilder<DeclarationPatternSyntax> _declarationPatterns;
+        private Binder _binder;
+        private ArrayBuilder<LocalSymbol> _declarationPatterns;
 
         internal static void FindPatternVariables(
-            ArrayBuilder<DeclarationPatternSyntax> builder,
-            CSharpSyntaxNode node = null,
-            ImmutableArray<CSharpSyntaxNode> nodes = default(ImmutableArray<CSharpSyntaxNode>))
+            Binder binder,
+            ArrayBuilder<LocalSymbol> builder,
+            CSharpSyntaxNode node)
         {
+            if (node == null)
+            {
+                return;
+            }
+
             var finder = s_poolInstance.Allocate();
+            finder._binder = binder;
             finder._declarationPatterns = builder;
+
             finder.Visit(node);
-            if (!nodes.IsDefaultOrEmpty)
+
+            finder._binder = null;
+            finder._declarationPatterns = null;
+            s_poolInstance.Free(finder);
+        }
+
+        internal static void FindPatternVariables(
+            Binder binder,
+            ArrayBuilder<LocalSymbol> builder,
+            SeparatedSyntaxList<ExpressionSyntax> nodes)
+        {
+            if (nodes.Count == 0)
             {
-                foreach (var n in nodes)
-                {
-                    finder.Visit(n);
-                }
+                return;
             }
 
+            var finder = s_poolInstance.Allocate();
+            finder._binder = binder;
+            finder._declarationPatterns = builder;
+
+            foreach (var n in nodes)
+            {
+                finder.Visit(n);
+            }
+
+            finder._binder = null;
             finder._declarationPatterns = null;
             s_poolInstance.Free(finder);
         }
 
+        public override void VisitLetStatement(LetStatementSyntax node)
+        {
+            Visit(node.Expression);
+            Visit(node.WhenClause?.Condition);
+        }
+
+        public override void VisitLocalDeclarationStatement(LocalDeclarationStatementSyntax node)
+        {
+            foreach (var decl in node.Declaration.Variables)
+            {
+                Visit(decl.Initializer?.Value);
+            }
+        }
+
+        public override void VisitSwitchSection(SwitchSectionSyntax node)
+        {
+            foreach (var label in node.Labels)
+            {
+                var match = label as CasePatternSwitchLabelSyntax;
+                if (match != null)
+                {
+                    Visit(match.Pattern);
+                    if (match.WhenClause != null)
+                    {
+                        Visit(match.WhenClause.Condition);
+                    }
+                }
+            }
+        }
+
+        public override void VisitWhileStatement(WhileStatementSyntax node)
+        {
+            Visit(node.Condition);
+        }
+
+        public override void VisitDoStatement(DoStatementSyntax node)
+        {
+            Visit(node.Condition);
+        }
+
+        public override void VisitLockStatement(LockStatementSyntax node)
+        {
+            Visit(node.Expression);
+        }
+
         public override void VisitDeclarationPattern(DeclarationPatternSyntax node)
         {
-            _declarationPatterns.Add(node);
+            _declarationPatterns.Add(SourceLocalSymbol.MakeLocal(_binder.ContainingMemberOrLambda, _binder, RefKind.None, node.Type, node.Identifier, LocalDeclarationKind.PatternVariable));
             base.VisitDeclarationPattern(node);
         }
         public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpressionSyntax node) { }

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

@@ -30,7 +30,7 @@ override protected ImmutableArray<LocalSymbol> BuildLocals()
             if (expressionSyntax != null)
             {
                 var locals = ArrayBuilder<LocalSymbol>.GetInstance();
-                BuildAndAddPatternVariables(locals, expressionSyntax);
+                PatternVariableFinder.FindPatternVariables(this, locals, expressionSyntax);
                 return locals.ToImmutableAndFree();
             }
             else
@@ -42,7 +42,7 @@ override protected ImmutableArray<LocalSymbol> BuildLocals()
 
                     if (declarator.Initializer != null)
                     {
-                        BuildAndAddPatternVariables(locals, declarator.Initializer.Value);
+                        PatternVariableFinder.FindPatternVariables(this, locals, declarator.Initializer.Value);
                     }
                 }
 

src/Compilers/CSharp/Portable/Syntax/SyntaxNodeExtensions.cs

@@ -72,6 +72,7 @@ internal static bool CanHaveAssociatedLocalBinder(this CSharpSyntaxNode syntax)
                 kind == SyntaxKind.ArgumentList ||
                 kind == SyntaxKind.ArrowExpressionClause ||
                 (syntax is ExpressionSyntax && 
+                    // All these nodes are valid scope designators due to the pattern matching feature.
                     ((syntax.Parent as LambdaExpressionSyntax)?.Body == syntax ||
                      (syntax.Parent as SwitchStatementSyntax)?.Expression == syntax ||
                      (syntax.Parent as ForEachStatementSyntax)?.Expression == syntax ||