Merge pull request #13096 from AlekseyTs/NewScopingRules_05

Adjust scope of locals declared within While/Do/Lock/If Statements according to the latest design.

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

@@ -250,7 +250,7 @@ private BoundStatement BindLockStatement(LockStatementSyntax node, DiagnosticBag
         {
             var lockBinder = this.GetBinder(node);
             Debug.Assert(lockBinder != null);
-            return lockBinder.WrapWithVariablesIfAny(node, lockBinder.BindLockStatementParts(diagnostics, lockBinder));
+            return lockBinder.BindLockStatementParts(diagnostics, lockBinder);
         }
 
         internal virtual BoundStatement BindLockStatementParts(DiagnosticBag diagnostics, Binder originalBinder)
@@ -276,6 +276,10 @@ internal BoundStatement BindPossibleEmbeddedStatement(StatementSyntax node, Diag
             switch (node.Kind())
             {
                 case SyntaxKind.ExpressionStatement:
+                case SyntaxKind.WhileStatement:
+                case SyntaxKind.DoStatement:
+                case SyntaxKind.LockStatement:
+                case SyntaxKind.IfStatement: 
                     Binder binder = this.GetBinder(node);
                     Debug.Assert(binder != null);
                     return binder.WrapWithVariablesIfAny(node, binder.BindStatement(node, diagnostics)); 
@@ -2822,17 +2826,11 @@ internal BoundExpression GenerateConversionForAssignment(TypeSymbol targetType,
 
         private BoundStatement BindIfStatement(IfStatementSyntax node, DiagnosticBag diagnostics)
         {
-            Binder binder = this.GetBinder(node.Condition);
-            Debug.Assert(binder != null);
-
-            var condition = binder.BindBooleanExpression(node.Condition, diagnostics);
-            var consequence = binder.BindPossibleEmbeddedStatement(node.Statement, diagnostics);
-
-            // Note that the else clause does not use the pattern variable binder;
-            // pattern variables from the condition are not in scope in the else statement.
+            var condition = BindBooleanExpression(node.Condition, diagnostics);
+            var consequence = BindPossibleEmbeddedStatement(node.Statement, diagnostics);
             BoundStatement alternative = (node.Else == null) ? null : BindPossibleEmbeddedStatement(node.Else.Statement, diagnostics);
 
-            BoundStatement result = new BoundIfStatement(node, binder.GetDeclaredLocalsForScope(node.Condition), condition, consequence, alternative);
+            BoundStatement result = new BoundIfStatement(node, condition, consequence, alternative);
             return result;
         }
 
@@ -2990,7 +2988,7 @@ private BoundStatement BindWhile(WhileStatementSyntax node, DiagnosticBag diagno
 
             var loopBinder = this.GetBinder(node);
             Debug.Assert(loopBinder != null);
-            return loopBinder.WrapWithVariablesIfAny(node, loopBinder.BindWhileParts(diagnostics, loopBinder));
+            return loopBinder.BindWhileParts(diagnostics, loopBinder);
         }
 
         internal virtual BoundWhileStatement BindWhileParts(DiagnosticBag diagnostics, Binder originalBinder)
@@ -3003,7 +3001,7 @@ private BoundStatement BindDo(DoStatementSyntax node, DiagnosticBag diagnostics)
             var loopBinder = this.GetBinder(node);
             Debug.Assert(loopBinder != null);
 
-            return loopBinder.WrapWithVariablesIfAny(node, loopBinder.BindDoParts(diagnostics, loopBinder));
+            return loopBinder.BindDoParts(diagnostics, loopBinder);
         }
 
         internal virtual BoundDoStatement BindDoParts(DiagnosticBag diagnostics, Binder originalBinder)

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

@@ -103,6 +103,11 @@ public override void VisitLockStatement(LockStatementSyntax node)
             Visit(node.Expression);
         }
 
+        public override void VisitIfStatement(IfStatementSyntax node)
+        {
+            Visit(node.Condition);
+        }
+
         public override void VisitDeclarationPattern(DeclarationPatternSyntax node)
         {
             _localsBuilder.Add(SourceLocalSymbol.MakeLocal(_scopeBinder.ContainingMemberOrLambda, _scopeBinder, false, node.Type, node.Identifier, LocalDeclarationKind.PatternVariable));

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

@@ -337,8 +337,7 @@ public override void VisitUsingStatement(UsingStatementSyntax node)
         public override void VisitWhileStatement(WhileStatementSyntax node)
         {
             Debug.Assert((object)_containingMemberOrLambda == _enclosing.ContainingMemberOrLambda);
-            var patternBinder = new ExpressionVariableBinder(node, _enclosing);
-            var whileBinder = new WhileBinder(patternBinder, node);
+            var whileBinder = new WhileBinder(_enclosing, node);
             AddToMap(node, whileBinder);
 
             Visit(node.Condition, whileBinder);
@@ -348,8 +347,7 @@ public override void VisitWhileStatement(WhileStatementSyntax node)
         public override void VisitDoStatement(DoStatementSyntax node)
         {
             Debug.Assert((object)_containingMemberOrLambda == _enclosing.ContainingMemberOrLambda);
-            var patternBinder = new ExpressionVariableBinder(node, _enclosing);
-            var whileBinder = new WhileBinder(patternBinder, node);
+            var whileBinder = new WhileBinder(_enclosing, node);
             AddToMap(node, whileBinder);
 
             Visit(node.Condition, whileBinder);
@@ -456,8 +454,7 @@ public override void VisitFixedStatement(FixedStatementSyntax node)
 
         public override void VisitLockStatement(LockStatementSyntax node)
         {
-            var patternBinder = new ExpressionVariableBinder(node, _enclosing);
-            var lockBinder = new LockBinder(patternBinder, node);
+            var lockBinder = new LockBinder(_enclosing, node);
             AddToMap(node, lockBinder);
 
             Visit(node.Expression, lockBinder);
@@ -515,13 +512,8 @@ public override void VisitSwitchSection(SwitchSectionSyntax node)
 
         public override void VisitIfStatement(IfStatementSyntax node)
         {
-            var ifBinder = new ExpressionVariableBinder(node.Condition, _enclosing);
-            AddToMap(node.Condition, ifBinder);
-            Visit(node.Condition, ifBinder);
-            VisitPossibleEmbeddedStatement(node.Statement, ifBinder);
-
-            // pattern variables from the condition are not in scope within the else clause
-            if (node.Else != null) AddToMap(node.Else.Statement, _enclosing);
+            Visit(node.Condition, _enclosing);
+            VisitPossibleEmbeddedStatement(node.Statement, _enclosing);
             Visit(node.Else, _enclosing);
         }
 
@@ -729,6 +721,10 @@ private void VisitPossibleEmbeddedStatement(StatementSyntax statement, Binder en
                         // inside a block.
 
                     case SyntaxKind.ExpressionStatement:
+                    case SyntaxKind.WhileStatement:
+                    case SyntaxKind.DoStatement:
+                    case SyntaxKind.LockStatement:
+                    case SyntaxKind.IfStatement:
                         Debug.Assert((object)_containingMemberOrLambda == enclosing.ContainingMemberOrLambda);
                         var blockBinder = new BlockBinder(enclosing, new SyntaxList<StatementSyntax>(statement));
                         AddToMap(statement, blockBinder);

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

@@ -192,9 +192,16 @@ protected ImmutableArray<LocalSymbol> BuildLocals(SyntaxList<StatementSyntax> st
                         break;
 
                     case SyntaxKind.ExpressionStatement:
+                    case SyntaxKind.IfStatement:
                         ExpressionVariableFinder.FindExpressionVariables(this, locals, innerStatement, enclosingBinder);
                         break;
 
+                    case SyntaxKind.WhileStatement:
+                    case SyntaxKind.DoStatement:
+                    case SyntaxKind.LockStatement:
+                        ExpressionVariableFinder.FindExpressionVariables(this, locals, innerStatement, enclosingBinder.GetBinder(innerStatement));
+                        break;
+
                     default:
                         // no other statement introduces local variables into the enclosing scope
                         break;

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

@@ -834,7 +834,6 @@
   </Node>
 
   <Node Name="BoundIfStatement" Base="BoundStatement">
-    <Field Name="Locals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>
     <Field Name="Condition" Type="BoundExpression"/>
     <Field Name="Consequence" Type="BoundStatement"/>
     <Field Name="AlternativeOpt" Type="BoundStatement" Null="allow"/>

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

@@ -522,14 +522,6 @@ public static BoundBlock SynthesizedNoLocals(CSharpSyntaxNode syntax, params Bou
         }
     }
 
-    internal partial class BoundIfStatement
-    {
-        public BoundIfStatement(CSharpSyntaxNode syntax, BoundExpression condition, BoundStatement consequence, BoundStatement alternativeOpt, bool hasErrors = false)
-            : this(syntax, ImmutableArray<LocalSymbol>.Empty, condition, consequence, alternativeOpt, hasErrors)
-        {
-        }
-    }
-
     internal partial class BoundDefaultOperator
     {
         public BoundDefaultOperator(CSharpSyntaxNode syntax, TypeSymbol type)

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IfStatement.cs

@@ -25,7 +25,7 @@ public override BoundNode VisitIfStatement(BoundIfStatement node)
                 rewrittenCondition = _instrumenter.InstrumentIfStatementCondition(node, rewrittenCondition, _factory);
             }
 
-            var result = RewriteIfStatement(syntax, node.Locals, rewrittenCondition, rewrittenConsequence, rewrittenAlternative, node.HasErrors);
+            var result = RewriteIfStatement(syntax, rewrittenCondition, rewrittenConsequence, rewrittenAlternative, node.HasErrors);
 
             // add sequence point before the whole statement
             if (this.Instrument && !node.WasCompilerGenerated)
@@ -42,17 +42,6 @@ public override BoundNode VisitIfStatement(BoundIfStatement node)
             BoundStatement rewrittenConsequence,
             BoundStatement rewrittenAlternativeOpt,
             bool hasErrors)
-        {
-            return RewriteIfStatement(syntax, ImmutableArray<LocalSymbol>.Empty, rewrittenCondition, rewrittenConsequence, rewrittenAlternativeOpt, hasErrors);
-        }
-
-        private static BoundStatement RewriteIfStatement(
-            CSharpSyntaxNode syntax,
-            ImmutableArray<LocalSymbol> patternVariables,
-            BoundExpression rewrittenCondition,
-            BoundStatement rewrittenConsequence,
-            BoundStatement rewrittenAlternativeOpt,
-            bool hasErrors)
         {
             var afterif = new GeneratedLabelSymbol("afterif");
             var builder = ArrayBuilder<BoundStatement>.GetInstance();
@@ -72,9 +61,7 @@ public override BoundNode VisitIfStatement(BoundIfStatement node)
                 builder.Add(rewrittenConsequence);
                 builder.Add(new BoundLabelStatement(syntax, afterif));
                 var statements = builder.ToImmutableAndFree();
-                return (patternVariables.IsDefaultOrEmpty)
-                    ? new BoundStatementList(syntax, statements, hasErrors)
-                    : new BoundBlock(syntax, patternVariables, ImmutableArray<LocalFunctionSymbol>.Empty, statements, hasErrors) { WasCompilerGenerated = true };
+                return new BoundStatementList(syntax, statements, hasErrors);
             }
             else
             {
@@ -98,13 +85,6 @@ public override BoundNode VisitIfStatement(BoundIfStatement node)
                 builder.Add(rewrittenConsequence);
                 builder.Add(new BoundGotoStatement(syntax, afterif));
                 builder.Add(new BoundLabelStatement(syntax, alt));
-                if (!patternVariables.IsDefaultOrEmpty)
-                {
-                    // pattern variables are not in scope in the else part
-                    var firstPart = new BoundBlock(syntax, patternVariables, ImmutableArray<LocalFunctionSymbol>.Empty, builder.ToImmutableAndFree(), hasErrors) { WasCompilerGenerated = true };
-                    builder = ArrayBuilder<BoundStatement>.GetInstance();
-                    builder.Add(firstPart);
-                }
                 builder.Add(rewrittenAlternativeOpt);
                 builder.Add(new BoundLabelStatement(syntax, afterif));
                 return new BoundStatementList(syntax, builder.ToImmutableAndFree(), hasErrors);