From 1e5d81de1d8485e9ce2995bc6b1559f25c4d86e5 Mon Sep 17 00:00:00 2001 From: Wonwoo Choi Date: Sun, 22 Mar 2020 18:02:18 +0900 Subject: [PATCH] Fix invalid suggestion on `&mut` iterators yielding `&` references --- .../diagnostics/mutability_errors.rs | 82 +++++++++++++------ .../issue-69789-iterator-mut-suggestion.rs | 11 +++ ...issue-69789-iterator-mut-suggestion.stderr | 12 +++ 3 files changed, 82 insertions(+), 23 deletions(-) create mode 100644 src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.rs create mode 100644 src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.stderr diff --git a/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs b/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs index c462f93414874..ee654431d8892 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs @@ -1,9 +1,10 @@ -use rustc::mir::{self, ClearCrossCrate, Local, LocalInfo, Location, ReadOnlyBodyAndCache}; +use rustc::mir::{self, ClearCrossCrate, Local, LocalInfo, Location}; use rustc::mir::{Mutability, Place, PlaceRef, ProjectionElem}; use rustc::ty::{self, Ty, TyCtxt}; use rustc_hir as hir; use rustc_hir::Node; use rustc_index::vec::Idx; +use rustc_span::source_map::DesugaringKind; use rustc_span::symbol::kw; use rustc_span::Span; @@ -338,10 +339,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { match self.local_names[local] { Some(name) if !local_decl.from_compiler_desugaring() => { - let suggestion = match local_decl.local_info { + let label = match local_decl.local_info { LocalInfo::User(ClearCrossCrate::Set( mir::BindingForm::ImplicitSelf(_), - )) => Some(suggest_ampmut_self(self.infcx.tcx, local_decl)), + )) => { + let (span, suggestion) = + suggest_ampmut_self(self.infcx.tcx, local_decl); + Some((true, span, suggestion)) + } LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var( mir::VarBindingForm { @@ -349,13 +354,38 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { opt_ty_info, .. }, - ))) => Some(suggest_ampmut( - self.infcx.tcx, - self.body, - local, - local_decl, - opt_ty_info, - )), + ))) => { + // check if the RHS is from desugaring + let locations = self.body.find_assignments(local); + let opt_assignment_rhs_span = locations + .first() + .map(|&location| self.body.source_info(location).span); + let opt_desugaring_kind = + opt_assignment_rhs_span.and_then(|span| span.desugaring_kind()); + match opt_desugaring_kind { + // on for loops, RHS points to the iterator part + Some(DesugaringKind::ForLoop) => Some(( + false, + opt_assignment_rhs_span.unwrap(), + format!( + "this iterator yields `{SIGIL}` {DESC}s", + SIGIL = pointer_sigil, + DESC = pointer_desc + ), + )), + // don't create labels for compiler-generated spans + Some(_) => None, + None => { + let (span, suggestion) = suggest_ampmut( + self.infcx.tcx, + local_decl, + opt_assignment_rhs_span, + opt_ty_info, + ); + Some((true, span, suggestion)) + } + } + } LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var( mir::VarBindingForm { @@ -365,7 +395,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ))) => { let pattern_span = local_decl.source_info.span; suggest_ref_mut(self.infcx.tcx, pattern_span) - .map(|replacement| (pattern_span, replacement)) + .map(|replacement| (true, pattern_span, replacement)) } LocalInfo::User(ClearCrossCrate::Clear) => { @@ -375,13 +405,22 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { _ => unreachable!(), }; - if let Some((err_help_span, suggested_code)) = suggestion { - err.span_suggestion( - err_help_span, - &format!("consider changing this to be a mutable {}", pointer_desc), - suggested_code, - Applicability::MachineApplicable, - ); + match label { + Some((true, err_help_span, suggested_code)) => { + err.span_suggestion( + err_help_span, + &format!( + "consider changing this to be a mutable {}", + pointer_desc + ), + suggested_code, + Applicability::MachineApplicable, + ); + } + Some((false, err_label_span, message)) => { + err.span_label(err_label_span, &message); + } + None => {} } err.span_label( span, @@ -581,14 +620,11 @@ fn suggest_ampmut_self<'tcx>( // by trying (3.), then (2.) and finally falling back on (1.). fn suggest_ampmut<'tcx>( tcx: TyCtxt<'tcx>, - body: ReadOnlyBodyAndCache<'_, 'tcx>, - local: Local, local_decl: &mir::LocalDecl<'tcx>, + opt_assignment_rhs_span: Option, opt_ty_info: Option, ) -> (Span, String) { - let locations = body.find_assignments(local); - if !locations.is_empty() { - let assignment_rhs_span = body.source_info(locations[0]).span; + if let Some(assignment_rhs_span) = opt_assignment_rhs_span { if let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span) { if let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(|c: char| -> bool { c.is_whitespace() })) diff --git a/src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.rs b/src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.rs new file mode 100644 index 0000000000000..f6d0e9e04d321 --- /dev/null +++ b/src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.rs @@ -0,0 +1,11 @@ +// Regression test for #69789: rustc generated an invalid suggestion +// when `&` reference from `&mut` iterator is mutated. + +fn main() { + for item in &mut std::iter::empty::<&'static ()>() { + //~^ NOTE this iterator yields `&` references + *item = (); + //~^ ERROR cannot assign + //~| NOTE cannot be written + } +} diff --git a/src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.stderr b/src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.stderr new file mode 100644 index 0000000000000..d2865ffd196a5 --- /dev/null +++ b/src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.stderr @@ -0,0 +1,12 @@ +error[E0594]: cannot assign to `*item` which is behind a `&` reference + --> $DIR/issue-69789-iterator-mut-suggestion.rs:7:9 + | +LL | for item in &mut std::iter::empty::<&'static ()>() { + | -------------------------------------- this iterator yields `&` references +LL | +LL | *item = (); + | ^^^^^^^^^^ `item` is a `&` reference, so the data it refers to cannot be written + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0594`.