Fix DataFlowsOut for certain local function situations #41802
Conversation
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 and also fixes dotnet#41600.
agocke
force-pushed
the
localfunc-data-flow
branch
from
70c922b to
a6a0f3c
Compare
There's no strict design, but it seems reasonable that region analysis consumers would only expect captured variables to show up in the input/output variables when calling a local function.
agocke
force-pushed
the
localfunc-data-flow
branch
from
a6a0f3c to
869353b
Compare
src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs
Outdated
Show resolved
Hide resolved
|
Does this PR fix #41853 as well? |
|
No, that looks like an issue specific to DataFlowsIn. |
| { | ||
| BitVector mask = BitVector.Empty; | ||
| for (int slot = 1; slot < state.Capacity; slot++) | ||
| BitVector mask = BitVector.AllSet(1); |
There was a problem hiding this comment.
1 [](start = 46, length = 1)
nextVariableSlot (so the internal array doesn't have to be reallocated). Then in the loop,
mask[slot] = IsCapturedInLocalFunction(slot) #Closed
I would have expected this to override Refers to: src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs:8552 in 64a4c6a. [](commit_id = 64a4c6a, deletion_comment = False) |
| @@ -4131,6 +4131,125 @@ static void Main(string[] args) | |||
|
|
|||
| #region "lambda" | |||
|
|
|||
| [Fact] | |||
| [WorkItem(41802, "https://github.com/dotnet/roslyn/pull/41802")] | |||
There was a problem hiding this comment.
41802 [](start = 18, length = 5)
41600 #Closed
|
Done reviewing Iteration 3. |
|
@gafter For nullable, we do a slightly different analysis since it's not possible to be flow dependent, as only one type can be reported in the semantic model. The design is to track a single "StartingState" that's Joined at every local function invocation and the starting state for the local function is the Join of every use site. I think the right behavior here is to treat undeclared variables as unassigned, since effectively they are at that point in the code. |
ghost
added this to the 
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.
This PR is structured into two commits, each of which fix the associated
problem.
Fixes fixes #41600