diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 305df37466d02..2a7cbe28952eb 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -42,6 +42,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; @@ -1504,6 +1505,36 @@ 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!("data in a `&` reference")) + } else { + None + } + } + + 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. @@ -1530,43 +1561,70 @@ 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_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"); 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)); } } 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 { + Place::Local(local) => { + let locations = self.mir.find_assignments(local); + if locations.len() > 0 { + let item_msg = if error_reported { + 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 \ + mutable reference: `&mut`", item_msg, + self.get_primary_err_msg(base))); + } + }, + _ => {}, + } + }, + _ => {}, + } - let item_msg = 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)); + 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); + 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_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) { + err.note(&format!("the value which is causing this path not to be \ + mutable is...: `{}`", name)); + } + } + err.emit(); } - - err.emit(); } } Reservation(WriteKind::Move) @@ -1585,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)) @@ -2209,3 +2265,4 @@ impl ContextKind { } } } + diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 84baa8c541781..682250391b67b 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -38,6 +38,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![cfg_attr(stage0, feature(underscore_lifetimes))] #![cfg_attr(stage0, feature(never_type))] #![feature(inclusive_range_fields)] +#![feature(crate_visibility_modifier)] extern crate arena; #[macro_use] diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 5e15348de5e71..d6b3e674f8f80 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -284,7 +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) -> DiagnosticBuilder<'cx> + fn cannot_assign(self, span: Span, desc: &str, o: Origin) + -> DiagnosticBuilder<'cx> { let err = struct_span_err!(self, span, E0594, "cannot assign to {}{OGN}", diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs new file mode 100644 index 0000000000000..f04f9233447c3 --- /dev/null +++ b/src/librustc_mir/util/collect_writes.rs @@ -0,0 +1,67 @@ +// 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. + +use rustc::mir::{Local, Location}; +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. +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 => { + // TO-DO + // self.super_local(local) + } + } + } + // TO-DO + // fn super_local() +} 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}; diff --git a/src/test/compile-fail/borrowck/borrowck-issue-14498.rs b/src/test/compile-fail/borrowck/borrowck-issue-14498.rs index 8a09ab3fd06c8..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 to immutable item `***p` + //[mir]~^ ERROR cannot assign to data in a `&` reference drop(p); } diff --git a/src/test/ui/nll/issue-47388.rs b/src/test/ui/nll/issue-47388.rs new file mode 100644 index 0000000000000..39feea08aa489 --- /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; //~ ERROR E0594 + println!("{}", fancy_ref.num); +} diff --git a/src/test/ui/nll/issue-47388.stderr b/src/test/ui/nll/issue-47388.stderr new file mode 100644 index 0000000000000..272cb6510aa3d --- /dev/null +++ b/src/test/ui/nll/issue-47388.stderr @@ -0,0 +1,11 @@ +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 + | ^^^^^^^^^^^^^^^^^ `fancy_ref` 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`.