match_single_binding: Fix invalid suggestion when match scrutinee has side effects #7095
Conversation
|
r? @phansch (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Y-Nak
force-pushed
the
match_single_binding
branch
3 times, most recently
from
f45381a to
c8e3da4
Compare
|
ping from triage @phansch. Can you have time to review this? |
There was a problem hiding this comment.
@Y-Nak Sorry for the late review.
Expr::can_have_side_effectsis used to determine the scrutinee has side effects, while this method is a little bit conservative for our use case.
Yes I think so, but I also think it's not bad. At least, this lint is improved. I made some comment on this, but overall looks good.
Anyway, thanks for your patience!
Y-Nak
force-pushed
the
match_single_binding
branch
from
c8e3da4 to
8214bf0
Compare
|
Thanks for your review @giraffate! Happy to move forward. |
|
@bors r+ It looks good, thanks! |
|
📌 Commit 8214bf0 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
fixes #7094
changelog:
match_single_binding: Fix invalid suggestion when match scrutinee has side effectsExpr::can_have_side_effectsis used to determine the scrutinee has side effects, while this method is a little bit conservative for our use case. But I'd like to use it to avoid reimplementation of the method and too much heuristics. If you think this is problematic, then I'll implement a custom visitor to address it.