Skip to content

Commit

Permalink
Rollup merge of rust-lang#70264 - tirr-c:issue-69789-mut-suggestion, …
Browse files Browse the repository at this point in the history
…r=estebank

Fix invalid suggestion on `&mut` iterators yielding `&` references

Fixes rust-lang#69789.

rustc suggested an invalid code when `&` reference from `&mut` iterator is mutated. The compiler knew we're mutating a value behind `&` reference, but as the assignment RHS is from desugaring, it could only see the iterator expression from source and inserted `mut` there.

r? @estebank
  • Loading branch information
Centril committed Mar 23, 2020
2 parents c8fb14e + 1e5d81d commit a897944
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 23 deletions.
82 changes: 59 additions & 23 deletions src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -338,24 +339,53 @@ 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 {
binding_mode: ty::BindingMode::BindByValue(_),
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 {
Expand All @@ -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) => {
Expand All @@ -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,
Expand Down Expand Up @@ -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() }))
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
12 changes: 12 additions & 0 deletions src/test/ui/borrowck/issue-69789-iterator-mut-suggestion.stderr
Original file line number Diff line number Diff line change
@@ -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`.

0 comments on commit a897944

Please sign in to comment.