commit 6a5b3d04576ec99160d48113c9c5139b8f35b40b
author Paul Berry <paulberry@google.com> Thu Oct 24 16:37:11 2024 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Oct 24 16:37:11 2024 +0000
tree 4743bf44df3a6311ec1a680c1d0f96d8e23b513c
parent e567b0ab5243df7658424f08673c8c0d022b7470

Flow analysis: don't capture type information in equalityOperand_end.

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>