Skip to content

Commit 17b2f92

Browse files
estebankestebank
committed
Tweak borrowing suggestion in for loop
1 parent eba3228 commit 17b2f92

File tree

5 files changed

+76
-44
lines changed

5 files changed

+76
-44
lines changed

‎compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
22
use rustc_middle::mir::*;
33
use rustc_middle::ty;
44
use rustc_span::source_map::DesugaringKind;
5-
use rustc_span::{sym, Span};
5+
use rustc_span::{sym, Span, DUMMY_SP};
6+
use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;
67

78
use crate::borrow_check::diagnostics::UseSpans;
89
use crate::borrow_check::prefixes::PrefixSet;
@@ -384,36 +385,44 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
384385
}
385386
}
386387
};
387-
if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) {
388-
let def_id = match *move_place.ty(self.body, self.infcx.tcx).ty.kind() {
389-
ty::Adt(self_def, _) => self_def.did,
390-
ty::Foreign(def_id)
391-
| ty::FnDef(def_id, _)
392-
| ty::Closure(def_id, _)
393-
| ty::Generator(def_id, ..)
394-
| ty::Opaque(def_id, _) => def_id,
395-
_ => return err,
388+
let ty = move_place.ty(self.body, self.infcx.tcx).ty;
389+
let def_id = match *ty.kind() {
390+
ty::Adt(self_def, _) => self_def.did,
391+
ty::Foreign(def_id)
392+
| ty::FnDef(def_id, _)
393+
| ty::Closure(def_id, _)
394+
| ty::Generator(def_id, ..)
395+
| ty::Opaque(def_id, _) => def_id,
396+
_ => return err,
397+
};
398+
let is_option = self.infcx.tcx.is_diagnostic_item(sym::option_type, def_id);
399+
let is_result = self.infcx.tcx.is_diagnostic_item(sym::result_type, def_id);
400+
if (is_option || is_result) && use_spans.map_or(true, |v| !v.for_closure()) {
401+
err.span_suggestion_verbose(
402+
span.shrink_to_hi(),
403+
&format!(
404+
"consider borrowing the `{}`'s content",
405+
if is_option { "Option" } else { "Result" }
406+
),
407+
".as_ref()".to_string(),
408+
Applicability::MaybeIncorrect,
409+
);
410+
} else if matches!(span.desugaring_kind(), Some(DesugaringKind::ForLoop(_))) {
411+
let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) {
412+
Some(def_id) => type_known_to_meet_bound_modulo_regions(
413+
&self.infcx,
414+
self.param_env,
415+
self.infcx.tcx.mk_imm_ref(self.infcx.tcx.lifetimes.re_erased, ty),
416+
def_id,
417+
DUMMY_SP,
418+
),
419+
_ => false,
396420
};
397-
let is_option = self.infcx.tcx.is_diagnostic_item(sym::option_type, def_id);
398-
let is_result = self.infcx.tcx.is_diagnostic_item(sym::result_type, def_id);
399-
if (is_option || is_result) && use_spans.map_or(true, |v| !v.for_closure()) {
400-
err.span_suggestion(
401-
span,
402-
&format!(
403-
"consider borrowing the `{}`'s content",
404-
if is_option { "Option" } else { "Result" }
405-
),
406-
format!("{}.as_ref()", snippet),
407-
Applicability::MaybeIncorrect,
408-
);
409-
} else if matches!(span.desugaring_kind(), Some(DesugaringKind::ForLoop(_)))
410-
&& self.infcx.tcx.is_diagnostic_item(sym::vec_type, def_id)
411-
{
412-
// FIXME: suggest for anything that implements `IntoIterator`.
413-
err.span_suggestion(
414-
span,
415-
"consider iterating over a slice of the `Vec<_>`'s content",
416-
format!("&{}", snippet),
421+
if suggest {
422+
err.span_suggestion_verbose(
423+
span.shrink_to_lo(),
424+
&format!("consider iterating over a slice of the `{}`'s content", ty),
425+
"&".to_string(),
417426
Applicability::MaybeIncorrect,
418427
);
419428
}

‎src/test/ui/suggestions/for-i-in-vec.fixed

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33

44
struct Foo {
55
v: Vec<u32>,
6+
h: std::collections::HashMap<i32, i32>,
67
}
78

89
impl Foo {
910
fn bar(&self) {
1011
for _ in &self.v { //~ ERROR cannot move out of `self.v` which is behind a shared reference
1112
}
13+
for _ in &self.h { //~ ERROR cannot move out of `self.h` which is behind a shared reference
14+
}
1215
}
1316
}
1417

‎src/test/ui/suggestions/for-i-in-vec.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33

44
struct Foo {
55
v: Vec<u32>,
6+
h: std::collections::HashMap<i32, i32>,
67
}
78

89
impl Foo {
910
fn bar(&self) {
1011
for _ in self.v { //~ ERROR cannot move out of `self.v` which is behind a shared reference
1112
}
13+
for _ in self.h { //~ ERROR cannot move out of `self.h` which is behind a shared reference
14+
}
1215
}
1316
}
1417

‎src/test/ui/suggestions/for-i-in-vec.stderr

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,25 @@
11
error[E0507]: cannot move out of `self.v` which is behind a shared reference
2-
--> $DIR/for-i-in-vec.rs:10:18
2+
--> $DIR/for-i-in-vec.rs:11:18
33
|
44
LL | for _ in self.v {
5-
| ^^^^^^
6-
| |
7-
| move occurs because `self.v` has type `Vec<u32>`, which does not implement the `Copy` trait
8-
| help: consider iterating over a slice of the `Vec<_>`'s content: `&self.v`
5+
| ^^^^^^ move occurs because `self.v` has type `Vec<u32>`, which does not implement the `Copy` trait
6+
|
7+
help: consider iterating over a slice of the `Vec<u32>`'s content
8+
|
9+
LL | for _ in &self.v {
10+
| ^
11+
12+
error[E0507]: cannot move out of `self.h` which is behind a shared reference
13+
--> $DIR/for-i-in-vec.rs:13:18
14+
|
15+
LL | for _ in self.h {
16+
| ^^^^^^ move occurs because `self.h` has type `HashMap<i32, i32>`, which does not implement the `Copy` trait
17+
|
18+
help: consider iterating over a slice of the `HashMap<i32, i32>`'s content
19+
|
20+
LL | for _ in &self.h {
21+
| ^
922

10-
error: aborting due to previous error
23+
error: aborting due to 2 previous errors
1124

1225
For more information about this error, try `rustc --explain E0507`.

‎src/test/ui/suggestions/option-content-move.stderr

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,23 @@ error[E0507]: cannot move out of `selection.1` which is behind a shared referenc
22
--> $DIR/option-content-move.rs:11:20
33
|
44
LL | if selection.1.unwrap().contains(selection.0) {
5-
| ^^^^^^^^^^^
6-
| |
7-
| move occurs because `selection.1` has type `Option<String>`, which does not implement the `Copy` trait
8-
| help: consider borrowing the `Option`'s content: `selection.1.as_ref()`
5+
| ^^^^^^^^^^^ move occurs because `selection.1` has type `Option<String>`, which does not implement the `Copy` trait
6+
|
7+
help: consider borrowing the `Option`'s content
8+
|
9+
LL | if selection.1.as_ref().unwrap().contains(selection.0) {
10+
| ^^^^^^^^^
911

1012
error[E0507]: cannot move out of `selection.1` which is behind a shared reference
1113
--> $DIR/option-content-move.rs:29:20
1214
|
1315
LL | if selection.1.unwrap().contains(selection.0) {
14-
| ^^^^^^^^^^^
15-
| |
16-
| move occurs because `selection.1` has type `Result<String, String>`, which does not implement the `Copy` trait
17-
| help: consider borrowing the `Result`'s content: `selection.1.as_ref()`
16+
| ^^^^^^^^^^^ move occurs because `selection.1` has type `Result<String, String>`, which does not implement the `Copy` trait
17+
|
18+
help: consider borrowing the `Result`'s content
19+
|
20+
LL | if selection.1.as_ref().unwrap().contains(selection.0) {
21+
| ^^^^^^^^^
1822

1923
error: aborting due to 2 previous errors
2024

0 commit comments

Comments
 (0)