Suggest removing &mut from a &mut borrow
#77110
Conversation
rust-highfive
added
the
S-waiting-on-review
tesuji
force-pushed
the
e0596-amp_mut
branch
3 times, most recently
from
3f4fa7f to
ff4c2e1
Compare
| } | ||
| }) | ||
| .unwrap_or(false) | ||
| if match self.body.local_decls.get(local) { |
There was a problem hiding this comment.
Should we refactor this into another function?
rust/compiler/rustc_middle/src/mir/mod.rs
Line 937 in 81e0270
as
if self.body.local_decls
.get(local)
.map_or(false, |local_decl| local_decl.mut_borrow_of_mutable_ref(self.local_names[local])) => {But I wonder if it will even failed to get local? The next block just use local_decls[local] but not sure if it will work here.
if self.body.local_decls[local]
.mut_borrow_of_mutable_ref(self.local_names[local]) => {There was a problem hiding this comment.
I don't get it. Did you mean to make a method on LocalDecls?
There was a problem hiding this comment.
I don't think there are code elsewhere that need to reuse the logic.
There was a problem hiding this comment.
Yes, that's true but I don't know if putting it as another function on itself would be good since it may easily be lost on context. Maybe refactor it as an implementation under MirBorrowckCtxt which is the same thing?
There was a problem hiding this comment.
Honestly I prefer free function as it since it don't need other states than passed arguments.
But I don't have strong opinion, let's let it for Esteban or David.
There was a problem hiding this comment.
I don't know if it is worth extracting this into a method, but I would prefer something like this than a match here:
self.body.local_decls
.get(local)
.map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local])
.unwrap_or(false)
|
Just wondering, could there be multiple layers for this? Like multiple |
|
estebank's backlog is really long, so let's find another reviewer: r? @davidtwco |
| } | ||
| }) | ||
| .unwrap_or(false) | ||
| if match self.body.local_decls.get(local) { |
There was a problem hiding this comment.
I don't know if it is worth extracting this into a method, but I would prefer something like this than a match here:
self.body.local_decls
.get(local)
.map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local])
.unwrap_or(false)
|
Sorry, I had to rebase because my local copy doesn't build with old worktree. |
Fix a typo: minding -> binding Add test for &mut &mut
|
@bors r=davidtwco |
|
📌 Commit ab226bd has been approved by |
bors
added
S-waiting-on-bors
| .local_decls | ||
| .get(local) | ||
| .map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local])) | ||
| .unwrap_or(false) => | ||
| { | ||
| err.span_label(span, format!("cannot {ACT}", ACT = act)); | ||
| err.span_label(span, "try removing `&mut` here"); |
There was a problem hiding this comment.
It'd be nice to come back to this at some other time to gather a good span to make this a structured suggestion :)
|
☀️ Test successful - checks-actions, checks-azure |
Modify the code added in #54720.
Closes #75871