perf(linter): detect diverging match blocks for node type skipping #14631
Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #14631 will improve performances by 5.65%Comparing Summary
Benchmarks breakdown
Footnotes
|
camchenry
force-pushed
the
10-15-perf_linter_detect_diverging_match_blocks_for_node_type_skipping
branch
from
5bd18a8 to
3c0d09b
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
graphite-app
bot
changed the base branch from
10-15-perf_linter_update_react_prefer-es6-class_to_use_top-level_match
to
graphite-base/14631
camc314
left a comment
camc314
left a comment
There was a problem hiding this comment.
nice! this is great to see
Merge activity
|
…14631) Adds a new kind of code analysis to the linter codegen. This new detector looks for diverging statements in lint rules, like: ```rust let something = match node.kind() { AstKind::CallExpression(_) => { ... } _ => return, } ``` The important part is that the expression _must_ diverge (i.e., it must have something like a `return` in its wildcard block). That way, we know only the matched node types in the match arms are used in the rule.
graphite-app
bot
force-pushed
the
10-15-perf_linter_detect_diverging_match_blocks_for_node_type_skipping
branch
from
3c0d09b to
12a9934
Compare
graphite-app
bot
deleted the
10-15-perf_linter_detect_diverging_match_blocks_for_node_type_skipping
branch
| } | ||
|
|
||
| // Look at the first statement in the function body. | ||
| let stmt = block.stmts.first()?; |
There was a problem hiding this comment.
Just a small note here:
IMO it'd likely be preferable to use block.stmts.first().unwrap().
.unwrap() will often cause the compiler to backtrack and try to prove that the unwrap can't fail. In this case I'd assume it can prove this fairly easily as the block.stmts.len() <= 1 check is right above. So it'll remove the branch. Whereas with ? it probably won't backtrack and won't figure this out.
Someone taught me this surprising fact last year - sometimes a well-placed assert! can result in less code, rather than more.
Adds a new kind of code analysis to the linter codegen. This new detector looks for diverging statements in lint rules, like:
The important part is that the expression must diverge (i.e., it must have something like a
returnin its wildcard block). That way, we know only the matched node types in the match arms are used in the rule.