Auto merge of #33688 - jonathandturner:fix_old_school, r=nikomatsakis

Fix for old school error issues, improvements to new school

This PR:
* Fixes some old school error issues, specifically #33559, #33543, #33366
* Improves wording borrowck errors with match patterns
* De-emphasize multi-line spans, so we don't color the single source character when we're trying to say "span starts here"
* Rollup of #33392 (which should help fix #33390)

r? @nikomatsakis
This commit is contained in:
bors 2016-05-18 21:19:07 -07:00
commit 0c5d651d0b
13 changed files with 125 additions and 68 deletions

View File

@ -90,7 +90,7 @@ use std::cell::{Cell, RefCell};
use std::char::from_u32;
use std::fmt;
use syntax::ast;
use syntax::errors::DiagnosticBuilder;
use syntax::errors::{DiagnosticBuilder, check_old_skool};
use syntax::codemap::{self, Pos, Span};
use syntax::parse::token;
use syntax::ptr::P;
@ -481,7 +481,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
"{}",
trace.origin);
if !is_simple_error {
if !is_simple_error || check_old_skool() {
err.note_expected_found(&"type", &expected, &found);
}

View File

@ -872,7 +872,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
&format!("borrow of `{}` occurs here",
self.bccx.loan_path_to_string(loan_path)))
.span_label(span,
&format!("assignment to `{}` occurs here",
&format!("assignment to borrowed `{}` occurs here",
self.bccx.loan_path_to_string(loan_path)))
.emit();
}

View File

