Skip to content

Commit

Permalink
Auto merge of #53830 - davidtwco:issue-53228, r=nikomatsakis
Browse files Browse the repository at this point in the history
Add help message for missing IndexMut impl with NLL

Fixes #53228.

r? @nikomatsakis
  • Loading branch information
bors committed Sep 7, 2018
2 parents 2ae11a9 + 08a4a37 commit 7366752
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 12 deletions.
5 changes: 4 additions & 1 deletion src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
if let Some(&init_index) = first_init_index {
// And, if so, report an error.
let init = &self.move_data.inits[init_index];
self.report_illegal_reassignment(context, place_span, init.span, place_span.0);
let span = init.span(&self.mir);
self.report_illegal_reassignment(
context, place_span, span, place_span.0
);
}
}

Expand Down
67 changes: 63 additions & 4 deletions src/librustc_mir/borrow_check/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,20 @@

use rustc::hir;
use rustc::hir::Node;
use rustc::mir::{self, BindingForm, ClearCrossCrate, Local, Location, Mir};
use rustc::mir::{Mutability, Place, Projection, ProjectionElem, Static};
use rustc::ty::{self, TyCtxt};
use rustc::mir::{self, BindingForm, Constant, ClearCrossCrate, Local, Location, Mir};
use rustc::mir::{Mutability, Operand, Place, Projection, ProjectionElem, Static, Terminator};
use rustc::mir::TerminatorKind;
use rustc::ty::{self, Const, DefIdTree, TyS, TyKind, TyCtxt};
use rustc_data_structures::indexed_vec::Idx;
use syntax_pos::Span;

use dataflow::move_paths::InitLocation;
use borrow_check::MirBorrowckCtxt;
use util::borrowck_errors::{BorrowckErrors, Origin};
use util::collect_writes::FindAssignments;
use util::suggest_ref_mut;

#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub(super) enum AccessKind {
MutableBorrow,
Mutate,
Expand Down Expand Up @@ -393,6 +395,63 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
);
}

Place::Projection(box Projection {
base: Place::Local(local),
elem: ProjectionElem::Deref,
}) if error_access == AccessKind::MutableBorrow => {
err.span_label(span, format!("cannot {ACT}", ACT = act));

let mpi = self.move_data.rev_lookup.find_local(*local);
for i in self.move_data.init_path_map[mpi].iter() {
if let InitLocation::Statement(location) = self.move_data.inits[*i].location {
if let Some(
Terminator {
kind: TerminatorKind::Call {
func: Operand::Constant(box Constant {
literal: Const {
ty: &TyS {
sty: TyKind::FnDef(id, substs),
..
},
..
},
..
}),
..
},
..
}
) = &self.mir.basic_blocks()[location.block].terminator {
if self.tcx.parent(id) == self.tcx.lang_items().index_trait() {

let mut found = false;
self.tcx.for_each_relevant_impl(
self.tcx.lang_items().index_mut_trait().unwrap(),
substs.type_at(0),
|_relevant_impl| {
found = true;
}
);

let extra = if found {
String::from("")
} else {
format!(", but it is not implemented for `{}`",
substs.type_at(0))
};

err.help(
&format!(
"trait `IndexMut` is required to modify indexed content{}",
extra,
),
);
}
}
}
}
}

_ => {
err.span_label(span, format!("cannot {ACT}", ACT = act));
}
Expand Down
7 changes: 3 additions & 4 deletions src/librustc_mir/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::mem;
use super::abs_domain::Lift;

use super::{LocationMap, MoveData, MovePath, MovePathLookup, MovePathIndex, MoveOut, MoveOutIndex};
use super::{MoveError, InitIndex, Init, LookupResult, InitKind};
use super::{MoveError, InitIndex, Init, InitLocation, LookupResult, InitKind};
use super::IllegalMoveOriginKind::*;

