Skip to content

Commit aa5dbee

Browse files
borsbors
committedAug 24, 2023
Auto merge of #115147 - estebank:issue-114311, r=davidtwco
Suggest mutable borrow on read only for-loop that should be mutable ``` error[E0596]: cannot borrow `*test` as mutable, as it is behind a `&` reference --> $DIR/suggest-mut-iterator.rs:22:9 | LL | for test in &tests { | ------ this iterator yields `&` references LL | test.add(2); | ^^^^ `test` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: use a mutable iterator instead | LL | for test in &mut tests { | +++ ``` Fix #114311.
·
1.86.01.74.0
2 parents 18be272 + c1a7af0 commit aa5dbee

File tree

4 files changed

+167
-88
lines changed

4 files changed

+167
-88
lines changed
 

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

+91-88
Original file line numberDiff line numberDiff line change
@@ -225,17 +225,17 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
225225
}
226226
if suggest {
227227
borrow_spans.var_subdiag(
228-
None,
229-
&mut err,
230-
Some(mir::BorrowKind::Mut { kind: mir::MutBorrowKind::Default }),
231-
|_kind, var_span| {
232-
let place = self.describe_any_place(access_place.as_ref());
233-
crate::session_diagnostics::CaptureVarCause::MutableBorrowUsePlaceClosure {
234-
place,
235-
var_span,
236-
}
237-
},
238-
);
228+
None,
229+
&mut err,
230+
Some(mir::BorrowKind::Mut { kind: mir::MutBorrowKind::Default }),
231+
|_kind, var_span| {
232+
let place = self.describe_any_place(access_place.as_ref());
233+
crate::session_diagnostics::CaptureVarCause::MutableBorrowUsePlaceClosure {
234+
place,
235+
var_span,
236+
}
237+
},
238+
);
239239
}
240240
borrow_span
241241
}
@@ -262,11 +262,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
262262
} => {
263263
err.span_label(span, format!("cannot {act}"));
264264

265-
if let Some(span) = get_mut_span_in_struct_field(
266-
self.infcx.tcx,
267-
Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty,
268-
*field,
269-
) {
265+
let place = Place::ty_from(local, proj_base, self.body, self.infcx.tcx);
266+
if let Some(span) = get_mut_span_in_struct_field(self.infcx.tcx, place.ty, *field) {
270267
err.span_suggestion_verbose(
271268
span,
272269
"consider changing this to be mutable",
@@ -781,83 +778,88 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
781778

782779
// Attempt to search similar mutable associated items for suggestion.
783780
// In the future, attempt in all path but initially for RHS of for_loop
784-
fn suggest_similar_mut_method_for_for_loop(&self, err: &mut Diagnostic) {
781+
fn suggest_similar_mut_method_for_for_loop(&self, err: &mut Diagnostic, span: Span) {
785782
use hir::{
786-
Expr,
787-
ExprKind::{Block, Call, DropTemps, Match, MethodCall},
783+
BorrowKind, Expr,
784+
ExprKind::{AddrOf, Block, Call, MethodCall},
788785
};
789786

790787
let hir_map = self.infcx.tcx.hir();
791-
if let Some(body_id) = hir_map.maybe_body_owned_by(self.mir_def_id()) {
792-
if let Block(
793-
hir::Block {
794-
expr:
795-
Some(Expr {
796-
kind:
797-
DropTemps(Expr {
798-
kind:
799-
Match(
800-
Expr {
801-
kind:
802-
Call(
803-
_,
804-
[
805-
Expr {
806-
kind:
807-
MethodCall(path_segment, _, _, span),
808-
hir_id,
809-
..
810-
},
811-
..,
812-
],
813-
),
814-
..
815-
},
816-
..,
817-
),
818-
..
819-
}),
820-
..
821-
}),
822-
..
823-
},
824-
_,
825-
) = hir_map.body(body_id).value.kind
826-
{
827-
let opt_suggestions = self
828-
.infcx
829-
.tcx
830-
.typeck(path_segment.hir_id.owner.def_id)
831-
.type_dependent_def_id(*hir_id)
832-
.and_then(|def_id| self.infcx.tcx.impl_of_method(def_id))
833-
.map(|def_id| self.infcx.tcx.associated_items(def_id))
834-
.map(|assoc_items| {
835-
assoc_items
836-
.in_definition_order()
837-
.map(|assoc_item_def| assoc_item_def.ident(self.infcx.tcx))
838-
.filter(|&ident| {
839-
let original_method_ident = path_segment.ident;
840-
original_method_ident != ident
841-
&& ident
842-
.as_str()
843-
.starts_with(&original_method_ident.name.to_string())
844-
})
845-
.map(|ident| format!("{ident}()"))
846-
.peekable()
847-
});
788+
struct Finder<'tcx> {
789+
span: Span,
790+
expr: Option<&'tcx Expr<'tcx>>,
791+
}
848792

849-
if let Some(mut suggestions) = opt_suggestions
850-
&& suggestions.peek().is_some()
851-
{
852-
err.span_suggestions(
853-
*span,
854-
"use mutable method",
855-
suggestions,
856-
Applicability::MaybeIncorrect,
857-
);
793+
impl<'tcx> Visitor<'tcx> for Finder<'tcx> {
794+
fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) {
795+
if e.span == self.span && self.expr.is_none() {
796+
self.expr = Some(e);
858797
}
798+
hir::intravisit::walk_expr(self, e);
859799
}
860-
};
800+
}
801+
if let Some(body_id) = hir_map.maybe_body_owned_by(self.mir_def_id())
802+
&& let Block(block, _) = hir_map.body(body_id).value.kind
803+
{
804+
// `span` corresponds to the expression being iterated, find the `for`-loop desugared
805+
// expression with that span in order to identify potential fixes when encountering a
806+
// read-only iterator that should be mutable.
807+
let mut v = Finder {
808+
span,
809+
expr: None,
810+
};
811+
v.visit_block(block);
812+
if let Some(expr) = v.expr && let Call(_, [expr]) = expr.kind {
813+
match expr.kind {
814+
MethodCall(path_segment, _, _, span) => {
815+
// We have `for _ in iter.read_only_iter()`, try to
816+
// suggest `for _ in iter.mutable_iter()` instead.
817+
let opt_suggestions = self
818+
.infcx
819+
.tcx
820+
.typeck(path_segment.hir_id.owner.def_id)
821+
.type_dependent_def_id(expr.hir_id)
822+
.and_then(|def_id| self.infcx.tcx.impl_of_method(def_id))
823+
.map(|def_id| self.infcx.tcx.associated_items(def_id))
824+
.map(|assoc_items| {
825+
assoc_items
826+
.in_definition_order()
827+
.map(|assoc_item_def| assoc_item_def.ident(self.infcx.tcx))
828+
.filter(|&ident| {
829+
let original_method_ident = path_segment.ident;
830+
original_method_ident != ident
831+
&& ident.as_str().starts_with(
832+
&original_method_ident.name.to_string(),
833+
)
834+
})
835+
.map(|ident| format!("{ident}()"))
836+
.peekable()
837+
});
838+
839+
if let Some(mut suggestions) = opt_suggestions
840+
&& suggestions.peek().is_some()
841+
{
842+
err.span_suggestions(
843+
span,
844+
"use mutable method",
845+
suggestions,
846+
Applicability::MaybeIncorrect,
847+
);
848+
}
849+
}
850+
AddrOf(BorrowKind::Ref, Mutability::Not, expr) => {
851+
// We have `for _ in &i`, suggest `for _ in &mut i`.
852+
err.span_suggestion_verbose(
853+
expr.span.shrink_to_lo(),
854+
"use a mutable iterator instead",
855+
"mut ".to_string(),
856+
Applicability::MachineApplicable,
857+
);
858+
}
859+
_ => {}
860+
}
861+
}
862+
}
861863
}
862864

863865
/// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected.
@@ -1003,9 +1005,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
10031005
match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) {
10041006
// on for loops, RHS points to the iterator part
10051007
Some(DesugaringKind::ForLoop) => {
1006-
self.suggest_similar_mut_method_for_for_loop(err);
1008+
let span = opt_assignment_rhs_span.unwrap();
1009+
self.suggest_similar_mut_method_for_for_loop(err, span);
10071010
err.span_label(
1008-
opt_assignment_rhs_span.unwrap(),
1011+
span,
10091012
format!("this iterator yields `{pointer_sigil}` {pointer_desc}s",),
10101013
);
10111014
None

‎tests/ui/borrowck/suggest-mut-iterator.fixed

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// run-rustfix
2+
struct Test {
3+
a: u32
4+
}
5+
6+
impl Test {
7+
pub fn add(&mut self, value: u32) {
8+
self.a += value;
9+
}
10+
11+
pub fn print_value(&self) {
12+
println!("Value of a is: {}", self.a);
13+
}
14+
}
15+
16+
fn main() {
17+
let mut tests = Vec::new();
18+
for i in 0..=10 {
19+
tests.push(Test {a: i});
20+
}
21+
for test in &mut tests {
22+
test.add(2); //~ ERROR cannot borrow `*test` as mutable, as it is behind a `&` reference
23+
}
24+
for test in &mut tests {
25+
test.add(2);
26+
}
27+
for test in &tests {
28+
test.print_value();
29+
}
30+
}

‎tests/ui/borrowck/suggest-mut-iterator.rs

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// run-rustfix
2+
struct Test {
3+
a: u32
4+
}
5+
6+
impl Test {
7+
pub fn add(&mut self, value: u32) {
8+
self.a += value;
9+
}
10+
11+
pub fn print_value(&self) {
12+
println!("Value of a is: {}", self.a);
13+
}
14+
}
15+
16+
fn main() {
17+
let mut tests = Vec::new();
18+
for i in 0..=10 {
19+
tests.push(Test {a: i});
20+
}
21+
for test in &tests {
22+
test.add(2); //~ ERROR cannot borrow `*test` as mutable, as it is behind a `&` reference
23+
}
24+
for test in &mut tests {
25+
test.add(2);
26+
}
27+
for test in &tests {
28+
test.print_value();
29+
}
30+
}

‎tests/ui/borrowck/suggest-mut-iterator.stderr

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error[E0596]: cannot borrow `*test` as mutable, as it is behind a `&` reference
2+
--> $DIR/suggest-mut-iterator.rs:22:9
3+
|
4+
LL | for test in &tests {
5+
| ------ this iterator yields `&` references
6+
LL | test.add(2);
7+
| ^^^^ `test` is a `&` reference, so the data it refers to cannot be borrowed as mutable
8+
|
9+
help: use a mutable iterator instead
10+
|
11+
LL | for test in &mut tests {
12+
| +++
13+
14+
error: aborting due to previous error
15+
16+
For more information about this error, try `rustc --explain E0596`.

0 commit comments

Comments
 (0)