-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Commit 8eeb1c1
[analysis server] Rework RemoveDeadCode using a bottom-up approach.
Previously, `RemoveDeadCode` worked by starting with the
`coveringNode` (the largest node that fully contains the dead code
warning), and then using pattern matching to handle special cases
where the span of the dead code warning didn't precisely match a
single AST node.
I'm working on adding functionality to `RemoveDeadCode` to support the
new sound flow analysis feature, and I've found this approach
difficult to build on, because the span of a dead_code warning is (by
design) not precisely associated with a single AST node.
This change reworks `RemoveDeadCode` based on the insight that the
most important thing about a dead_code warning is where in the source
code it _begins_. So it starts by finding the smallest AST node that
contains the beginning of the dead code range, and then checks whether
the dead code range starts at the beginning of that node.
If it _doesn't_, that only happens in a few special cases, so they are
handled in an ad hoc fashion:
- The node is a binary `||`, `&&`, or `??` operation whose RHS is dead
(in this case the dead code range starts with the binary operator).
- The node is a block whose statements are all live, but whose end is
dead (in this case the dead code range starts with the `}` of the
block).
- The node is a `do` statement whose body is live, but whose condition
is dead (in this case the dead code range starts with the `while`
keyword).
In the remaining cases, where the dead code range starts at the
beginning of the node that was found, `RemoveDeadCode` starts by
walking up the syntax tree to see if any ancestor nodes are fully
dead; if they are, it moves to the biggest fully dead ancestor. Then
it determines whether the resulting node can be removed by looking at
its type and the type of its parent. (For example, a `Statement` can
be removed if its parent is a `Block`).
There are a few notable behavioral differences from the previous
implementation:
- Previously, `RemoveDeadCode` would try to remove a dead `Statement`
regardless of the type of its parent, so `if (false) return;` would
get transformed into `if (false)`, changing the meaning of the code
that follows.
- Dead code elimination now works just as well for `ForElement`s as it
does for `ForStatement`s.
I've added test cases to cover these new behaviors.
In a follow-up commit, I will add more functionality to
`RemoveDeadCode` to specifically address the kinds of dead code that
are expected to occur when sound flow analysis is enabled.
Bug: #60438
Change-Id: I3c2428c905358a881a7eee9a0f06f48270980695
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424200
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Paul Berry <[email protected]>1 parent b2fdd8a commit 8eeb1c1Copy full SHA for 8eeb1c1
File tree
2 files changed
+329
-199
lines changedFilter options
- pkg/analysis_server
- lib/src/services/correction/dart
- test/src/services/correction/fix
2 files changed
+329
-199
lines changed
0 commit comments