struct MoveDataBuilder<'a, 'gcx: 'tcx, 'tcx: 'a> {
Expand Down Expand Up @@ -237,10 +237,9 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
fn gather_args(&mut self) {
for arg in self.mir.args_iter() {
let path = self.data.rev_lookup.locals[arg];
let span = self.mir.local_decls[arg].source_info.span;

let init = self.data.inits.push(Init {
path, span, kind: InitKind::Deep
path, kind: InitKind::Deep, location: InitLocation::Argument(arg),
});

debug!("gather_args: adding init {:?} of {:?} for argument {:?}",
Expand Down Expand Up @@ -428,7 +427,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {

if let LookupResult::Exact(path) = self.builder.data.rev_lookup.find(place) {
let init = self.builder.data.inits.push(Init {
span: self.builder.mir.source_info(self.loc).span,
location: InitLocation::Statement(self.loc),
path,
kind,
});
Expand Down
24 changes: 21 additions & 3 deletions src/librustc_mir/dataflow/move_paths/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,21 @@ impl fmt::Debug for MoveOut {
pub struct Init {
/// path being initialized
pub path: MovePathIndex,
/// span of initialization
pub span: Span,
/// location of initialization
pub location: InitLocation,
/// Extra information about this initialization
pub kind: InitKind,
}


/// Initializations can be from an argument or from a statement. Arguments
/// do not have locations, in those cases the `Local` is kept..
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum InitLocation {
Argument(Local),
Statement(Location),
}

/// Additional information about the initialization.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum InitKind {
Expand All @@ -215,7 +224,16 @@ pub enum InitKind {

impl fmt::Debug for Init {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "{:?}@{:?} ({:?})", self.path, self.span, self.kind)
write!(fmt, "{:?}@{:?} ({:?})", self.path, self.location, self.kind)
}
}

impl Init {
crate fn span<'gcx>(&self, mir: &Mir<'gcx>) -> Span {
match self.location {
InitLocation::Argument(local) => mir.local_decls[local].source_info.span,
InitLocation::Statement(location) => mir.source_info(location).span,
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/borrowck/index-mut-help-with-impl.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0596]: cannot borrow data in a `&` reference as mutable
--> $DIR/index-mut-help-with-impl.rs:19:5
|
LL | Index::index(&v, 1..2).make_ascii_uppercase(); //~ ERROR
| ^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content

error: aborting due to previous error

For more information about this error, try `rustc --explain E0596`.
20 changes: 20 additions & 0 deletions src/test/ui/borrowck/index-mut-help-with-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// When mutably indexing a type that implements `Index` and `IndexMut` but
// `Index::index` is being used specifically, the normal special help message
// should not mention a missing `IndexMut` impl.

fn main() {
use std::ops::Index;

let v = String::from("dinosaur");
Index::index(&v, 1..2).make_ascii_uppercase(); //~ ERROR
}
9 changes: 9 additions & 0 deletions src/test/ui/borrowck/index-mut-help-with-impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0596]: cannot borrow immutable borrowed content as mutable
--> $DIR/index-mut-help-with-impl.rs:19:5
|
LL | Index::index(&v, 1..2).make_ascii_uppercase(); //~ ERROR
| ^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0596`.
4 changes: 4 additions & 0 deletions src/test/ui/borrowck/index-mut-help.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0596]: cannot borrow data in a `&` reference as mutable
|
LL | map["peter"].clear(); //~ ERROR
| ^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<&str, std::string::String>`

error[E0594]: cannot assign to data in a `&` reference
--> $DIR/index-mut-help.rs:22:5
Expand All @@ -15,6 +17,8 @@ error[E0596]: cannot borrow data in a `&` reference as mutable
|
LL | let _ = &mut map["peter"]; //~ ERROR
| ^^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<&str, std::string::String>`

error: aborting due to 3 previous errors

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/issues/issue-41726.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0596]: cannot borrow data in a `&` reference as mutable
|
LL | things[src.as_str()].sort(); //~ ERROR cannot borrow immutable
| ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<std::string::String, std::vec::Vec<std::string::String>>`

error: aborting due to previous error

Expand Down

0 comments on commit 7366752

Please sign in to comment.