Avoid emitting assigning_clones when cloned data borrows from the place to clone into
#12756
Conversation
|
r? @Alexendoo rustbot has assigned @Alexendoo. Use |
rustbot
added
the
S-waiting-on-review
y21
force-pushed
the
assigning_clones_lifetimes
branch
from
a413cd1 to
0a93106
Compare
| let mut s = (NoDrop, NoDrop); | ||
| let s2 = &s.1; | ||
| s.0 = s2.clone(); |
There was a problem hiding this comment.
This is a false negative now but fixing it seems like it'd require large changes to PossibleBorrowerMap to teach it disjoint borrows, but I suppose this FN might be better than a FP
There was a problem hiding this comment.
In a sense, a performance lint is about reducing time wasted by the computer, but as human time is more expensive, it's surely more costly to waste human time if no computing performance is likely to be gained in the process.
|
☔ The latest upstream changes (presumably #12783) made this pull request unmergeable. Please resolve the merge conflicts. |
y21
force-pushed
the
assigning_clones_lifetimes
branch
from
0a93106 to
60508f5
Compare
|
It would be great to merge this. Even if it's not perfect, it's much better than what has shipped in stable and causing numerous annoyances. |
|
@bors r+ It seems reasonable to me, it is relying on the MIR layout a bit but it seems like it would be fairly straightforward to adapt if that ever changes |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
The lint is no longer triggered because of the switch from educe to derivative. educe generates an implementation of Clone::clone_from for every non-Copy struct and enum. derivative only generates one if explicitly requested. The false positives may have been fixed in rust-lang/rust-clippy#12756. The fix did not make it into Rust 1.79.0 or Rust 1.80.0.
Fixes #12444
Fixes #12460
Fixes #12749
Fixes #12757
Fixes #12929
I think the documentation for the function should describe what- and how this is fixing the issues well.
It avoids emitting a warning when the data being cloned borrows from the place to clone into, which is information that we can get from
PossibleBorrowerMap. Unfortunately, it is a tiny bit tedious to match on the MIR like that and I'm not sure if this is possibly relying a bit too much on the exact MIR lowering for assignments.Things left to do:
Droptypeschangelog: [
assigning_clones]: avoid warning when the suggestion would lead to a borrow-check error