From 17b2f92e44b65caa5e057c51560336311caf2fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 28 Jul 2021 10:08:39 -0700 Subject: [PATCH] Tweak borrowing suggestion in `for` loop --- .../borrow_check/diagnostics/move_errors.rs | 69 +++++++++++-------- src/test/ui/suggestions/for-i-in-vec.fixed | 3 + src/test/ui/suggestions/for-i-in-vec.rs | 3 + src/test/ui/suggestions/for-i-in-vec.stderr | 25 +++++-- .../ui/suggestions/option-content-move.stderr | 20 +++--- 5 files changed, 76 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs index 3f87d9c7ac948..2be23159bf563 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs @@ -2,7 +2,8 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_middle::mir::*; use rustc_middle::ty; use rustc_span::source_map::DesugaringKind; -use rustc_span::{sym, Span}; +use rustc_span::{sym, Span, DUMMY_SP}; +use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions; use crate::borrow_check::diagnostics::UseSpans; use crate::borrow_check::prefixes::PrefixSet; @@ -384,36 +385,44 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } }; - if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) { - let def_id = match *move_place.ty(self.body, self.infcx.tcx).ty.kind() { - ty::Adt(self_def, _) => self_def.did, - ty::Foreign(def_id) - | ty::FnDef(def_id, _) - | ty::Closure(def_id, _) - | ty::Generator(def_id, ..) - | ty::Opaque(def_id, _) => def_id, - _ => return err, + let ty = move_place.ty(self.body, self.infcx.tcx).ty; + let def_id = match *ty.kind() { + ty::Adt(self_def, _) => self_def.did, + ty::Foreign(def_id) + | ty::FnDef(def_id, _) + | ty::Closure(def_id, _) + | ty::Generator(def_id, ..) + | ty::Opaque(def_id, _) => def_id, + _ => return err, + }; + let is_option = self.infcx.tcx.is_diagnostic_item(sym::option_type, def_id); + let is_result = self.infcx.tcx.is_diagnostic_item(sym::result_type, def_id); + if (is_option || is_result) && use_spans.map_or(true, |v| !v.for_closure()) { + err.span_suggestion_verbose( + span.shrink_to_hi(), + &format!( + "consider borrowing the `{}`'s content", + if is_option { "Option" } else { "Result" } + ), + ".as_ref()".to_string(), + Applicability::MaybeIncorrect, + ); + } else if matches!(span.desugaring_kind(), Some(DesugaringKind::ForLoop(_))) { + let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) { + Some(def_id) => type_known_to_meet_bound_modulo_regions( + &self.infcx, + self.param_env, + self.infcx.tcx.mk_imm_ref(self.infcx.tcx.lifetimes.re_erased, ty), + def_id, + DUMMY_SP, + ), + _ => false, }; - let is_option = self.infcx.tcx.is_diagnostic_item(sym::option_type, def_id); - let is_result = self.infcx.tcx.is_diagnostic_item(sym::result_type, def_id); - if (is_option || is_result) && use_spans.map_or(true, |v| !v.for_closure()) { - err.span_suggestion( - span, - &format!( - "consider borrowing the `{}`'s content", - if is_option { "Option" } else { "Result" } - ), - format!("{}.as_ref()", snippet), - Applicability::MaybeIncorrect, - ); - } else if matches!(span.desugaring_kind(), Some(DesugaringKind::ForLoop(_))) - && self.infcx.tcx.is_diagnostic_item(sym::vec_type, def_id) - { - // FIXME: suggest for anything that implements `IntoIterator`. - err.span_suggestion( - span, - "consider iterating over a slice of the `Vec<_>`'s content", - format!("&{}", snippet), + if suggest { + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!("consider iterating over a slice of the `{}`'s content", ty), + "&".to_string(), Applicability::MaybeIncorrect, ); } diff --git a/src/test/ui/suggestions/for-i-in-vec.fixed b/src/test/ui/suggestions/for-i-in-vec.fixed index ec7358bd08ad2..223ddf0f0ad2a 100644 --- a/src/test/ui/suggestions/for-i-in-vec.fixed +++ b/src/test/ui/suggestions/for-i-in-vec.fixed @@ -3,12 +3,15 @@ struct Foo { v: Vec, + h: std::collections::HashMap, } impl Foo { fn bar(&self) { for _ in &self.v { //~ ERROR cannot move out of `self.v` which is behind a shared reference } + for _ in &self.h { //~ ERROR cannot move out of `self.h` which is behind a shared reference + } } } diff --git a/src/test/ui/suggestions/for-i-in-vec.rs b/src/test/ui/suggestions/for-i-in-vec.rs index 304fe8cc81f1a..7942698cc8eff 100644 --- a/src/test/ui/suggestions/for-i-in-vec.rs +++ b/src/test/ui/suggestions/for-i-in-vec.rs @@ -3,12 +3,15 @@ struct Foo { v: Vec, + h: std::collections::HashMap, } impl Foo { fn bar(&self) { for _ in self.v { //~ ERROR cannot move out of `self.v` which is behind a shared reference } + for _ in self.h { //~ ERROR cannot move out of `self.h` which is behind a shared reference + } } } diff --git a/src/test/ui/suggestions/for-i-in-vec.stderr b/src/test/ui/suggestions/for-i-in-vec.stderr index 48f3f423ac638..011fdf34c28b5 100644 --- a/src/test/ui/suggestions/for-i-in-vec.stderr +++ b/src/test/ui/suggestions/for-i-in-vec.stderr @@ -1,12 +1,25 @@ error[E0507]: cannot move out of `self.v` which is behind a shared reference - --> $DIR/for-i-in-vec.rs:10:18 + --> $DIR/for-i-in-vec.rs:11:18 | LL | for _ in self.v { - | ^^^^^^ - | | - | move occurs because `self.v` has type `Vec`, which does not implement the `Copy` trait - | help: consider iterating over a slice of the `Vec<_>`'s content: `&self.v` + | ^^^^^^ move occurs because `self.v` has type `Vec`, which does not implement the `Copy` trait + | +help: consider iterating over a slice of the `Vec`'s content + | +LL | for _ in &self.v { + | ^ + +error[E0507]: cannot move out of `self.h` which is behind a shared reference + --> $DIR/for-i-in-vec.rs:13:18 + | +LL | for _ in self.h { + | ^^^^^^ move occurs because `self.h` has type `HashMap`, which does not implement the `Copy` trait + | +help: consider iterating over a slice of the `HashMap`'s content + | +LL | for _ in &self.h { + | ^ -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0507`. diff --git a/src/test/ui/suggestions/option-content-move.stderr b/src/test/ui/suggestions/option-content-move.stderr index c00a0f1700bb4..9476653009155 100644 --- a/src/test/ui/suggestions/option-content-move.stderr +++ b/src/test/ui/suggestions/option-content-move.stderr @@ -2,19 +2,23 @@ error[E0507]: cannot move out of `selection.1` which is behind a shared referenc --> $DIR/option-content-move.rs:11:20 | LL | if selection.1.unwrap().contains(selection.0) { - | ^^^^^^^^^^^ - | | - | move occurs because `selection.1` has type `Option`, which does not implement the `Copy` trait - | help: consider borrowing the `Option`'s content: `selection.1.as_ref()` + | ^^^^^^^^^^^ move occurs because `selection.1` has type `Option`, which does not implement the `Copy` trait + | +help: consider borrowing the `Option`'s content + | +LL | if selection.1.as_ref().unwrap().contains(selection.0) { + | ^^^^^^^^^ error[E0507]: cannot move out of `selection.1` which is behind a shared reference --> $DIR/option-content-move.rs:29:20 | LL | if selection.1.unwrap().contains(selection.0) { - | ^^^^^^^^^^^ - | | - | move occurs because `selection.1` has type `Result`, which does not implement the `Copy` trait - | help: consider borrowing the `Result`'s content: `selection.1.as_ref()` + | ^^^^^^^^^^^ move occurs because `selection.1` has type `Result`, which does not implement the `Copy` trait + | +help: consider borrowing the `Result`'s content + | +LL | if selection.1.as_ref().unwrap().contains(selection.0) { + | ^^^^^^^^^ error: aborting due to 2 previous errors