@ -126,7 +126,7 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
move_from.descriptive_string(bccx.tcx));
err.span_label(
move_from.span,
&format!("move occurs here")
&format!("cannot move out of {}", move_from.descriptive_string(bccx.tcx))
);
err
}
@ -138,7 +138,7 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
"cannot move out of type `{}`, \
a non-copy fixed-size array",
b.ty);
err.span_label(move_from.span, &format!("can not move out of here"));
err.span_label(move_from.span, &format!("cannot move out of here"));
err
} else {
span_bug!(move_from.span, "this path should not cause illegal move");
@ -154,7 +154,7 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
"cannot move out of type `{}`, \
which defines the `Drop` trait",
b.ty);
err.span_label(move_from.span, &format!("can not move out of here"));
err.span_label(move_from.span, &format!("cannot move out of here"));
err
},
_ => {
@ -175,16 +175,12 @@ fn note_move_destination(mut err: DiagnosticBuilder,
if is_first_note {
err.span_label(
move_to_span,
&format!("attempting to move value to here"));
err.help(
&format!("to prevent the move, \
use `ref {0}` or `ref mut {0}` to capture value by \
reference",
&format!("hint: to prevent move, use `ref {0}` or `ref mut {0}`",
pat_name));
err
} else {
err.span_note(move_to_span,
&format!("and here (use `ref {0}` or `ref mut {0}`)",
err.span_label(move_to_span,
&format!("...and here (use `ref {0}` or `ref mut {0}`)",
pat_name));
err
}

View File

@ -367,7 +367,8 @@ impl EmitterWriter {
let mut output_vec = vec![];
for span_label in msp.span_labels() {
let mut snippet_data = snippet_data.clone();
let mut snippet_data = SnippetData::new(self.cm.clone(),
Some(span_label.span));
snippet_data.push(span_label.span,
span_label.is_primary,
@ -524,6 +525,13 @@ impl Destination {
}
Style::Quotation => {
}
Style::OldSkoolNote => {
self.start_attr(term::Attr::Bold)?;
self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_GREEN))?;
}
Style::OldSkoolNoteText => {
self.start_attr(term::Attr::Bold)?;
}
Style::UnderlinePrimary | Style::LabelPrimary => {
self.start_attr(term::Attr::Bold)?;
self.start_attr(term::Attr::ForegroundColor(lvl.color()))?;

View File

@ -699,13 +699,13 @@ pub fn expect<T, M>(diag: &Handler, opt: Option<T>, msg: M) -> T where
///
/// FIXME(#33240)
#[cfg(not(test))]
fn check_old_skool() -> bool {
pub fn check_old_skool() -> bool {
use std::env;
env::var("RUST_NEW_ERROR_FORMAT").is_err()
}
/// For unit tests, use the new format.
#[cfg(test)]
fn check_old_skool() -> bool {
pub fn check_old_skool() -> bool {
false
}

View File

@ -58,6 +58,9 @@ struct Annotation {
/// Is this annotation derived from primary span
is_primary: bool,
/// Is this a large span minimized down to a smaller span
is_minimized: bool,
/// Optional label to display adjacent to the annotation.
label: Option<String>,
}
@ -90,6 +93,8 @@ pub enum Style {
UnderlineSecondary,
LabelPrimary,
LabelSecondary,
OldSkoolNoteText,
OldSkoolNote,
NoStyle,
}
@ -382,10 +387,10 @@ impl FileInfo {
// Basically, although this loses information, multi-line spans just
// never look good.
let (line, start_col, mut end_col) = if lines.len() == 1 {
(lines[0].line_index, lines[0].start_col, lines[0].end_col)
let (line, start_col, mut end_col, is_minimized) = if lines.len() == 1 {
(lines[0].line_index, lines[0].start_col, lines[0].end_col, false)
} else {
(lines[0].line_index, lines[0].start_col, CharPos(lines[0].start_col.0 + 1))
(lines[0].line_index, lines[0].start_col, CharPos(lines[0].start_col.0 + 1), true)
};
// Watch out for "empty spans". If we get a span like 6..6, we
@ -401,6 +406,7 @@ impl FileInfo {
self.lines[index].push_annotation(start_col,
end_col,
is_primary,
is_minimized,
label);
}
@ -497,6 +503,30 @@ impl FileInfo {
match self.primary_span {
Some(span) => {
let lo = codemap.lookup_char_pos(span.lo);
let hi = codemap.lookup_char_pos(span.hi);
//Before each secondary line in old skool-mode, print the label
//as an old-style note
if !line.annotations[0].is_primary {
if let Some(ann) = line.annotations[0].label.clone() {
output.push(RenderedLine {
text: vec![StyledString {
text: lo.file.name.clone(),
style: Style::FileNameStyle,
}, StyledString {
text: format!(":{}:{}: {}:{} ", lo.line, lo.col.0 + 1,
hi.line, hi.col.0+1),
style: Style::LineAndColumn,
}, StyledString {
text: format!("note: "),
style: Style::OldSkoolNote,
}, StyledString {
text: format!("{}", ann),
style: Style::OldSkoolNoteText,
}],
kind: RenderedLineKind::Annotations,
});
}
}
rendered_lines[0].text.insert(0, StyledString {
text: format!(":{} ", lo.line),
style: Style::LineAndColumn,
@ -598,7 +628,7 @@ impl FileInfo {
if annotation.is_primary {
Style::UnderlinePrimary
} else {
Style::UnderlineSecondary
Style::OldSkoolNote
});
}
else {
@ -606,7 +636,7 @@ impl FileInfo {
if annotation.is_primary {
Style::UnderlinePrimary
} else {
Style::UnderlineSecondary
Style::OldSkoolNote
});
}
}
@ -615,10 +645,14 @@ impl FileInfo {
for p in annotation.start_col .. annotation.end_col {
if annotation.is_primary {
styled_buffer.putc(1, p, '^', Style::UnderlinePrimary);
styled_buffer.set_style(0, p, Style::UnderlinePrimary);
if !annotation.is_minimized {
styled_buffer.set_style(0, p, Style::UnderlinePrimary);
}
} else {
styled_buffer.putc(1, p, '-', Style::UnderlineSecondary);
styled_buffer.set_style(0, p, Style::UnderlineSecondary);
if !annotation.is_minimized {
styled_buffer.set_style(0, p, Style::UnderlineSecondary);
}
}
}
}
@ -819,11 +853,13 @@ impl Line {
start: CharPos,
end: CharPos,
is_primary: bool,
is_minimized: bool,
label: Option<String>) {
self.annotations.push(Annotation {
start_col: start.0,
end_col: end.0,
is_primary: is_primary,
is_minimized: is_minimized,
label: label,
});
}

View File

@ -19,8 +19,8 @@ enum Foo {
fn blah() {
let f = &Foo::Foo1(box 1, box 2);
match *f { //~ ERROR cannot move out of
//~| move occurs here
Foo::Foo1(num1, //~ NOTE attempting to move value to here
//~| cannot move out
Foo::Foo1(num1, //~ NOTE to prevent move
num2) => (), //~ NOTE and here
Foo::Foo2(num) => (), //~ NOTE and here
Foo::Foo3 => ()
@ -38,8 +38,8 @@ impl Drop for S {
fn move_in_match() {
match (S {f: "foo".to_string(), g: "bar".to_string()}) {
S { //~ ERROR cannot move out of type `S`, which defines the `Drop` trait
//~| can not move out of here
f: _s, //~ NOTE attempting to move value to here
//~| cannot move out of here
f: _s, //~ NOTE to prevent move
g: _t //~ NOTE and here
} => {}
}
@ -55,8 +55,8 @@ fn free<T>(_: T) {}
fn blah2() {
let a = &A { a: box 1 };
match a.a { //~ ERROR cannot move out of
//~| move occurs here
n => { //~ NOTE attempting to move value to here
//~| cannot move out
n => { //~ NOTE to prevent move
free(n)
}
}

View File

@ -29,8 +29,8 @@ pub fn main() {
match tail {
[Foo { string: a },
//~^ ERROR cannot move out of borrowed content
//~| move occurs here
//~| attempting to move value to here
//~| cannot move out
//~| to prevent move
Foo { string: b }] => {
//~^ NOTE and here
}

View File

@ -19,7 +19,7 @@ fn a() {
[box ref _a, _, _] => {
//~^ borrow of `vec[..]` occurs here
vec[0] = box 4; //~ ERROR cannot assign
//~^ assignment to `vec[..]` occurs here
//~^ assignment to borrowed `vec[..]` occurs here
}
}
}
@ -31,7 +31,7 @@ fn b() {
[_b..] => {
//~^ borrow of `vec[..]` occurs here
vec[0] = box 4; //~ ERROR cannot assign
//~^ assignment to `vec[..]` occurs here
//~^ assignment to borrowed `vec[..]` occurs here
}
}
}
@ -41,8 +41,8 @@ fn c() {
let vec: &mut [Box<isize>] = &mut vec;
match vec {
[_a, //~ ERROR cannot move out
//~| move occurs here
//~| attempting to move value to here
//~| cannot move out
//~| to prevent move
_b..] => {
// Note: `_a` is *moved* here, but `b` is borrowing,
// hence illegal.
@ -53,8 +53,8 @@ fn c() {
_ => {}
}
let a = vec[0]; //~ ERROR cannot move out
//~^ NOTE attempting to move value to here
//~| can not move out of here
//~^ NOTE to prevent move
//~| cannot move out of here
}
fn d() {
@ -62,13 +62,13 @@ fn d() {
let vec: &mut [Box<isize>] = &mut vec;
match vec {
[_a.., //~ ERROR cannot move out
//~^ move occurs here
_b] => {} //~ NOTE attempting to move value to here
//~^ cannot move out
_b] => {} //~ NOTE to prevent move
_ => {}
}
let a = vec[0]; //~ ERROR cannot move out
//~^ NOTE attempting to move value to here
//~| can not move out of here
//~^ NOTE to prevent move
//~| cannot move out of here
}
fn e() {
@ -76,15 +76,15 @@ fn e() {
let vec: &mut [Box<isize>] = &mut vec;
match vec {
[_a, _b, _c] => {} //~ ERROR cannot move out
//~| move occurs here
//~| NOTE attempting to move value to here
//~| cannot move out
//~| NOTE to prevent move
//~| NOTE and here
//~| NOTE and here
_ => {}
}
let a = vec[0]; //~ ERROR cannot move out
//~^ NOTE attempting to move value to here
//~| can not move out of here
//~^ NOTE to prevent move
//~| cannot move out of here
}
fn main() {}

View File

@ -26,6 +26,8 @@ macro_rules! write {
$arr.len() * size_of($arr[0]));
//~^ ERROR mismatched types
//~| expected u64, found usize
//~| expected type
//~| found type
}
}}
}
@ -38,6 +40,8 @@ fn main() {
let hello = ['H', 'e', 'y'];
write!(hello);
//~^ NOTE in this expansion of write!
//~| NOTE in this expansion of write!
//~| NOTE in this expansion of write!
cast!(2);
//~^ NOTE in this expansion of cast!

View File

@ -32,9 +32,10 @@ fn main() {
loop {
f(&s, |hellothere| {
match hellothere.x { //~ ERROR cannot move out
//~| move occurs here
//~| cannot move out of borrowed content
box E::Foo(_) => {}
box E::Bar(x) => println!("{}", x.to_string()), //~ NOTE attempting to move value to here
box E::Bar(x) => println!("{}", x.to_string()),
//~^ NOTE to prevent move
box E::Baz => {}
}
})

View File

@ -12,6 +12,7 @@ use errors::{Error, ErrorKind};
use rustc_serialize::json;
use std::str::FromStr;
use std::path::Path;
use runtest::{ProcRes};
// These structs are a subset of the ones found in
// `syntax::errors::json`.
@ -55,13 +56,13 @@ struct DiagnosticCode {
explanation: Option<String>,
}
pub fn parse_output(file_name: &str, output: &str) -> Vec<Error> {
pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
output.lines()
.flat_map(|line| parse_line(file_name, line))
.flat_map(|line| parse_line(file_name, line, output, proc_res))
.collect()
}
fn parse_line(file_name: &str, line: &str) -> Vec<Error> {
fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
// The compiler sometimes intermingles non-JSON stuff into the
// output. This hack just skips over such lines. Yuck.
if line.chars().next() == Some('{') {
@ -72,7 +73,9 @@ fn parse_line(file_name: &str, line: &str) -> Vec<Error> {
expected_errors
}
Err(error) => {
panic!("failed to decode compiler output as json: `{}`", error);
proc_res.fatal(Some(&format!(
"failed to decode compiler output as json: `{}`\noutput: {}\nline: {}",
error, line, output)));
}
}
} else {

View File

@ -1003,7 +1003,7 @@ actual:\n\
let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note));
// Parse the JSON output from the compiler and extract out the messages.
let actual_errors = json::parse_output(&file_name, &proc_res.stderr);
let actual_errors = json::parse_output(&file_name, &proc_res.stderr, &proc_res);
let mut unexpected = 0;
let mut not_found = 0;
let mut found = vec![false; expected_errors.len()];
@ -1547,21 +1547,7 @@ actual:\n\
fn fatal_proc_rec(&self, err: &str, proc_res: &ProcRes) -> ! {
self.error(err);
print!("\
status: {}\n\
command: {}\n\
stdout:\n\
------------------------------------------\n\
{}\n\
------------------------------------------\n\
stderr:\n\
------------------------------------------\n\
{}\n\
------------------------------------------\n\
\n",
proc_res.status, proc_res.cmdline, proc_res.stdout,
proc_res.stderr);
panic!();
proc_res.fatal(None);
}
fn _arm_exec_compiled_test(&self, env: Vec<(String, String)>) -> ProcRes {
@ -2211,7 +2197,7 @@ struct ProcArgs {
args: Vec<String>,
}
struct ProcRes {
pub struct ProcRes {
status: Status,
stdout: String,
stderr: String,
@ -2223,6 +2209,29 @@ enum Status {
Normal(ExitStatus),
}
impl ProcRes {
pub fn fatal(&self, err: Option<&str>) -> ! {
if let Some(e) = err {
println!("\nerror: {}", e);
}
print!("\
status: {}\n\
command: {}\n\
stdout:\n\
------------------------------------------\n\
{}\n\
------------------------------------------\n\
stderr:\n\
------------------------------------------\n\
{}\n\
------------------------------------------\n\
\n",
self.status, self.cmdline, self.stdout,
self.stderr);
panic!();
}
}
impl Status {
fn code(&self) -> Option<i32> {
match *self {