Change local function definite assignment

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

@@ -124,17 +124,29 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen
 
             var oldPending = SavePending(); // we do not support branches into a lambda
 
-            // Local functions don't affect outer state and are analyzed
-            // with everything unassigned and reachable
+            // SPEC: The entry point to a local function is always reachable.
+            // Captured variables are assigned if they are assigned on all
+            // branches into the local function.
+
             var savedState = this.State;
             this.State = this.ReachableState();
 
-            var usages = GetOrCreateLocalFuncUsages(localFuncSymbol);
-            var oldReads = usages.ReadVars;
-            usages.ReadVars = BitVector.Empty;
-
             if (!localFunc.WasCompilerGenerated) EnterParameters(localFuncSymbol.Parameters);
 
+            // Captured variables are definitely assigned if they are assigned on
+            // all branches into the local function, so we store all reads from
+            // possibly unassigned captured variables and later report definite
+            // assignment errors if any of the captured variables is not assigned
+            // on a particular branch.
+            // Assignments to captured variables are also recorded, as a local function
+            // definitely assigns captured variables on a call to a local function
+            // if that variable is definitely assigned at all branches out of the
+            // local function
+
+            var usages = GetOrCreateLocalFuncUsages(localFuncSymbol);
+            var oldReads = usages.ReadVars.Clone();
+            usages.ReadVars.Clear();
+
             var oldPending2 = SavePending();
 
             // If this is an iterator, there's an implicit branch before the first statement
@@ -158,6 +170,7 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen
 
             LeaveParameters(localFuncSymbol.Parameters, localFunc.Syntax, location);
 
+            // Intersect the state of all branches out of the local function
             LocalState stateAtReturn = this.State;
             foreach (PendingBranch pending in pendingReturns)
             {
@@ -173,13 +186,20 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen
                 IntersectWith(ref stateAtReturn, ref this.State);
             }
 
-            // Check for changes to the read and write sets
+            // Check for changes to the possibly unassigned and assigned sets
+            // of captured variables
             if (RecordChangedVars(ref usages.WrittenVars,
                                   ref stateAtReturn,
                                   ref oldReads,
                                   ref usages.ReadVars) &&
                 usages.LocalFuncVisited)
             {
+                // If the sets have changed and we already used the results
+                // of this local function in another computation, the previous
+                // calculations may be invalid. We need to analyze until we
+                // reach a fixed-point. The previous writes are always valid,
+                // so they are stored in usages.WrittenVars, while the reads
+                // may be invalidated by new writes, so we throw the results out.
                 stateChangedAfterUse = true;
                 usages.LocalFuncVisited = false;
             }
@@ -261,8 +281,7 @@ private BitVector GetCapturedBitmask(ref BitVector state)
             return mask;
         }
 
