Fix suspicious_xor_used_as_pow.rs performance
#11255
Conversation
|
r? @xFrednet (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
blyxyas
force-pushed
the
fix-perf-sus_xor_used_as_pow
branch
from
c5d685d
to
7a8b9ab
Compare
There was a problem hiding this comment.
Looks good to me, just two tiny nits and a question. If the question is interional, you can also r=me afterwards :)
btw, sorry that the review took so long. I'm currently visiting my hometown, and I was at a festival last weekend. I spent less time on open source than usual, which means it's probably at a normal/healthy level right now ^^.
| format!("{}.pow({})", lit_left | ||
| .node, lit_right.node), |
There was a problem hiding this comment.
NIT: This format is a little weird. rustfmt sadly doesn't work in functions that have if-let chains in them. Can you fix this?
| ); | ||
| matches!(lit_right.node, LitKind::Int(..) | LitKind::Float(..)) && | ||
| matches!(lit_left.node, LitKind::Int(..) | LitKind::Float(..)) && | ||
| NumericLiteral::from_lit_kind(&snippet(cx, lit_right.span, ".."), &lit_right.node).is_some_and(|x| x.is_decimal()) |
There was a problem hiding this comment.
Is it intentional, that you only check the is_decimal for one of the nodes?
There was a problem hiding this comment.
I kinda feel like it's intentional. It doesn't really make sense to check if the left value is decimal, as in mathematical notation the ^ symbol speaks more about the right value than the left.
| @@ -34,21 +36,20 @@ impl LateLintPass<'_> for ConfusingXorAndPow { | |||
| left.span.ctxt() == right.span.ctxt() && | |||
There was a problem hiding this comment.
Since this was your first lint, I'll also be a bit more nitpicky 😉
Can you fix the formatting, to have the && before the condition and not at the end of the line and also remove the tab character in line 34?
blyxyas
force-pushed
the
fix-perf-sus_xor_used_as_pow
branch
from
2f8e879
to
b8fba79
Compare
blyxyas
force-pushed
the
fix-perf-sus_xor_used_as_pow
branch
from
b8fba79
to
3fb8441
Compare
|
@bors r=xFrednet |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
The original
suspicious_xor_used_as_powlint had poor performance, so I fixed that + a little refactor so that module is readable.107 millis. -> 106 millis. Using
SPEEDTESTon Rust's VMsfix #11060
changelog: [
suspicious_xor_used_as_pow]: Improve performance by 0.934%