From c775927d7fee06743631d138eac91a862c8f6faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 21 Jan 2020 11:11:00 -0800 Subject: [PATCH 1/2] Suggest borrowing `Vec` in for loop Partially address #64167. --- .../borrow_check/diagnostics/move_errors.rs | 11 +++++++++++ src/test/ui/suggestions/for-i-in-vec.fixed | 15 +++++++++++++++ src/test/ui/suggestions/for-i-in-vec.rs | 15 +++++++++++++++ src/test/ui/suggestions/for-i-in-vec.stderr | 12 ++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 src/test/ui/suggestions/for-i-in-vec.fixed create mode 100644 src/test/ui/suggestions/for-i-in-vec.rs create mode 100644 src/test/ui/suggestions/for-i-in-vec.stderr diff --git a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs index eb6db7c145c3c..c016cc90b1b86 100644 --- a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs @@ -1,6 +1,7 @@ use rustc::mir::*; use rustc::ty; use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_span::source_map::DesugaringKind; use rustc_span::Span; use crate::borrow_check::diagnostics::UseSpans; @@ -397,6 +398,16 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { format!("{}.as_ref()", snippet), Applicability::MaybeIncorrect, ); + } else if span.is_desugaring(DesugaringKind::ForLoop) + && move_ty.starts_with("std::vec::Vec") + { + // FIXME: suggest for anything that implements `IntoIterator`. + err.span_suggestion( + span, + "consider iterating over a slice of the `Vec`'s content", + format!("&{}", snippet), + Applicability::MaybeIncorrect, + ); } } err diff --git a/src/test/ui/suggestions/for-i-in-vec.fixed b/src/test/ui/suggestions/for-i-in-vec.fixed new file mode 100644 index 0000000000000..ec7358bd08ad2 --- /dev/null +++ b/src/test/ui/suggestions/for-i-in-vec.fixed @@ -0,0 +1,15 @@ +// run-rustfix +#![allow(dead_code)] + +struct Foo { + v: Vec, +} + +impl Foo { + fn bar(&self) { + for _ in &self.v { //~ ERROR cannot move out of `self.v` which is behind a shared reference + } + } +} + +fn main() {} diff --git a/src/test/ui/suggestions/for-i-in-vec.rs b/src/test/ui/suggestions/for-i-in-vec.rs new file mode 100644 index 0000000000000..304fe8cc81f1a --- /dev/null +++ b/src/test/ui/suggestions/for-i-in-vec.rs @@ -0,0 +1,15 @@ +// run-rustfix +#![allow(dead_code)] + +struct Foo { + v: Vec, +} + +impl Foo { + fn bar(&self) { + for _ in self.v { //~ ERROR cannot move out of `self.v` which is behind a shared reference + } + } +} + +fn main() {} diff --git a/src/test/ui/suggestions/for-i-in-vec.stderr b/src/test/ui/suggestions/for-i-in-vec.stderr new file mode 100644 index 0000000000000..0fd10489bd0df --- /dev/null +++ b/src/test/ui/suggestions/for-i-in-vec.stderr @@ -0,0 +1,12 @@ +error[E0507]: cannot move out of `self.v` which is behind a shared reference + --> $DIR/for-i-in-vec.rs:10:18 + | +LL | for _ in self.v { + | ^^^^^^ + | | + | move occurs because `self.v` has type `std::vec::Vec`, which does not implement the `Copy` trait + | help: consider iterating over a slice of the `Vec`'s content: `&self.v` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0507`. From 6eaf59dfc8be4ee5647f9c090c5a7668682f30c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 23 Jan 2020 11:51:56 -0800 Subject: [PATCH 2/2] use `diagnostic_item` and modify wording --- src/libcore/option.rs | 1 + src/libcore/result.rs | 1 + .../borrow_check/diagnostics/move_errors.rs | 22 ++++++++++++++----- src/test/ui/suggestions/for-i-in-vec.stderr | 2 +- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libcore/option.rs b/src/libcore/option.rs index a471b174534aa..cb4247d98745e 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -151,6 +151,7 @@ use crate::{ /// The `Option` type. See [the module level documentation](index.html) for more. #[derive(Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] +#[rustc_diagnostic_item = "option_type"] #[stable(feature = "rust1", since = "1.0.0")] pub enum Option { /// No value diff --git a/src/libcore/result.rs b/src/libcore/result.rs index c657ce33f60ad..bc70dbd62eb52 100644 --- a/src/libcore/result.rs +++ b/src/libcore/result.rs @@ -242,6 +242,7 @@ use crate::ops::{self, Deref, DerefMut}; /// [`Err`]: enum.Result.html#variant.Err #[derive(Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] #[must_use = "this `Result` may be an `Err` variant, which should be handled"] +#[rustc_diagnostic_item = "result_type"] #[stable(feature = "rust1", since = "1.0.0")] pub enum Result { /// Contains the success value diff --git a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs index c016cc90b1b86..43121b38da01a 100644 --- a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs @@ -2,7 +2,7 @@ use rustc::mir::*; use rustc::ty; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_span::source_map::DesugaringKind; -use rustc_span::Span; +use rustc_span::{Span, Symbol}; use crate::borrow_check::diagnostics::UseSpans; use crate::borrow_check::prefixes::PrefixSet; @@ -384,10 +384,20 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } }; - let move_ty = format!("{:?}", move_place.ty(*self.body, self.infcx.tcx).ty,); if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) { - let is_option = move_ty.starts_with("std::option::Option"); - let is_result = move_ty.starts_with("std::result::Result"); + 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 is_option = + self.infcx.tcx.is_diagnostic_item(Symbol::intern("option_type"), def_id); + let is_result = + self.infcx.tcx.is_diagnostic_item(Symbol::intern("result_type"), def_id); if (is_option || is_result) && use_spans.map_or(true, |v| !v.for_closure()) { err.span_suggestion( span, @@ -399,12 +409,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { Applicability::MaybeIncorrect, ); } else if span.is_desugaring(DesugaringKind::ForLoop) - && move_ty.starts_with("std::vec::Vec") + && self.infcx.tcx.is_diagnostic_item(Symbol::intern("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", + "consider iterating over a slice of the `Vec<_>`'s content", format!("&{}", snippet), Applicability::MaybeIncorrect, ); diff --git a/src/test/ui/suggestions/for-i-in-vec.stderr b/src/test/ui/suggestions/for-i-in-vec.stderr index 0fd10489bd0df..576a7cc2f6043 100644 --- a/src/test/ui/suggestions/for-i-in-vec.stderr +++ b/src/test/ui/suggestions/for-i-in-vec.stderr @@ -5,7 +5,7 @@ LL | for _ in self.v { | ^^^^^^ | | | move occurs because `self.v` has type `std::vec::Vec`, which does not implement the `Copy` trait - | help: consider iterating over a slice of the `Vec`'s content: `&self.v` + | help: consider iterating over a slice of the `Vec<_>`'s content: `&self.v` error: aborting due to previous error