From 7d590b533ea93ef916cc82498506a373db1843b6 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Wed, 14 Feb 2018 08:53:10 +0530 Subject: [PATCH 01/23] changes for new compile-fail/E0389 message --- src/librustc_mir/borrow_check/mod.rs | 59 +++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index e51b16a373618..e5e8ffdc2f388 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1551,12 +1551,46 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let item_msg = match self.describe_place(place) { Some(name) => format!("immutable item `{}`", name), None => "immutable item".to_owned(), }; + // let item_msg = match place{ + // Place::Projection(ref proj) => { + // let Projection { ref base, ref elem } = **proj; + // match *elem { + // ProjectionElem::Deref => { + // if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { + // debug!("place_err = {:?} and base={:?}", place_err, base); + // format!("`&`-reference {:?}", place_err) + + + // }else{ + // match self.describe_place(place) { + // Some(name) => format!("immutable item `{}`", name), + // None => "immutable item".to_owned(), + // } + // } + // } + // _ => { + // match self.describe_place(place) { + // Some(name) => format!("immutable item `{}`", name), + // None => "immutable item".to_owned(), + // } + + // } + // } + // }, + + // _=> { + // match self.describe_place(place) { + // Some(name) => format!("immutable item `{}`", name), + // None => "immutable item".to_owned(), + // } + // } + // }; + let mut err = self.tcx .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir); err.span_label(span, "cannot borrow as mutable"); @@ -1573,20 +1607,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let item_msg = match self.describe_place(place) { - Some(name) => format!("immutable item `{}`", name), - None => "immutable item".to_owned(), + let item_msg = if error_reported{ + if let Some(name) = self.describe_place(place_err) { + format!("`&`-reference {}", name) + }else{ + match self.describe_place(place) { + Some(name) => {format!("immutable item `{}`", name)} + None => {"immutable item".to_owned()} + } + } + }else{ + match self.describe_place(place) { + Some(name) => {format!("immutable item `{}`", name)} + None => {"immutable item".to_owned()} + } }; let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir); - err.span_label(span, "cannot mutate"); if place != place_err { - if let Some(name) = self.describe_place(place_err) { - err.note(&format!("Value not mutable causing this error: `{}`", name)); - } + err.span_label(span, "cannot assign through `&`-reference"); } - err.emit(); } } From 308e30eb7a3ec98392b0cbb0ebad354318a86148 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Wed, 28 Feb 2018 00:27:29 +0530 Subject: [PATCH 02/23] adding Visitor changes --- src/librustc_mir/borrow_check/mod.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index e5e8ffdc2f388..207097a71c37d 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -17,7 +17,7 @@ use rustc::hir::map::definitions::DefPathData; use rustc::infer::InferCtxt; use rustc::ty::{self, ParamEnv, TyCtxt}; use rustc::ty::maps::Providers; -use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Location, Place}; +use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Local, Location, Place, Visitor}; use rustc::mir::{Mir, Mutability, Operand, Projection, ProjectionElem, Rvalue}; use rustc::mir::{Field, Statement, StatementKind, Terminator, TerminatorKind}; use rustc::mir::ClosureRegionRequirements; @@ -56,6 +56,20 @@ mod prefixes; use std::borrow::Cow; +struct FindLocalAssignmentVisitor { + from: Local, + loc: Vec, +} + +impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { + fn visit_local(&mut self, + local: &mut Local, + _: PlaceContext<'tcx>, + _: Location) { + Visitor::visit_local(local,) + } +} + pub(crate) mod nll; pub fn provide(providers: &mut Providers) { @@ -2271,3 +2285,11 @@ impl ContextKind { } } } + +impl Mir { + fn find_assignments(&self, local: Local) -> Vec + { + + } +} + From f60788b9b2267eb9b0de742939207730f0b9b3e1 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 1 Mar 2018 19:14:44 +0530 Subject: [PATCH 03/23] update visit_local for FindLocalAssignmentVisitor --- src/librustc_mir/borrow_check/mod.rs | 73 +++++++++++----------------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 207097a71c37d..aa4a704254e5b 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -57,16 +57,33 @@ mod prefixes; use std::borrow::Cow; struct FindLocalAssignmentVisitor { - from: Local, - loc: Vec, + needle: Local, + locations: Vec, + placectxt: PlaceContext, + location: Location, } impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { fn visit_local(&mut self, - local: &mut Local, - _: PlaceContext<'tcx>, - _: Location) { - Visitor::visit_local(local,) + local: &Local, + place_context: PlaceContext<'tcx>, + location: Location) { + if self.needle != *local { + return; + } + + match place_context { + PlaceContext::Store | PlaceContext::Call => { + self.locations.push(location); + } + PlaceContext::AsmOutput | PlaceContext::Drop| PlaceContext::Inspect | + PlaceContext::Borrow| PlaceContext::Projection| PlaceContext::Copy| + PlaceContext::Move| PlaceContext::StorageLive| PlaceContext::StorageDead| + PlaceContext::Validate => { + } + } + + Visitor::visit_local(local,place_context,location) } } @@ -1570,41 +1587,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { None => "immutable item".to_owned(), }; - // let item_msg = match place{ - // Place::Projection(ref proj) => { - // let Projection { ref base, ref elem } = **proj; - // match *elem { - // ProjectionElem::Deref => { - // if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { - // debug!("place_err = {:?} and base={:?}", place_err, base); - // format!("`&`-reference {:?}", place_err) - - - // }else{ - // match self.describe_place(place) { - // Some(name) => format!("immutable item `{}`", name), - // None => "immutable item".to_owned(), - // } - // } - // } - // _ => { - // match self.describe_place(place) { - // Some(name) => format!("immutable item `{}`", name), - // None => "immutable item".to_owned(), - // } - - // } - // } - // }, - - // _=> { - // match self.describe_place(place) { - // Some(name) => format!("immutable item `{}`", name), - // None => "immutable item".to_owned(), - // } - // } - // }; - + // call find_assignments() here let mut err = self.tcx .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir); err.span_label(span, "cannot borrow as mutable"); @@ -2286,10 +2269,12 @@ impl ContextKind { } } -impl Mir { - fn find_assignments(&self, local: Local) -> Vec +impl Mir { + fn find_assignments(&self, local: Local, place_context:PlaceContext, location:Location) -> Vec { - + let mut visitor = FindLocalAssignmentVisitor { needle: local, locations: vec![], location:location, place_context: }; + visitor.visit_mir(self); + visitor.locations } } From 3f0ce0858e4a07239ea6d38c14991bba46c413fc Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Fri, 2 Mar 2018 10:46:01 +0530 Subject: [PATCH 04/23] minor refactorings to fix trait import issue --- src/librustc_mir/borrow_check/mod.rs | 52 ++++++---------------------- src/librustc_mir/util/mod.rs | 1 + 2 files changed, 11 insertions(+), 42 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index aa4a704254e5b..90e84a4780957 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -17,7 +17,7 @@ use rustc::hir::map::definitions::DefPathData; use rustc::infer::InferCtxt; use rustc::ty::{self, ParamEnv, TyCtxt}; use rustc::ty::maps::Providers; -use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Local, Location, Place, Visitor}; +use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Local, Location, Place}; use rustc::mir::{Mir, Mutability, Operand, Projection, ProjectionElem, Rvalue}; use rustc::mir::{Field, Statement, StatementKind, Terminator, TerminatorKind}; use rustc::mir::ClosureRegionRequirements; @@ -43,6 +43,7 @@ use dataflow::indexes::BorrowIndex; use dataflow::move_paths::{IllegalMoveOriginKind, MoveError}; use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex}; use util::borrowck_errors::{BorrowckErrors, Origin}; +use util::collect_writes::FindAssignments; use std::iter; @@ -56,37 +57,6 @@ mod prefixes; use std::borrow::Cow; -struct FindLocalAssignmentVisitor { - needle: Local, - locations: Vec, - placectxt: PlaceContext, - location: Location, -} - -impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { - fn visit_local(&mut self, - local: &Local, - place_context: PlaceContext<'tcx>, - location: Location) { - if self.needle != *local { - return; - } - - match place_context { - PlaceContext::Store | PlaceContext::Call => { - self.locations.push(location); - } - PlaceContext::AsmOutput | PlaceContext::Drop| PlaceContext::Inspect | - PlaceContext::Borrow| PlaceContext::Projection| PlaceContext::Copy| - PlaceContext::Move| PlaceContext::StorageLive| PlaceContext::StorageDead| - PlaceContext::Validate => { - } - } - - Visitor::visit_local(local,place_context,location) - } -} - pub(crate) mod nll; pub fn provide(providers: &mut Providers) { @@ -1587,7 +1557,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { None => "immutable item".to_owned(), }; - // call find_assignments() here let mut err = self.tcx .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir); err.span_label(span, "cannot borrow as mutable"); @@ -1604,6 +1573,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; + match *place{ + Place::Local(local) => {let locations = self.mir.find_assignments(local); + + for n in &locations{ + debug!("locations ={:?}", n);} + } + _ => {}} + let item_msg = if error_reported{ if let Some(name) = self.describe_place(place_err) { format!("`&`-reference {}", name) @@ -2269,12 +2246,3 @@ impl ContextKind { } } -impl Mir { - fn find_assignments(&self, local: Local, place_context:PlaceContext, location:Location) -> Vec - { - let mut visitor = FindLocalAssignmentVisitor { needle: local, locations: vec![], location:location, place_context: }; - visitor.visit_mir(self); - visitor.locations - } -} - diff --git a/src/librustc_mir/util/mod.rs b/src/librustc_mir/util/mod.rs index eebe5a86018ea..19cd376688627 100644 --- a/src/librustc_mir/util/mod.rs +++ b/src/librustc_mir/util/mod.rs @@ -17,6 +17,7 @@ mod alignment; mod graphviz; pub(crate) mod pretty; pub mod liveness; +pub mod collect_writes; pub use self::alignment::is_disaligned; pub use self::pretty::{dump_enabled, dump_mir, write_mir_pretty, PassWhere}; From bfc9b761596a024e4ba6fc32997ffc74d0ba9abb Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 3 Mar 2018 01:49:55 +0530 Subject: [PATCH 05/23] add collect_writes.rs --- src/librustc_mir/lib.rs | 1 + src/librustc_mir/util/collect_writes.rs | 50 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 src/librustc_mir/util/collect_writes.rs diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index c31e95fd826c6..e21fa63ccc27e 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -39,6 +39,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![feature(collection_placement)] #![feature(nonzero)] #![feature(underscore_lifetimes)] +#![feature(crate_visibility_modifier)] extern crate arena; #[macro_use] diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs new file mode 100644 index 0000000000000..a0c01fa834b69 --- /dev/null +++ b/src/librustc_mir/util/collect_writes.rs @@ -0,0 +1,50 @@ +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use rustc::mir::{Local, Location}; +use rustc::mir::Mir; +use rustc::mir::visit::PlaceContext; +use rustc::mir::visit::Visitor; + +pub struct FindLocalAssignmentVisitor { + needle: Local, + locations: Vec, +} + +impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { + fn visit_local(&mut self, + local: &Local, + place_context: PlaceContext<'tcx>, + location: Location) { + if self.needle != *local { + return; + } + + match place_context { + PlaceContext::Store | PlaceContext::Call => { + self.locations.push(location); + } + PlaceContext::AsmOutput | PlaceContext::Drop| PlaceContext::Inspect | + PlaceContext::Borrow{..}| PlaceContext::Projection(..)| PlaceContext::Copy| + PlaceContext::Move| PlaceContext::StorageLive| PlaceContext::StorageDead| + PlaceContext::Validate => { + } + } + + Visitor::visit_local(self,local,place_context,location) + } +} + +crate trait FindAssignments { + fn find_assignments(&self, local: Local) -> Vec; + } + +impl<'tcx> FindAssignments for Mir<'tcx>{ + fn find_assignments(&self, local: Local) -> Vec{ + let mut visitor = FindLocalAssignmentVisitor{ needle: local, locations: vec![]}; + visitor.visit_mir(self); + visitor.locations + } +} + From 7a266a690289e972a5571fece7585613edac50bf Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 3 Mar 2018 17:50:11 +0530 Subject: [PATCH 06/23] minor changes --- src/librustc_mir/borrow_check/mod.rs | 25 +++++++++++++++++++------ src/librustc_mir/util/collect_writes.rs | 9 +++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 90e84a4780957..db3dd73d138f3 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1536,8 +1536,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { is_local_mutation_allowed: LocalMutationIsAllowed, ) -> bool { debug!( +<<<<<<< HEAD "check_access_permissions({:?}, {:?}, {:?})", place, kind, is_local_mutation_allowed +======= + " ({:?}, {:?}, {:?})", + place, + kind, + is_local_mutation_allowed +>>>>>>> minor changes ); let mut error_reported = false; match kind { @@ -1573,13 +1580,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - match *place{ - Place::Local(local) => {let locations = self.mir.find_assignments(local); - - for n in &locations{ - debug!("locations ={:?}", n);} + let err_help = match *place { + Place::Local(local) => { + let locations = self.mir.find_assignments(local); + Some((self.mir.source_info(locations[0]).span, "consider changing this to be a mutable reference: `&mut `")) } - _ => {}} + _ => { + None + } + }; let item_msg = if error_reported{ if let Some(name) = self.describe_place(place_err) { @@ -1602,6 +1611,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if place != place_err { err.span_label(span, "cannot assign through `&`-reference"); } + + if !err_help.is_none(){ + let (err_help_span, err_help_stmt) = err_help.unwrap(); + err.span_help(err_help_span, err_help_stmt);} err.emit(); } } diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs index a0c01fa834b69..f455a657b4784 100644 --- a/src/librustc_mir/util/collect_writes.rs +++ b/src/librustc_mir/util/collect_writes.rs @@ -26,18 +26,19 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { self.locations.push(location); } PlaceContext::AsmOutput | PlaceContext::Drop| PlaceContext::Inspect | - PlaceContext::Borrow{..}| PlaceContext::Projection(..)| PlaceContext::Copy| + PlaceContext::Borrow{..}| PlaceContext::Projection(..)| PlaceContext::Copy| PlaceContext::Move| PlaceContext::StorageLive| PlaceContext::StorageDead| PlaceContext::Validate => { + // self.super_local(local) } } - - Visitor::visit_local(self,local,place_context,location) } + + // fn super_local() } crate trait FindAssignments { - fn find_assignments(&self, local: Local) -> Vec; + fn find_assignments(&self, local: Local) -> Vec; } impl<'tcx> FindAssignments for Mir<'tcx>{ From 0c7fc046d3271b8fe31b3c2d168af4f4ab90bcdc Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 10 Mar 2018 20:24:59 +0530 Subject: [PATCH 07/23] code refactor, modify compile-fail tests --- src/librustc_borrowck/borrowck/mod.rs | 2 +- src/librustc_mir/borrow_check/mod.rs | 99 +++++++++++-------- src/librustc_mir/util/borrowck_errors.rs | 15 ++- src/librustc_mir/util/collect_writes.rs | 28 ++++-- .../borrowck/borrowck-issue-14498.rs | 2 +- .../borrowck-overloaded-index-ref-index.rs | 2 +- 6 files changed, 92 insertions(+), 56 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index bb198adea4a6a..a14163719067f 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -839,7 +839,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { let mut db = match err.cause { MutabilityViolation => { - let mut db = self.cannot_assign(error_span, &descr, Origin::Ast); + let mut db = self.cannot_assign(error_span, &descr, Origin::Ast, false); if let mc::NoteClosureEnv(upvar_id) = err.cmt.note { let node_id = self.tcx.hir.hir_to_node_id(upvar_id.var_id); let sp = self.tcx.hir.span(node_id); diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index db3dd73d138f3..2a74eff1cfc18 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1422,6 +1422,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } + fn get_main_error_message(&self, place:&Place<'tcx>) -> String{ + match self.describe_place(place) { + Some(name) => format!("immutable item `{}`", name), + None => "immutable item".to_owned(), + } + } + /// Currently MoveData does not store entries for all places in /// the input MIR. For example it will currently filter out /// places that are Copy; thus we do not track places of shared @@ -1536,15 +1543,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { is_local_mutation_allowed: LocalMutationIsAllowed, ) -> bool { debug!( -<<<<<<< HEAD "check_access_permissions({:?}, {:?}, {:?})", place, kind, is_local_mutation_allowed -======= - " ({:?}, {:?}, {:?})", - place, - kind, - is_local_mutation_allowed ->>>>>>> minor changes ); let mut error_reported = false; match kind { @@ -1559,11 +1559,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let item_msg = match self.describe_place(place) { - Some(name) => format!("immutable item `{}`", name), - None => "immutable item".to_owned(), - }; - + let item_msg = self.get_main_error_message(place); let mut err = self.tcx .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir); err.span_label(span, "cannot borrow as mutable"); @@ -1580,42 +1576,61 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let err_help = match *place { - Place::Local(local) => { - let locations = self.mir.find_assignments(local); - Some((self.mir.source_info(locations[0]).span, "consider changing this to be a mutable reference: `&mut `")) - } - _ => { - None + let err_info = match *place_err { + Place::Projection(ref proj) => { + match proj.elem { + ProjectionElem::Deref => { + match proj.base { + Place::Local(local) => { + let locations = self.mir.find_assignments(local); + if locations.len() > 0 { + let item_msg = if error_reported { + if let Some(name) = + self.describe_place(place_err) { + let var = str::replace(&name, "*", ""); + format!("`&`-reference `{}`", var) + } else { + self.get_main_error_message(place) + } + } else { + self.get_main_error_message(place) + }; + Some((self.mir.source_info(locations[0]).span, + "consider changing this to be a \ + mutable reference: `&mut`", item_msg, + "cannot assign through `&`-reference")) + } else { + None + } + } + _ => None, + } + } + _ => None, } + } + _ => None, }; - let item_msg = if error_reported{ - if let Some(name) = self.describe_place(place_err) { - format!("`&`-reference {}", name) - }else{ - match self.describe_place(place) { - Some(name) => {format!("immutable item `{}`", name)} - None => {"immutable item".to_owned()} - } - } + if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info { + let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir, true); + err.span_suggestion(err_help_span, err_help_stmt, format!("")); + if place != place_err { + err.span_label(span, sec_span); + } + err.emit() }else{ - match self.describe_place(place) { - Some(name) => {format!("immutable item `{}`", name)} - None => {"immutable item".to_owned()} + let item_msg_ = self.get_main_error_message(place); + let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir, false); + err.span_label(span, "cannot mutate"); + if place != place_err { + if let Some(name) = self.describe_place(place_err) { + err.note(&format!("Value not mutable causing this error: `{}`", + name)); + } } - }; - - let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir); - - if place != place_err { - err.span_label(span, "cannot assign through `&`-reference"); + err.emit(); } - - if !err_help.is_none(){ - let (err_help_span, err_help_stmt) = err_help.unwrap(); - err.span_help(err_help_span, err_help_stmt);} - err.emit(); } } Reservation(WriteKind::Move) diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 89242ca32bcbf..5da6d135f5f2f 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -284,18 +284,25 @@ pub trait BorrowckErrors { self.cancel_if_wrong_origin(err, o) } - fn cannot_assign(&self, span: Span, desc: &str, o: Origin) -> DiagnosticBuilder + fn cannot_assign(&self, span: Span, desc: &str, o: Origin, is_reference:bool) + -> DiagnosticBuilder { + let msg = if is_reference { + "through" + } else { + "to" + }; + let err = struct_span_err!(self, span, E0594, - "cannot assign to {}{OGN}", - desc, OGN=o); + "cannot assign {} {}{OGN}", + msg, desc, OGN=o); self.cancel_if_wrong_origin(err, o) } fn cannot_assign_static(&self, span: Span, desc: &str, o: Origin) -> DiagnosticBuilder { - self.cannot_assign(span, &format!("immutable static item `{}`", desc), o) + self.cannot_assign(span, &format!("immutable static item `{}`", desc), o, false) } fn cannot_move_out_of(&self, move_from_span: Span, move_from_desc: &str, o: Origin) diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs index f455a657b4784..7bb1214914235 100644 --- a/src/librustc_mir/util/collect_writes.rs +++ b/src/librustc_mir/util/collect_writes.rs @@ -1,3 +1,9 @@ +// 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. @@ -7,6 +13,8 @@ use rustc::mir::Mir; use rustc::mir::visit::PlaceContext; use rustc::mir::visit::Visitor; +// The Visitor walks the MIR to return the assignment statements corresponding +// to a Local. pub struct FindLocalAssignmentVisitor { needle: Local, locations: Vec, @@ -19,25 +27,32 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { location: Location) { if self.needle != *local { return; - } + } match place_context { PlaceContext::Store | PlaceContext::Call => { self.locations.push(location); } - PlaceContext::AsmOutput | PlaceContext::Drop| PlaceContext::Inspect | - PlaceContext::Borrow{..}| PlaceContext::Projection(..)| PlaceContext::Copy| - PlaceContext::Move| PlaceContext::StorageLive| PlaceContext::StorageDead| + PlaceContext::AsmOutput | + PlaceContext::Drop | + PlaceContext::Inspect | + PlaceContext::Borrow { .. } | + PlaceContext::Projection(..) | + PlaceContext::Copy | + PlaceContext::Move | + PlaceContext::StorageLive | + PlaceContext::StorageDead | PlaceContext::Validate => { + // TO-DO // self.super_local(local) } } } - + // TO-DO // fn super_local() } -crate trait FindAssignments { +crate trait FindAssignments { fn find_assignments(&self, local: Local) -> Vec; } @@ -48,4 +63,3 @@ impl<'tcx> FindAssignments for Mir<'tcx>{ visitor.locations } } - diff --git a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs index 8a09ab3fd06c8..f70e2b4816b4f 100644 --- a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs +++ b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs @@ -27,7 +27,7 @@ fn indirect_write_to_imm_box() { let y: Box<_> = box &mut x; let p = &y; ***p = 2; //[ast]~ ERROR cannot assign to data in a `&` reference - //[mir]~^ ERROR cannot assign to immutable item `***p` + //[mir]~^ ERROR cannot assign through `&`-reference `p` drop(p); } diff --git a/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs b/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs index 3a4c22eb1395a..86abd114d6afa 100644 --- a/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs +++ b/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs @@ -70,5 +70,5 @@ fn main() { }; s[2] = 20; //[ast]~^ ERROR cannot assign to immutable indexed content - //[mir]~^^ ERROR cannot assign to immutable item + //[mir]~^^ ERROR cannot assign through immutable item } From fdb2f7f97ca6e29da9f0c4cff025b2053343c8a2 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 10 Mar 2018 21:13:15 +0530 Subject: [PATCH 08/23] tidy fixes --- src/librustc_mir/util/collect_writes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs index 7bb1214914235..cd52f39c08087 100644 --- a/src/librustc_mir/util/collect_writes.rs +++ b/src/librustc_mir/util/collect_writes.rs @@ -53,9 +53,9 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { } crate trait FindAssignments { - fn find_assignments(&self, local: Local) -> Vec; + fn find_assignments(&self, local: Local) -> Vec; } - + impl<'tcx> FindAssignments for Mir<'tcx>{ fn find_assignments(&self, local: Local) -> Vec{ let mut visitor = FindLocalAssignmentVisitor{ needle: local, locations: vec![]}; From 55bd9148eb966691dd54c4afe03416acc62bb9b9 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sun, 11 Mar 2018 00:07:26 +0530 Subject: [PATCH 09/23] fix warnings --- src/librustc_mir/borrow_check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 2a74eff1cfc18..484b37691ee31 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -17,7 +17,7 @@ use rustc::hir::map::definitions::DefPathData; use rustc::infer::InferCtxt; use rustc::ty::{self, ParamEnv, TyCtxt}; use rustc::ty::maps::Providers; -use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Local, Location, Place}; +use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Location, Place}; use rustc::mir::{Mir, Mutability, Operand, Projection, ProjectionElem, Rvalue}; use rustc::mir::{Field, Statement, StatementKind, Terminator, TerminatorKind}; use rustc::mir::ClosureRegionRequirements; From 50299c6491ade2dab9f2ccf1edd6ac9123f0697d Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sun, 11 Mar 2018 00:25:23 +0530 Subject: [PATCH 10/23] add ui test for E0594 --- src/test/ui/nll/issue-47388.rs | 20 ++++++++++++++++++++ src/test/ui/nll/issue-47388.stderr | 11 +++++++++++ 2 files changed, 31 insertions(+) create mode 100644 src/test/ui/nll/issue-47388.rs create mode 100644 src/test/ui/nll/issue-47388.stderr diff --git a/src/test/ui/nll/issue-47388.rs b/src/test/ui/nll/issue-47388.rs new file mode 100644 index 0000000000000..683eec8f1bbd0 --- /dev/null +++ b/src/test/ui/nll/issue-47388.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. +#![feature(nll)] +struct FancyNum { + num: u8, +} + +fn main() { + let mut fancy = FancyNum{ num: 5 }; + let fancy_ref = &(&mut fancy); + fancy_ref.num = 6; + println!("{}", fancy_ref.num); +} \ No newline at end of file diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr new file mode 100644 index 0000000000000..331836bebbaac --- /dev/null +++ b/src/test/ui/nll/issue-47388.stderr @@ -0,0 +1,11 @@ +error[E0594]: cannot assign through `&`-reference `fancy_ref` + --> $DIR/issue-47388.rs:18:5 + | +LL | let fancy_ref = &(&mut fancy); + | ------------- help: consider changing this to be a mutable reference: `&mut` +LL | fancy_ref.num = 6; + | ^^^^^^^^^^^^^^^^^ cannot assign through `&`-reference + +error: aborting due to previous error + +If you want more information on this error, try using "rustc --explain E0594" From 7745b52524db1b5e334a92044c00ba92facf9f99 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sun, 11 Mar 2018 00:57:03 +0530 Subject: [PATCH 11/23] fix tidy issues --- src/test/ui/nll/issue-47388.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/nll/issue-47388.rs b/src/test/ui/nll/issue-47388.rs index 683eec8f1bbd0..a3ce70438cb90 100644 --- a/src/test/ui/nll/issue-47388.rs +++ b/src/test/ui/nll/issue-47388.rs @@ -17,4 +17,4 @@ fn main() { let fancy_ref = &(&mut fancy); fancy_ref.num = 6; println!("{}", fancy_ref.num); -} \ No newline at end of file +} From 311a8bef6e879bd0f393154e7a01a41bce385ad9 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Mon, 12 Mar 2018 20:14:37 +0530 Subject: [PATCH 12/23] address code review comments --- src/librustc_mir/borrow_check/mod.rs | 6 +++--- src/test/ui/nll/issue-47388.rs | 2 +- src/test/ui/nll/issue-47388.stderr | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 484b37691ee31..c774a8b9d9281 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1566,7 +1566,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if place != place_err { if let Some(name) = self.describe_place(place_err) { - err.note(&format!("Value not mutable causing this error: `{}`", name)); + err.note(&format!("value not mutable causing this error: `{}`", name)); } } @@ -1619,13 +1619,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label(span, sec_span); } err.emit() - }else{ + } else { let item_msg_ = self.get_main_error_message(place); let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir, false); err.span_label(span, "cannot mutate"); if place != place_err { if let Some(name) = self.describe_place(place_err) { - err.note(&format!("Value not mutable causing this error: `{}`", + err.note(&format!("value not mutable causing this error: `{}`", name)); } } diff --git a/src/test/ui/nll/issue-47388.rs b/src/test/ui/nll/issue-47388.rs index a3ce70438cb90..65748eca2a0cd 100644 --- a/src/test/ui/nll/issue-47388.rs +++ b/src/test/ui/nll/issue-47388.rs @@ -15,6 +15,6 @@ struct FancyNum { fn main() { let mut fancy = FancyNum{ num: 5 }; let fancy_ref = &(&mut fancy); - fancy_ref.num = 6; + fancy_ref.num = 6; //~^ ERROR E0594 println!("{}", fancy_ref.num); } diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr index 331836bebbaac..41899e97c9e2d 100644 --- a/src/test/ui/nll/issue-47388.stderr +++ b/src/test/ui/nll/issue-47388.stderr @@ -3,7 +3,7 @@ error[E0594]: cannot assign through `&`-reference `fancy_ref` | LL | let fancy_ref = &(&mut fancy); | ------------- help: consider changing this to be a mutable reference: `&mut` -LL | fancy_ref.num = 6; +LL | fancy_ref.num = 6; //~^ ERROR E0594 | ^^^^^^^^^^^^^^^^^ cannot assign through `&`-reference error: aborting due to previous error From 6c649fbed4d4d86aed16dff8c0245b4871353cd1 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Wed, 14 Mar 2018 23:51:54 +0530 Subject: [PATCH 13/23] address code review comments --- src/librustc_mir/borrow_check/mod.rs | 52 +++++++++++++------------ src/librustc_mir/util/collect_writes.rs | 28 ++++++------- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index c774a8b9d9281..dec32f5b2e53c 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1422,13 +1422,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - fn get_main_error_message(&self, place:&Place<'tcx>) -> String{ - match self.describe_place(place) { - Some(name) => format!("immutable item `{}`", name), - None => "immutable item".to_owned(), - } - } - /// Currently MoveData does not store entries for all places in /// the input MIR. For example it will currently filter out /// places that are Copy; thus we do not track places of shared @@ -1533,6 +1526,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } + fn specialized_description(&self, place:&Place<'tcx>) -> Option{ + if let Some(name) = self.describe_place(place) { + Some(format!("`&`-reference `{}`", name)) + } else { + None + } + } + + fn get_main_error_message(&self, place:&Place<'tcx>) -> String{ + match self.describe_place(place) { + Some(name) => format!("immutable item `{}`", name), + None => "immutable item".to_owned(), + } + } + /// Check the permissions for the given place and read or write kind /// /// Returns true if an error is reported, false otherwise. @@ -1566,7 +1574,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if place != place_err { if let Some(name) = self.describe_place(place_err) { - err.note(&format!("value not mutable causing this error: `{}`", name)); + err.note(&format!("the value which is causing this path not to be mutable is...: `{}`", name)); } } @@ -1576,7 +1584,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let err_info = match *place_err { + let mut err_info = None; + match *place_err { Place::Projection(ref proj) => { match proj.elem { ProjectionElem::Deref => { @@ -1585,32 +1594,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let locations = self.mir.find_assignments(local); if locations.len() > 0 { let item_msg = if error_reported { - if let Some(name) = - self.describe_place(place_err) { - let var = str::replace(&name, "*", ""); - format!("`&`-reference `{}`", var) - } else { - self.get_main_error_message(place) + match self.specialized_description(&proj.base){ + Some(msg) => msg, + None => self.get_main_error_message(place) } } else { self.get_main_error_message(place) }; - Some((self.mir.source_info(locations[0]).span, + err_info = Some((self.mir.source_info(locations[0]).span, "consider changing this to be a \ mutable reference: `&mut`", item_msg, - "cannot assign through `&`-reference")) - } else { - None + "cannot assign through `&`-reference")); } } - _ => None, + _ => {}, } } - _ => None, + _ => {} } } - _ => None, - }; + _ => {} + } if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info { let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir, true); @@ -1625,7 +1629,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label(span, "cannot mutate"); if place != place_err { if let Some(name) = self.describe_place(place_err) { - err.note(&format!("value not mutable causing this error: `{}`", + err.note(&format!("the value which is causing this path not to be mutable is...: `{}`", name)); } } diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs index cd52f39c08087..f04f9233447c3 100644 --- a/src/librustc_mir/util/collect_writes.rs +++ b/src/librustc_mir/util/collect_writes.rs @@ -13,9 +13,23 @@ use rustc::mir::Mir; use rustc::mir::visit::PlaceContext; use rustc::mir::visit::Visitor; +crate trait FindAssignments { + // Finds all statements that assign directly to local (i.e., X = ...) + // and returns their locations. + fn find_assignments(&self, local: Local) -> Vec; +} + +impl<'tcx> FindAssignments for Mir<'tcx>{ + fn find_assignments(&self, local: Local) -> Vec{ + let mut visitor = FindLocalAssignmentVisitor{ needle: local, locations: vec![]}; + visitor.visit_mir(self); + visitor.locations + } +} + // The Visitor walks the MIR to return the assignment statements corresponding // to a Local. -pub struct FindLocalAssignmentVisitor { +struct FindLocalAssignmentVisitor { needle: Local, locations: Vec, } @@ -51,15 +65,3 @@ impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { // TO-DO // fn super_local() } - -crate trait FindAssignments { - fn find_assignments(&self, local: Local) -> Vec; - } - -impl<'tcx> FindAssignments for Mir<'tcx>{ - fn find_assignments(&self, local: Local) -> Vec{ - let mut visitor = FindLocalAssignmentVisitor{ needle: local, locations: vec![]}; - visitor.visit_mir(self); - visitor.locations - } -} From c1192065ea02612989f71fe03808011323fd7378 Mon Sep 17 00:00:00 2001 From: Gauri Kholkar Date: Thu, 5 Apr 2018 22:04:20 +0530 Subject: [PATCH 14/23] Update borrowck_errors.rs --- src/librustc_mir/util/borrowck_errors.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 1078013a3e223..6c979de7a6874 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -284,12 +284,8 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { self.cancel_if_wrong_origin(err, o) } -<<<<<<< HEAD fn cannot_assign(&self, span: Span, desc: &str, o: Origin, is_reference:bool) -> DiagnosticBuilder -======= - fn cannot_assign(self, span: Span, desc: &str, o: Origin) -> DiagnosticBuilder<'cx> ->>>>>>> 56714acc5eb0687ed9a7566fdebe5528657fc5b3 { let msg = if is_reference { "through" From 12d141561dcd33d3074e09351df46c7129ce46ce Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 5 Apr 2018 22:49:09 +0530 Subject: [PATCH 15/23] tidy fixes --- src/librustc_mir/borrow_check/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 5f54b7b4d9d53..636909d096e18 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1553,7 +1553,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if place != place_err { if let Some(name) = self.describe_place(place_err) { - err.note(&format!("the value which is causing this path not to be mutable is...: `{}`", name)); + err.note(&format!("the value which is causing this path not to be mutable \ + is...: `{}`", name)); } } @@ -1580,7 +1581,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } else { self.get_main_error_message(place) }; - err_info = Some((self.mir.source_info(locations[0]).span, + err_info = Some(( + self.mir.source_info(locations[0]).span, "consider changing this to be a \ mutable reference: `&mut`", item_msg, "cannot assign through `&`-reference")); @@ -1608,8 +1610,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label(span, "cannot mutate"); if place != place_err { if let Some(name) = self.describe_place(place_err) { - err.note(&format!("the value which is causing this path not to be mutable is...: `{}`", - name)); + err.note(&format!("the value which is causing this path not to be \ + mutable is...: `{}`", name)); } } err.emit(); From 1fb25fbbe30e81bdbc9b8b63b01603d7a6124209 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Fri, 6 Apr 2018 20:00:21 +0530 Subject: [PATCH 16/23] reduce nested loops in the code --- src/librustc_mir/borrow_check/mod.rs | 86 ++++++++++++++++-------- src/librustc_mir/util/borrowck_errors.rs | 4 +- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 636909d096e18..9f84e3469cdc3 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1565,38 +1565,70 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { error_reported = true; let mut err_info = None; + match *place_err { - Place::Projection(ref proj) => { - match proj.elem { - ProjectionElem::Deref => { - match proj.base { - Place::Local(local) => { - let locations = self.mir.find_assignments(local); - if locations.len() > 0 { - let item_msg = if error_reported { - match self.specialized_description(&proj.base){ - Some(msg) => msg, - None => self.get_main_error_message(place) - } - } else { - self.get_main_error_message(place) - }; - err_info = Some(( - self.mir.source_info(locations[0]).span, - "consider changing this to be a \ - mutable reference: `&mut`", item_msg, - "cannot assign through `&`-reference")); - } + + Place::Projection(box Projection { + ref base, elem:ProjectionElem::Deref}) => { + + match *base { + Place::Local(local) => { + let locations = self.mir.find_assignments(local); + if locations.len() > 0 { + let item_msg = if error_reported { + match self.specialized_description(base){ + Some(msg) => msg, + None => self.get_main_error_message(place) + } + } else { + self.get_main_error_message(place) + }; + err_info = Some(( + self.mir.source_info(locations[0]).span, + "consider changing this to be a \ + mutable reference: `&mut`", item_msg, + "cannot assign through `&`-reference")); } - _ => {}, - } - } - _ => {} + }, + _ => {}, } - } - _ => {} + }, + _ => {}, } + + // match *place_err { + // Place::Projection(ref proj) => { + // match proj.elem { + // ProjectionElem::Deref => { + // match proj.base { + // Place::Local(local) => { + // let locations = self.mir.find_assignments(local); + // if locations.len() > 0 { + // let item_msg = if error_reported { + // match self.specialized_description(base){ + // Some(msg) => msg, + // None => self.get_main_error_message(place) + // } + // } else { + // self.get_main_error_message(place) + // }; + // err_info = Some(( + // self.mir.source_info(locations[0]).span, + // "consider changing this to be a \ + // mutable reference: `&mut`", item_msg, + // "cannot assign through `&`-reference")); + // } + // } + // _ => {}, + // } + // } + // _ => {} + // } + // } + // _ => {} + // } + if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info { let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir, true); err.span_suggestion(err_help_span, err_help_stmt, format!("")); diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 6c979de7a6874..bcb3b605450da 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -284,8 +284,8 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { self.cancel_if_wrong_origin(err, o) } - fn cannot_assign(&self, span: Span, desc: &str, o: Origin, is_reference:bool) - -> DiagnosticBuilder + fn cannot_assign(self, span: Span, desc: &str, o: Origin, is_reference: bool) + -> DiagnosticBuilder<'cx> { let msg = if is_reference { "through" From e6938ee08e687dca9a6ac0f138f00b225e1b9038 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Fri, 6 Apr 2018 20:04:07 +0530 Subject: [PATCH 17/23] fix ui test --- src/test/ui/nll/issue-47388.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr index 41899e97c9e2d..3b620eaca4bc2 100644 --- a/src/test/ui/nll/issue-47388.stderr +++ b/src/test/ui/nll/issue-47388.stderr @@ -8,4 +8,4 @@ LL | fancy_ref.num = 6; //~^ ERROR E0594 error: aborting due to previous error -If you want more information on this error, try using "rustc --explain E0594" +For more information about this error, try `rustc --explain E0594`. From 6686d104c87891bb1476dfc7036b622fb2ee5210 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Fri, 6 Apr 2018 20:28:00 +0530 Subject: [PATCH 18/23] tidy fix --- src/librustc_mir/borrow_check/mod.rs | 35 +--------------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 9f84e3469cdc3..0f529c77aea6b 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1592,43 +1592,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }, _ => {}, } - }, + }, _ => {}, } - - // match *place_err { - // Place::Projection(ref proj) => { - // match proj.elem { - // ProjectionElem::Deref => { - // match proj.base { - // Place::Local(local) => { - // let locations = self.mir.find_assignments(local); - // if locations.len() > 0 { - // let item_msg = if error_reported { - // match self.specialized_description(base){ - // Some(msg) => msg, - // None => self.get_main_error_message(place) - // } - // } else { - // self.get_main_error_message(place) - // }; - // err_info = Some(( - // self.mir.source_info(locations[0]).span, - // "consider changing this to be a \ - // mutable reference: `&mut`", item_msg, - // "cannot assign through `&`-reference")); - // } - // } - // _ => {}, - // } - // } - // _ => {} - // } - // } - // _ => {} - // } - if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info { let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir, true); err.span_suggestion(err_help_span, err_help_stmt, format!("")); From e18a83be8846010f3dbb1ef96b86941210517cc9 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Fri, 6 Apr 2018 23:03:20 +0530 Subject: [PATCH 19/23] fix ui test --- src/test/ui/nll/issue-47388.rs | 2 +- src/test/ui/nll/issue-47388.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/nll/issue-47388.rs b/src/test/ui/nll/issue-47388.rs index 65748eca2a0cd..39feea08aa489 100644 --- a/src/test/ui/nll/issue-47388.rs +++ b/src/test/ui/nll/issue-47388.rs @@ -15,6 +15,6 @@ struct FancyNum { fn main() { let mut fancy = FancyNum{ num: 5 }; let fancy_ref = &(&mut fancy); - fancy_ref.num = 6; //~^ ERROR E0594 + fancy_ref.num = 6; //~ ERROR E0594 println!("{}", fancy_ref.num); } diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr index 3b620eaca4bc2..ae6d8d7a59814 100644 --- a/src/test/ui/nll/issue-47388.stderr +++ b/src/test/ui/nll/issue-47388.stderr @@ -3,7 +3,7 @@ error[E0594]: cannot assign through `&`-reference `fancy_ref` | LL | let fancy_ref = &(&mut fancy); | ------------- help: consider changing this to be a mutable reference: `&mut` -LL | fancy_ref.num = 6; //~^ ERROR E0594 +LL | fancy_ref.num = 6; //~ ERROR E0594 | ^^^^^^^^^^^^^^^^^ cannot assign through `&`-reference error: aborting due to previous error From e5a96a4b9537d415c62aab2aacc79d89abdb35e8 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 7 Apr 2018 10:54:13 +0530 Subject: [PATCH 20/23] modify the error message- CR Comments --- src/librustc_borrowck/borrowck/mod.rs | 2 +- src/librustc_mir/borrow_check/mod.rs | 39 ++++++++++++------- src/librustc_mir/util/borrowck_errors.rs | 14 ++----- .../borrowck/borrowck-issue-14498.rs | 2 +- .../borrowck-overloaded-index-ref-index.rs | 2 +- src/test/ui/nll/issue-47388.stderr | 4 +- 6 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 6e8ebce84151c..93d6247eeae47 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -839,7 +839,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { let mut db = match err.cause { MutabilityViolation => { - let mut db = self.cannot_assign(error_span, &descr, Origin::Ast, false); + let mut db = self.cannot_assign(error_span, &descr, Origin::Ast); if let mc::NoteClosureEnv(upvar_id) = err.cmt.note { let node_id = self.tcx.hir.hir_to_node_id(upvar_id.var_id); let sp = self.tcx.hir.span(node_id); diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 0f529c77aea6b..db7427ddae596 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1506,20 +1506,35 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } fn specialized_description(&self, place:&Place<'tcx>) -> Option{ - if let Some(name) = self.describe_place(place) { - Some(format!("`&`-reference `{}`", name)) + if let Some(_name) = self.describe_place(place) { + Some(format!("data in a `&` reference")) } else { None } } - fn get_main_error_message(&self, place:&Place<'tcx>) -> String{ + fn get_default_err_msg(&self, place:&Place<'tcx>) -> String{ match self.describe_place(place) { Some(name) => format!("immutable item `{}`", name), None => "immutable item".to_owned(), } } + fn get_secondary_err_msg(&self, place:&Place<'tcx>) -> String{ + match self.specialized_description(place) { + Some(_) => format!("data in a `&` reference"), + None => self.get_default_err_msg(place) + } + } + + fn get_primary_err_msg(&self, place:&Place<'tcx>) -> String{ + if let Some(name) = self.describe_place(place) { + format!("`{}` is a `&` reference, so the data it refers to cannot be written", name) + } else { + format!("cannot assign through `&`-reference") + } + } + /// Check the permissions for the given place and read or write kind /// /// Returns true if an error is reported, false otherwise. @@ -1546,7 +1561,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let item_msg = self.get_main_error_message(place); + let item_msg = self.get_default_err_msg(place); let mut err = self.tcx .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir); err.span_label(span, "cannot borrow as mutable"); @@ -1576,18 +1591,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let locations = self.mir.find_assignments(local); if locations.len() > 0 { let item_msg = if error_reported { - match self.specialized_description(base){ - Some(msg) => msg, - None => self.get_main_error_message(place) - } + self.get_secondary_err_msg(base) } else { - self.get_main_error_message(place) + self.get_default_err_msg(place) }; + err_info = Some(( self.mir.source_info(locations[0]).span, "consider changing this to be a \ mutable reference: `&mut`", item_msg, - "cannot assign through `&`-reference")); + self.get_primary_err_msg(base))); } }, _ => {}, @@ -1597,15 +1610,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info { - let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir, true); + let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir); err.span_suggestion(err_help_span, err_help_stmt, format!("")); if place != place_err { err.span_label(span, sec_span); } err.emit() } else { - let item_msg_ = self.get_main_error_message(place); - let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir, false); + let item_msg_ = self.get_default_err_msg(place); + let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir); err.span_label(span, "cannot mutate"); if place != place_err { if let Some(name) = self.describe_place(place_err) { diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index bcb3b605450da..d6b3e674f8f80 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -284,25 +284,19 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { self.cancel_if_wrong_origin(err, o) } - fn cannot_assign(self, span: Span, desc: &str, o: Origin, is_reference: bool) + fn cannot_assign(self, span: Span, desc: &str, o: Origin) -> DiagnosticBuilder<'cx> { - let msg = if is_reference { - "through" - } else { - "to" - }; - let err = struct_span_err!(self, span, E0594, - "cannot assign {} {}{OGN}", - msg, desc, OGN=o); + "cannot assign to {}{OGN}", + desc, OGN=o); self.cancel_if_wrong_origin(err, o) } fn cannot_assign_static(self, span: Span, desc: &str, o: Origin) -> DiagnosticBuilder<'cx> { - self.cannot_assign(span, &format!("immutable static item `{}`", desc), o, false) + self.cannot_assign(span, &format!("immutable static item `{}`", desc), o) } fn cannot_move_out_of(self, move_from_span: Span, move_from_desc: &str, o: Origin) diff --git a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs index f70e2b4816b4f..fbdd013024db5 100644 --- a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs +++ b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs @@ -27,7 +27,7 @@ fn indirect_write_to_imm_box() { let y: Box<_> = box &mut x; let p = &y; ***p = 2; //[ast]~ ERROR cannot assign to data in a `&` reference - //[mir]~^ ERROR cannot assign through `&`-reference `p` + //[mir]~^ ERROR cannot assign to data in a `&` reference drop(p); } diff --git a/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs b/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs index 86abd114d6afa..3a4c22eb1395a 100644 --- a/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs +++ b/src/test/compile-fail/borrowck/borrowck-overloaded-index-ref-index.rs @@ -70,5 +70,5 @@ fn main() { }; s[2] = 20; //[ast]~^ ERROR cannot assign to immutable indexed content - //[mir]~^^ ERROR cannot assign through immutable item + //[mir]~^^ ERROR cannot assign to immutable item } diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr index ae6d8d7a59814..272cb6510aa3d 100644 --- a/src/test/ui/nll/issue-47388.stderr +++ b/src/test/ui/nll/issue-47388.stderr @@ -1,10 +1,10 @@ -error[E0594]: cannot assign through `&`-reference `fancy_ref` +error[E0594]: cannot assign to data in a `&` reference --> $DIR/issue-47388.rs:18:5 | LL | let fancy_ref = &(&mut fancy); | ------------- help: consider changing this to be a mutable reference: `&mut` LL | fancy_ref.num = 6; //~ ERROR E0594 - | ^^^^^^^^^^^^^^^^^ cannot assign through `&`-reference + | ^^^^^^^^^^^^^^^^^ `fancy_ref` is a `&` reference, so the data it refers to cannot be written error: aborting due to previous error From cbde62c2cc5cfcbbb39bfc1e3ef61016a1693298 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 7 Apr 2018 14:36:29 +0530 Subject: [PATCH 21/23] fix tidy errors --- src/librustc_mir/borrow_check/mod.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index db7427ddae596..e3714802e2a91 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1578,14 +1578,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Reservation(WriteKind::Mutate) | Write(WriteKind::Mutate) => { if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; - let mut err_info = None; - match *place_err { - Place::Projection(box Projection { ref base, elem:ProjectionElem::Deref}) => { - match *base { Place::Local(local) => { let locations = self.mir.find_assignments(local); @@ -1594,8 +1590,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.get_secondary_err_msg(base) } else { self.get_default_err_msg(place) - }; - + }; err_info = Some(( self.mir.source_info(locations[0]).span, "consider changing this to be a \ From 2ad20e8127094bb22d88e2e98012ad4ea20e2d3f Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sun, 8 Apr 2018 22:57:48 +0530 Subject: [PATCH 22/23] tidy fixes --- src/librustc_mir/borrow_check/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index e3714802e2a91..3d1d64bb71ccd 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1576,10 +1576,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.emit(); }, Reservation(WriteKind::Mutate) | Write(WriteKind::Mutate) => { + if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) { error_reported = true; let mut err_info = None; match *place_err { + Place::Projection(box Projection { ref base, elem:ProjectionElem::Deref}) => { match *base { @@ -1641,9 +1643,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); } } - Activation(..) => {} // permission checks are done at Reservation point. - Read(ReadKind::Borrow(BorrowKind::Unique)) | Read(ReadKind::Borrow(BorrowKind::Mut { .. })) | Read(ReadKind::Borrow(BorrowKind::Shared)) From c792d1e0c1a95bdc6505d8012efbede4600985d1 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Mon, 9 Apr 2018 23:12:13 +0530 Subject: [PATCH 23/23] tidy fixes --- src/librustc_mir/borrow_check/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 3d1d64bb71ccd..2a7cbe28952eb 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1529,11 +1529,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { fn get_primary_err_msg(&self, place:&Place<'tcx>) -> String{ if let Some(name) = self.describe_place(place) { - format!("`{}` is a `&` reference, so the data it refers to cannot be written", name) + format!("`{}` is a `&` reference, so the data it refers to cannot be written", name) } else { format!("cannot assign through `&`-reference") } - } + } /// Check the permissions for the given place and read or write kind /// @@ -1592,7 +1592,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.get_secondary_err_msg(base) } else { self.get_default_err_msg(place) - }; + }; err_info = Some(( self.mir.source_info(locations[0]).span, "consider changing this to be a \