From 1e5d81de1d8485e9ce2995bc6b1559f25c4d86e5 Mon Sep 17 00:00:00 2001
From: Wonwoo Choi <chwo9843@gmail.com>
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<Span>,
opt_ty_info: Option<Span>,
) -> (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`.