Don't mark replace_box suggestion machine-applicable for Box<&mut _> - #17554
Don't mark replace_box suggestion machine-applicable for Box<&mut _>#17554DoTuanAnh2k1 wants to merge 1 commit into
replace_box suggestion machine-applicable for Box<&mut _>#17554Conversation
rustbot
added
S-waiting-on-community-reviews
|
Thanks for the pull request, and welcome! You should hear from one of our reviewers after this PR gets at least 2 reviews from the community. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
| // `*b = &mut x` keeps the box's existing borrow alive across the assignment, | ||
| // while `b = Box::new(&mut x)` drops the old box first; for `Box<&mut _>` that | ||
| // difference can change borrow-check results, so don't auto-apply the rewrite. | ||
| let mut app = if matches!(inner_ty.kind(), ty::Ref(_, _, Mutability::Mut)) { |
There was a problem hiding this comment.
This applies to any type with a region variable, not just mutable references. You can check inner_ty.flags().contains(HAS_RE_ERASED).
There was a problem hiding this comment.
Good call — switched to inner_ty.has_type_flags(TypeFlags::HAS_RE_ERASED) so it covers any boxed type carrying a lifetime, not just &mut. Checked &mut _, &_, and a struct with a lifetime param all come out MaybeIncorrect now, while plain values stay MachineApplicable.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
…nces Rewriting `b = Box::new(x)` to `*b = x` keeps any borrow already stored in the box alive across the assignment, whereas reassigning the box drops the old one first. When the boxed type carries a lifetime that difference can change borrow-check results, so the suggestion can't be applied blindly. Keep emitting the lint but downgrade the suggestion to MaybeIncorrect when the boxed type contains a region; other element types stay unchanged.
8a6d8de to
a49d0e7
Compare
|
Moved the cases into @rustbot ready |
rustbot
added
S-waiting-on-review
|
☔ The latest upstream changes (possibly #17607) made this pull request unmergeable. Please resolve the merge conflicts. |
5 participants
replace_boxflagsb = Box::new(x)on an existingBox<T>and suggests writing to the boxed storage instead (*b = x) to skip an allocation. That suggestion isMachineApplicable, but forBox<&mut T>the two forms aren't interchangeable:*b = &mut xkeeps the reference already stored in the box borrowed across the assignment, whileb = Box::new(&mut x)drops the old box (and its borrow) first. As shown in the issue, that can change borrow-check results, socargo clippy --fixcould turn compiling code into code that no longer builds.Keep emitting the lint (the extra allocation is still real), but downgrade the "replace existing content with inner value instead" suggestion to
MaybeIncorrectwhen the boxed element type is a&mutreference. Other element types are unaffected.Testing: added a
Box<&mut _>case totests/ui/replace_box.rs; thereplace_boxUI test passes andcargo dev fmt --checkis clean. Clippy's UI harness applies suggestions regardless of applicability, so the downgrade isn't visible in the.fixedoutput — I confirmed it via the diagnostic'sapplicabilityfield: theBox<&mut _>suggestion is nowMaybeIncorrect, whileBox<u32>staysMachineApplicable.fixes #17549
changelog: [
replace_box]: don't emit a machine-applicable suggestion forBox<&mut _>Prepared with AI assistance; reviewed and tested by me.