-        private bool IsCapturedInLocalFunction(int slot,
-            ParameterSymbol rangeVariableUnderlyingParameter = null)
+        private bool IsCapturedInLocalFunction(int slot)
         {
             if (slot <= 0) return false;
 
@@ -276,8 +295,8 @@ private bool IsCapturedInLocalFunction(int slot,
             // container is higher in the tree than the nearest
             // local function
             var nearestLocalFunc = GetNearestLocalFunctionOpt(currentMethodOrLambda);
-            return (object)nearestLocalFunc != null &&
-                   IsCaptured(rootSymbol, nearestLocalFunc, rangeVariableUnderlyingParameter);
+
+            return !(nearestLocalFunc is null) && IsCaptured(rootSymbol, nearestLocalFunc);
         }
 
         private LocalFuncUsages GetOrCreateLocalFuncUsages(LocalFunctionSymbol localFunc)

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

@@ -394,38 +394,52 @@ protected void Analyze(ref bool badRegion, DiagnosticBag diagnostics)
         /// <param name="rangeVariableUnderlyingParameter">If variable.Kind is RangeVariable, its underlying lambda parameter. Else null.</param>
         private void CheckCaptured(Symbol variable, ParameterSymbol rangeVariableUnderlyingParameter = null)
         {
-            if (IsCaptured(variable,
-                           currentMethodOrLambda,
-                           rangeVariableUnderlyingParameter))
+            if (IsCaptured(rangeVariableUnderlyingParameter ?? variable, currentMethodOrLambda))
             {
                 NoteCaptured(variable);
             }
         }
 
-        private static bool IsCaptured(Symbol variable,
-                                         MethodSymbol containingMethodOrLambda,
-                                         ParameterSymbol rangeVariableUnderlyingParameter)
+        private static bool IsCaptured(Symbol variable, MethodSymbol containingMethodOrLambda)
         {
             switch (variable.Kind)
             {
+                case SymbolKind.Field:
+                case SymbolKind.Property:
+                case SymbolKind.Event:
+                // Range variables are not captured, but their underlying parameters
+                // may be. If this is a range underlying parameter it will be a
+                // ParameterSymbol, not a RangeVariableSymbol.
+                case SymbolKind.RangeVariable:
+                    return false;
+
                 case SymbolKind.Local:
-                    if (((LocalSymbol)variable).IsConst) break;
-                    goto case SymbolKind.Parameter;
-                case SymbolKind.Parameter:
-                    if (containingMethodOrLambda != variable.ContainingSymbol)
+                    if (((LocalSymbol)variable).IsConst)
                     {
-                        return true;
+                        return false;
                     }
                     break;
-                case SymbolKind.RangeVariable:
-                    if (rangeVariableUnderlyingParameter != null &&
-                        containingMethodOrLambda != rangeVariableUnderlyingParameter.ContainingSymbol)
-                    {
-                        return true;
-                    }
+
+                case SymbolKind.Parameter:
                     break;
+
+                default:
+                    throw ExceptionUtilities.UnexpectedValue(variable.Kind);
+            }
+
+            // Walk up the containing symbols until we find the target function, in which
+            // case the variable is not captured by the target function, or null, in which 
+            // case it is.
+            var currentFunction = variable.ContainingSymbol;
+            while (currentFunction != null)
+            {
+                if (currentFunction == containingMethodOrLambda)
+                {
+                    return false;
+                }
+                currentFunction = currentFunction.ContainingSymbol;
             }
-            return false;
+            return true;
         }
 
         /// <summary>

src/Compilers/CSharp/Test/Semantic/FlowAnalysis/LocalFunctions.cs

@@ -7,6 +7,119 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
     [CompilerTrait(CompilerFeature.LocalFunctions)]
     public class LocalFunctions : FlowTestBase
     {
+        [Fact]
+        [WorkItem(17829, "https://github.com/dotnet/roslyn/issues/17829")]
+        public void UncalledLambdaInLocalFunction()
+        {
+            var comp = CreateStandardCompilation(@"
+using System;
+class C
+{
+    void M()
+    {
+        void L()
+        {
+            Action a = () =>
+            {
+                int x;
+                x++;
+            };
+        }
+    }
+}");
+            comp.VerifyDiagnostics(
+                // (12,17): error CS0165: Use of unassigned local variable 'x'
+                //                 x++;
+                Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(12, 17),
+                // (7,14): warning CS8321: The local function 'L' is declared but never used
+                //         void L()
+                Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "L").WithArguments("L").WithLocation(7, 14));
+        }
+
+        [Fact]
+        [WorkItem(17829, "https://github.com/dotnet/roslyn/issues/17829")]
+        public void LambdaInNestedUncalledLocalFunctions()
+        {
+            var comp = CreateStandardCompilation(@"
+using System;
+class C
+{
+    void M()
+    {
+        void L1()
+        {
+            void L2()
+            {
+                Action a = () =>
+                {
+                    int x;
+                    x++;
+                };
+            }
+            L2();
+        }
+    }
+}");
+            comp.VerifyDiagnostics(
+                // (14,21): error CS0165: Use of unassigned local variable 'x'
+                //                     x++;
+                Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(14, 21),
+                // (7,14): warning CS8321: The local function 'L1' is declared but never used
+                //         void L1()
+                Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "L1").WithArguments("L1").WithLocation(7, 14));
+        }
+
+        [Fact]
+        [WorkItem(17829, "https://github.com/dotnet/roslyn/issues/17829")]
+        public void CapturedInLambdaInUncalledLocalFunction()
+        {
+            var comp = CreateStandardCompilation(@"
+using System;
+class C
+{
+    void M()
+    {
+        void L()
+        {
+            int x;
+            Action f = () => x++;
+        }
+    }
+}");
+            comp.VerifyDiagnostics(
+                // (10,30): error CS0165: Use of unassigned local variable 'x'
+                //             Action f = () => x++;
+                Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(10, 30),
+                // (7,14): warning CS8321: The local function 'L' is declared but never used
+                //         void L()
+                Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "L").WithArguments("L").WithLocation(7, 14));
+        }
+
+        [Fact]
+        [WorkItem(17829, "https://github.com/dotnet/roslyn/issues/17829")]
+        public void CapturedInNestedUncalledLocalFunctions()
+        {
+            var comp = CreateStandardCompilation(@"
+class C
+{
+    void M()
+    {
+        void L1()
+        {
+            int x;
+            void L2() => x++;
+        }
+    }
+}");
+            comp.VerifyDiagnostics(
+                // (9,18): warning CS8321: The local function 'L2' is declared but never used
+                //             void L2() => x++;
+                Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "L2").WithArguments("L2").WithLocation(9, 18),
+                // (6,14): warning CS8321: The local function 'L1' is declared but never used
+                //         void L1()
+                Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "L1").WithArguments("L1").WithLocation(6, 14));
+        }
+
         [Fact]
         public void ConstUnassigned()
         {

src/Compilers/Core/Portable/Collections/BitVector.cs

@@ -216,7 +216,7 @@ public bool IsNull
         /// For the purposes of the intersection, any bits beyond the current length will be treated as zeroes.
         /// Return true if any changes were made to the bits of this bit vector.
         /// </summary>
-        public bool IntersectWith(BitVector other)
+        public bool IntersectWith(in BitVector other)
         {
             bool anyChanged = false;
             int otherLength = other._bits.Length;
@@ -268,7 +268,7 @@ public bool IntersectWith(BitVector other)
         /// <returns>
         /// True if any bits were set as a result of the union.
         /// </returns>
-        public bool UnionWith(BitVector other)
+        public bool UnionWith(in BitVector other)
         {
             bool anyChanged = false;
 

src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs

@@ -1827,9 +1827,7 @@ private static int[] GetValue(out string token)
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)]
         public async Task TestInLocalFunction()
         {
-            // Note: this currently works, but it should be missing.  
-            // This test validates that we don't crash in this case though.
-            await TestInRegularAndScript1Async(
+            await TestMissingInRegularAndScriptAsync(
 @"
 using System;
 using System.Collections.Generic;
@@ -1850,26 +1848,6 @@ void F()
             };
         }
     }
-}",
-@"
-using System;
-using System.Collections.Generic;
-
-class Demo
-{
-    static void Main()
-    {
-        F();
-        void F()
-        {
-            Action f = () =>
-            {
-                Dictionary<int, int> dict = null;
-                dict?.TryGetValue(0, out int x);
-                Console.WriteLine(x);
-            };
-        }
-    }
 }");
         }