There are two pieces of information flow analysis needs to know about
an equality test operand (i.e., an operands of `==`, `!=`, or
`identical()`):
- Their static types. This is used for reachability (e.g., flow
analysis knows that if `f()` has type `Null`, then the body of `if
(f() != null)` is unreachable).
- Whether they take the form of a null literal or a reference to
something promotable. This is used to determine when an `if` test
should promote a something to a non-nullable type.
Previous to this change, both pieces of information were captured by
`FlowAnalysis.equalityOperand_end` into an `ExpressionInfo` object,
and then those objects were passed into
`FlowAnalysis.equalityOperation_end`.
With this change, the client is now responsible for passing the static
types of the operands as separate arguments to
`FlowAnalysis.equalityOperation_end`, and the only information
captured by `equalityOperand_end` is whether the operand is a null
literal or a reference to something promotable.
This has two advantages:
- It avoids unnecessary allocations when analyzing code that doesn't
have flow analysis consequences, since flow analysis no longer needs
to allocate an `ExpressionInfo` for every equality test operand; it
only has to allocate them for null literals and references to things
that are promotable (which is a much smaller number of allocations).
- It means that `FlowAnalysis.equalityOperation_end` no longer needs
to use the `type` field of `ExpressionInfo`. This helps build toward
an eventual goal I have of removing this field, so that
`ExpressionInfo` will simply be a container for a pair of flow
models (one representing the flow state if the expression is `true`,
one representing the flow state if the expression is `false`). I
believe this will make flow analysis easier to reason about, and
will help build toward a long term goal of cleaning up bugs in the
"why not promoted" logic.
Making this change required adding a little bit of plumbing to the
analyzer, so that when analyzing an invocation of `identical`, it
keeps track of both the `ExpressionInfo` and the static type of the
operands; previously it just had to keep track of an `ExpressionInfo`
for each operand. The performance impact of this additional tracking
should be negligible, since this tracking doesn't happen for
invocations of anything other than `identical`.
Change-Id: I3e5473af095f3c8a747e9f527d7e14a21269dc95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389361
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Kallen Tu <kallentu@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Paul Berry
6a5b3d0457