Fix DataFlowsOut for certain local function situations (#41802)

There are two problems here, the first of which is the most important.

First, certain data flow analyses use "normalization" to adjust an existing
state variable for more variables declared after that state was created.
The default behavior is to extend that state to the "top" state, meaning
the starting state for that data flow analysis. This is usually the
right extension. However, some analysis requires extending the
variables the other way, to the bottom state. This PR adds functionality
for that customization.

The second problem is that data flow for local functions may include
variables that are not captured. This is by filtering the transfer function
to only include captured variables.

Fixes fixes #41600

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

@@ -34,10 +34,10 @@ internal abstract class AbstractLocalFunctionState
             /// </summary>
             public TLocalState StateFromTop;
 
-            public AbstractLocalFunctionState(TLocalState unreachableState)
+            public AbstractLocalFunctionState(TLocalState stateFromBottom, TLocalState stateFromTop)
             {
-                StateFromBottom = unreachableState.Clone();
-                StateFromTop = unreachableState.Clone();
+                StateFromBottom = stateFromBottom;
+                StateFromTop = stateFromTop;
             }
 
             public bool Visited = false;
@@ -155,7 +155,8 @@ private bool RecordStateChange(
             TLocalFunctionState currentState,
             ref TLocalState stateAtReturn)
         {
-            bool anyChanged = Join(ref currentState.StateFromTop, ref stateAtReturn);
+            bool anyChanged = LocalFunctionEnd(savedState, currentState, ref stateAtReturn);
+            anyChanged |= Join(ref currentState.StateFromTop, ref stateAtReturn);
 
             if (NonMonotonicState.HasValue)
             {
@@ -166,10 +167,6 @@ private bool RecordStateChange(
                 Meet(ref value, ref stateAtReturn);
                 anyChanged |= Join(ref currentState.StateFromBottom, ref value);
             }
-
-            // N.B. Do NOT shortcut this operation. LocalFunctionEnd may have important
-            // side effects to the local function state
-            anyChanged |= LocalFunctionEnd(savedState, currentState, ref stateAtReturn);
             return anyChanged;
         }
 

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

@@ -67,7 +67,7 @@ public bool Reachable
         internal sealed class LocalFunctionState : AbstractLocalFunctionState
         {
             public LocalFunctionState(LocalState unreachableState)
-                : base(unreachableState)
+                : base(unreachableState.Clone(), unreachableState.Clone())
             { }
         }
 

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

@@ -12,14 +12,20 @@ internal partial class DefiniteAssignmentPass
         internal sealed class LocalFunctionState : AbstractLocalFunctionState
         {
             public BitVector ReadVars = BitVector.Empty;
-            public ref LocalState WrittenVars => ref StateFromTop;
 
-            public LocalFunctionState(LocalState unreachableState)
-                : base(unreachableState)
+            public BitVector CapturedMask = BitVector.Null;
+            public BitVector InvertedCapturedMask = BitVector.Null;
+
+            public LocalFunctionState(LocalState stateFromBottom, LocalState stateFromTop)
+                : base(stateFromBottom, stateFromTop)
             { }
         }
 
-        protected override LocalFunctionState CreateLocalFunctionState() => new LocalFunctionState(UnreachableState());
+        protected override LocalFunctionState CreateLocalFunctionState()
+            => new LocalFunctionState(
+                // The bottom state should assume all variables, even new ones, are assigned
+                new LocalState(BitVector.AllSet(nextVariableSlot), normalizeToBottom: true),
+                UnreachableState());
 
         protected override void VisitLocalFunctionUse(
             LocalFunctionSymbol localFunc,
@@ -112,15 +118,12 @@ private void RecordReadInLocalFunction(int slot)
             }
         }
 
-        private BitVector GetCapturedBitmask(ref BitVector state)
+        private BitVector GetCapturedBitmask()
         {
-            BitVector mask = BitVector.Empty;
-            for (int slot = 1; slot < state.Capacity; slot++)
+            BitVector mask = BitVector.AllSet(nextVariableSlot);
+            for (int slot = 1; slot < nextVariableSlot; slot++)
             {
-                if (IsCapturedInLocalFunction(slot))
-                {
-                    mask[slot] = true;
-                }
+                mask[slot] = IsCapturedInLocalFunction(slot);
             }
 
             return mask;
@@ -166,7 +169,7 @@ protected override LocalFunctionState LocalFunctionStart(LocalFunctionState star
             // assignment errors if any of the captured variables is not assigned
             // on a particular branch.
 
-            var savedState = new LocalFunctionState(UnreachableState());
+            var savedState = CreateLocalFunctionState();
             savedState.ReadVars = startState.ReadVars.Clone();
             startState.ReadVars.Clear();
             return savedState;
@@ -181,10 +184,24 @@ protected override bool LocalFunctionEnd(
             LocalFunctionState currentState,
             ref LocalState stateAtReturn)
         {
+            if (currentState.CapturedMask.IsNull)
+            {
+                currentState.CapturedMask = GetCapturedBitmask();
+                currentState.InvertedCapturedMask = currentState.CapturedMask.Clone();
+                currentState.InvertedCapturedMask.Invert();
+            }
+            // Filter the modified state variables to only captured variables
+            stateAtReturn.Assigned.IntersectWith(currentState.CapturedMask);
+            if (NonMonotonicState.HasValue)
+            {
+                var state = NonMonotonicState.Value;
+                state.Assigned.UnionWith(currentState.InvertedCapturedMask);
+                NonMonotonicState = state;
+            }
+
             // Build a list of variables that are both captured and read before assignment
-            var capturedMask = GetCapturedBitmask(ref currentState.ReadVars);
             var capturedAndRead = currentState.ReadVars;
-            capturedAndRead.IntersectWith(capturedMask);
+            capturedAndRead.IntersectWith(currentState.CapturedMask);
 
             // Union and check to see if there are any changes
             return savedState.ReadVars.UnionWith(capturedAndRead);

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

@@ -741,9 +741,18 @@ protected override void Normalize(ref LocalState state)
             {
                 var id = variableBySlot[i];
                 int slot = id.ContainingSlot;
-                state.Assigned[i] = (slot > 0) &&
+
+                bool assign = (slot > 0) &&
                     state.Assigned[slot] &&
                     variableBySlot[slot].Symbol.GetTypeOrReturnType().TypeKind == TypeKind.Struct;
+
+                if (state.NormalizeToBottom && slot == 0)
+                {
+                    // NormalizeToBottom means new variables are assumed to be assigned (bottom state)
+                    assign = true;
+                }
+
+                state.Assigned[i] = assign;
             }
         }
 
@@ -2165,16 +2174,19 @@ protected override bool Join(ref LocalState self, ref LocalState other)
         }
 
 #if REFERENCE_STATE
-        internal class LocalState : ILocalState
+        internal class LocalState : ILocalDataFlowState
 #else
-        internal struct LocalState : ILocalState
+        internal struct LocalState : ILocalDataFlowState
 #endif
         {
             internal BitVector Assigned;
 
-            internal LocalState(BitVector assigned)
+            public bool NormalizeToBottom { get; }
+
+            internal LocalState(BitVector assigned, bool normalizeToBottom = false)
             {
                 this.Assigned = assigned;
+                NormalizeToBottom = normalizeToBottom;
                 Debug.Assert(!assigned.IsNull);
             }
 

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

@@ -14,9 +14,18 @@ namespace Microsoft.CodeAnalysis.CSharp
     /// Does a data flow analysis for state attached to local variables and fields of struct locals.
     /// </summary>
     internal abstract partial class LocalDataFlowPass<TLocalState, TLocalFunctionState> : AbstractFlowPass<TLocalState, TLocalFunctionState>
-        where TLocalState : AbstractFlowPass<TLocalState, TLocalFunctionState>.ILocalState
+        where TLocalState : LocalDataFlowPass<TLocalState, TLocalFunctionState>.ILocalDataFlowState
         where TLocalFunctionState : AbstractFlowPass<TLocalState, TLocalFunctionState>.AbstractLocalFunctionState
     {
+        internal interface ILocalDataFlowState : ILocalState
+        {
+            /// <summary>
+            /// True if new variables introduced in <see cref="AbstractFlowPass{TLocalState, TLocalFunctionState}" /> should be set
+            /// to the bottom state. False if they should be set to the top state.
+            /// </summary>
+            bool NormalizeToBottom { get; }
+        }
+
         /// <summary>
         /// A mapping from local variables to the index of their slot in a flow analysis local state.
         /// </summary>
@@ -172,6 +181,9 @@ private int GetSlotDepth(int slot)
             return depth;
         }
 
+        /// <summary>
+        /// Sets the starting state for any newly declared variables in the LocalDataFlowPass.
+        /// </summary>
         protected abstract void Normalize(ref TLocalState state);
 
         /// <summary>

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

@@ -8516,9 +8516,9 @@ protected override bool Join(ref LocalState self, ref LocalState other)
 
         [DebuggerDisplay("{GetDebuggerDisplay(), nq}")]
 #if REFERENCE_STATE
-        internal class LocalState : ILocalState
+        internal class LocalState : ILocalDataFlowState
 #else
-        internal struct LocalState : ILocalState
+        internal struct LocalState : ILocalDataFlowState
 #endif
         {
             // The representation of a state is a bit vector with two bits per slot:
@@ -8530,6 +8530,8 @@ internal struct LocalState : ILocalState
 
             public bool Reachable => _state[0];
 
+            public bool NormalizeToBottom => false;
+
             public static LocalState ReachableState(int capacity)
             {
                 if (capacity < 1)
@@ -8612,7 +8614,7 @@ internal sealed class LocalFunctionState : AbstractLocalFunctionState
             /// </summary>
             public LocalState StartingState;
             public LocalFunctionState(LocalState unreachableState)
-                : base(unreachableState)
+                : base(unreachableState.Clone(), unreachableState.Clone())
             {
                 StartingState = unreachableState;
             }

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

@@ -4131,6 +4131,125 @@ static void Main(string[] args)
 
         #region "lambda"
 
+        [Fact]
+        [WorkItem(41600, "https://github.com/dotnet/roslyn/pull/41600")]
+        public void DataFlowAnalysisLocalFunctions10()
+        {
+            var dataFlow = CompileAndAnalyzeDataFlowExpression(@"
+class C
+{
+    public void M()
+    {
+        bool Dummy(params object[] x) {return true;}
+
+        try {}
+        catch when (/*<bind>*/TakeOutParam(out var x1)/*</bind>*/ && x1 > 0)
+        {
+            Dummy(x1);
+        }
+
+        var x4 = 11;
+        Dummy(x4);
+
+        try {}
+        catch when (TakeOutParam(out var x4) && x4 > 0)
+        {
+            Dummy(x4);
+        }
+
+        try {}
+        catch when (x6 && TakeOutParam(out var x6))
+        {
+            Dummy(x6);
+        }
+
+        try {}
+        catch when (TakeOutParam(out var x7) && x7 > 0)
+        {
+            var x7 = 12;
+            Dummy(x7);
+        }
+
+        try {}
+        catch when (TakeOutParam(out var x8) && x8 > 0)
+        {
+            Dummy(x8);
+        }
+
+        System.Console.WriteLine(x8);
+
+        try {}
+        catch when (TakeOutParam(out var x9) && x9 > 0)
+        {   
+            Dummy(x9);
+            try {}
+            catch when (TakeOutParam(out var x9) && x9 > 0) // 2
+            {
+                Dummy(x9);
+            }
+        }
+
+        try {}
+        catch when (TakeOutParam(y10, out var x10))
+        {   
+            var y10 = 12;
+            Dummy(y10);
+        }
+
+        //    try {}
+        //    catch when (TakeOutParam(y11, out var x11)
+        //    {   
+        //        let y11 = 12;
+        //        Dummy(y11);
+        //    }
+
+        try {}
+        catch when (Dummy(TakeOutParam(out var x14), 
+                            TakeOutParam(out var x14), // 2
+                            x14))
+        {
+            Dummy(x14);
+        }
+
+        try {}
+        catch (System.Exception x15)
+                when (Dummy(TakeOutParam(out var x15), x15))
+        {
+            Dummy(x15);
+        }
+
+        static bool TakeOutParam(out int x) 
+        {
+            x = 123;
+            return true;
+        }
+        static bool TakeOutParam(object y, out int x)
+        {
+            x = 123;
+            return true;
+        }
+
+    }
+}
+");
+            Assert.True(dataFlow.Succeeded);
+            Assert.Null(GetSymbolNamesJoined(dataFlow.Captured));
+            Assert.Null(GetSymbolNamesJoined(dataFlow.CapturedInside));
+            Assert.Null(GetSymbolNamesJoined(dataFlow.CapturedOutside));
+            Assert.Equal("x1", GetSymbolNamesJoined(dataFlow.VariablesDeclared));
+            Assert.Null(GetSymbolNamesJoined(dataFlow.DataFlowsIn));
+            Assert.Equal("x1", GetSymbolNamesJoined(dataFlow.DataFlowsOut));
+            Assert.Equal("this", GetSymbolNamesJoined(dataFlow.DefinitelyAssignedOnEntry));
+            Assert.Equal("this, x1", GetSymbolNamesJoined(dataFlow.DefinitelyAssignedOnExit));
+            Assert.Null(GetSymbolNamesJoined(dataFlow.ReadInside));
+            Assert.Equal("x1", GetSymbolNamesJoined(dataFlow.WrittenInside));
+            Assert.Equal("this, x1, x4, x4, x6, x7, x7, x8, x9, x9, y10, x14, x15, x, x",
+                GetSymbolNamesJoined(dataFlow.ReadOutside));
+            Assert.Equal("this, x, x4, x4, x6, x7, x7, x8, x9, x9, x10, " +
+                         "y10, x14, x14, x15, x15, x, y, x",
+                GetSymbolNamesJoined(dataFlow.WrittenOutside));
+        }
+
         [Fact]
         [WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
         public void DataFlowAnalysisLocalFunctions9()

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

@@ -22,7 +22,7 @@ internal struct BitVector : IEquatable<BitVector>
 
         // Cannot expose the following two field publicly because this structure is mutable
         // and might become not null/empty, unless we restrict access to it.
-        private static readonly Word[] s_emptyArray = Array.Empty<Word>();
+        private static Word[] s_emptyArray => Array.Empty<Word>();
         private static readonly BitVector s_nullValue = default;
         private static readonly BitVector s_emptyValue = new BitVector(0, s_emptyArray, 0);
 
@@ -222,6 +222,21 @@ public BitVector Clone()
             return new BitVector(_bits0, newBits, _capacity);
         }
 
+        /// <summary>
+        /// Invert all the bits in the vector.
+        /// </summary>
+        public void Invert()
+        {
+            _bits0 = ~_bits0;
+            if (!(_bits is null))
+            {
+                for (int i = 0; i < _bits.Length; i++)
+                {
+                    _bits[i] = ~_bits[i];
+                }
+            }
+        }
+
         /// <summary>
         /// Is the given bit array null?
         /// </summary>