Skip to content

Modify compile-fail/E0389 error message WIP #48914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Apr 10, 2018
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 81 additions & 19 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1504,6 +1505,36 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

fn specialized_description(&self, place:&Place<'tcx>) -> Option<String>{
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.
Expand All @@ -1530,19 +1561,15 @@ 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));
}
}

Expand All @@ -1552,21 +1579,55 @@ 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 mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
err.span_label(span, "cannot mutate");
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)
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A line consisting of only spaces is also considered "trailing whitespaces".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @kennytm :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kennytm can you help out with the fixes for the tidy errors? I've tried certain things but they don't seem to work. One of the errors is also in the previously written code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaurikholkar The line numbers seems wrong. You could run tidy locally by

./x.py test src/tools/tidy

The lines with trailing whitespaces are:

tidy error: src/librustc_mir/borrow_check/mod.rs:1532: trailing whitespace
tidy error: src/librustc_mir/borrow_check/mod.rs:1536: trailing whitespace
tidy error: src/librustc_mir/borrow_check/mod.rs:1595: trailing whitespace

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks you @kennytm . Yeah the line nos were wrong. My local tidy script panics -> logs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaurikholkar you should just remove the entire binaryen folder, it is no longer used.

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)));
}
},
_ => {},
}
},
_ => {},
}

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)
Expand Down Expand Up @@ -2209,3 +2270,4 @@ impl ContextKind {
}
}
}

1 change: 1 addition & 0 deletions src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this needed?

Copy link
Author

@gaurikholkar-zz gaurikholkar-zz Mar 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, due to the usage here crate trait FindAssignments { in collect_writes.rs


extern crate arena;
#[macro_use]
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/util/borrowck_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down
67 changes: 67 additions & 0 deletions src/librustc_mir/util/collect_writes.rs
Original file line number Diff line number Diff line change
@@ -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 <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.

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<Location>;
}

impl<'tcx> FindAssignments for Mir<'tcx>{
fn find_assignments(&self, local: Local) -> Vec<Location>{
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<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 => {
// TO-DO
// self.super_local(local)
}
}
}
// TO-DO
// fn super_local()
}
1 change: 1 addition & 0 deletions src/librustc_mir/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/borrowck/borrowck-issue-14498.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/nll/issue-47388.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.
#![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);
}
11 changes: 11 additions & 0 deletions src/test/ui/nll/issue-47388.stderr
Original file line number Diff line number Diff line change
@@ -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`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe either the span or the suggestion text might be incorrect. It should either suggest &mut (&mut fancy) for this span, or use a span only selecting the &.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #49859

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`.