From ef6851cdb285d18e54b9770b455b595565572462 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 30 Aug 2018 18:14:54 +0200 Subject: [PATCH 1/3] Added InitLocation to encode Location or Local depending on source of Init --- src/librustc_mir/borrow_check/mod.rs | 4 +++- .../borrow_check/mutability_errors.rs | 1 + .../dataflow/move_paths/builder.rs | 7 +++--- src/librustc_mir/dataflow/move_paths/mod.rs | 24 ++++++++++++++++--- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 16d34082642bd..b06a1e0e19f6a 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1601,7 +1601,9 @@ 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); + self.report_illegal_reassignment( + context, place_span, init.span(&self.mir), place_span.0 + ); } } diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index f96ef909c0df8..645cce8dd793b 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -16,6 +16,7 @@ use rustc::ty::{self, 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; diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 2148363c79712..0162bb84b94b6 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -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> { @@ -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 {:?}", @@ -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, }); diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 7b4cbdf7131b0..8728afc228fcc 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -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 { @@ -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, + } } } From 1ad2f5cec45869311f3a51731f77bbed009a66e9 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 30 Aug 2018 19:46:27 +0200 Subject: [PATCH 2/3] Added help message for missing IndexMut impl. --- src/librustc_mir/borrow_check/mod.rs | 3 +- .../borrow_check/mutability_errors.rs | 50 +++++++++++++++++-- .../ui/borrowck/index-mut-help.nll.stderr | 4 ++ src/test/ui/issues/issue-41726.nll.stderr | 2 + 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index b06a1e0e19f6a..a04d202efcfb0 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1601,8 +1601,9 @@ 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]; + let span = init.span(&self.mir); self.report_illegal_reassignment( - context, place_span, init.span(&self.mir), place_span.0 + context, place_span, span, place_span.0 ); } } diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 645cce8dd793b..38cdd17a7273c 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -10,9 +10,10 @@ 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; @@ -22,7 +23,7 @@ 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, @@ -394,6 +395,47 @@ 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() { + err.help( + &format!( + "trait `IndexMut` is required to modify indexed content, \ + but it is not implemented for `{}`", + substs.type_at(0), + ), + ); + } + } + } + } + } + _ => { err.span_label(span, format!("cannot {ACT}", ACT = act)); } diff --git a/src/test/ui/borrowck/index-mut-help.nll.stderr b/src/test/ui/borrowck/index-mut-help.nll.stderr index cc058f1fde5da..a15f37684e738 100644 --- a/src/test/ui/borrowck/index-mut-help.nll.stderr +++ b/src/test/ui/borrowck/index-mut-help.nll.stderr @@ -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 @@ -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 diff --git a/src/test/ui/issues/issue-41726.nll.stderr b/src/test/ui/issues/issue-41726.nll.stderr index 06ff743a0f5ed..9f727881beaf2 100644 --- a/src/test/ui/issues/issue-41726.nll.stderr +++ b/src/test/ui/issues/issue-41726.nll.stderr @@ -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>` error: aborting due to previous error From 08a4a376782f31b73f21646a5fcde5c52b6c8c37 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 31 Aug 2018 00:54:04 +0200 Subject: [PATCH 3/3] Omit 'missing IndexMut impl' suggestion when IndexMut is implemented. --- .../borrow_check/mutability_errors.rs | 22 ++++++++++++++++--- .../index-mut-help-with-impl.nll.stderr | 11 ++++++++++ .../ui/borrowck/index-mut-help-with-impl.rs | 20 +++++++++++++++++ .../borrowck/index-mut-help-with-impl.stderr | 9 ++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/borrowck/index-mut-help-with-impl.nll.stderr create mode 100644 src/test/ui/borrowck/index-mut-help-with-impl.rs create mode 100644 src/test/ui/borrowck/index-mut-help-with-impl.stderr diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 38cdd17a7273c..78ab772d9ad5b 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -423,11 +423,27 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { } ) = &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, \ - but it is not implemented for `{}`", - substs.type_at(0), + "trait `IndexMut` is required to modify indexed content{}", + extra, ), ); } diff --git a/src/test/ui/borrowck/index-mut-help-with-impl.nll.stderr b/src/test/ui/borrowck/index-mut-help-with-impl.nll.stderr new file mode 100644 index 0000000000000..8e7ffa67b061c --- /dev/null +++ b/src/test/ui/borrowck/index-mut-help-with-impl.nll.stderr @@ -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`. diff --git a/src/test/ui/borrowck/index-mut-help-with-impl.rs b/src/test/ui/borrowck/index-mut-help-with-impl.rs new file mode 100644 index 0000000000000..a5bab48c5cc58 --- /dev/null +++ b/src/test/ui/borrowck/index-mut-help-with-impl.rs @@ -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 or the MIT license +// , 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 +} diff --git a/src/test/ui/borrowck/index-mut-help-with-impl.stderr b/src/test/ui/borrowck/index-mut-help-with-impl.stderr new file mode 100644 index 0000000000000..9c28b86c45a92 --- /dev/null +++ b/src/test/ui/borrowck/index-mut-help-with-impl.stderr @@ -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`.