From b7982bbbe0a002272b86ed2f7f7902b2c3471087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 22 Nov 2016 23:32:16 -0800 Subject: [PATCH] review comments --- src/librustc_errors/emitter.rs | 82 +++++-------- src/librustc_errors/snippet.rs | 108 +++++++++--------- src/libsyntax/test_snippet.rs | 100 ++++++++++++++++ ...dropck-eyepatch-implies-unsafe-impl.stderr | 4 +- .../consider-using-explicit-lifetime.stderr | 2 +- src/test/ui/span/multiline-span-simple.rs | 2 +- src/test/ui/span/multiline-span-simple.stderr | 7 +- 7 files changed, 191 insertions(+), 114 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 808a1683b843..808fe504b95c 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -14,7 +14,7 @@ use syntax_pos::{COMMAND_LINE_SP, DUMMY_SP, FileMap, Span, MultiSpan, CharPos}; use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapper}; use RenderSpan::*; -use snippet::{Annotation, AnnotationType, Line, StyledString, Style}; +use snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style}; use styled_buffer::StyledBuffer; use std::io::prelude::*; @@ -181,12 +181,17 @@ impl EmitterWriter { if is_minimized { ann.annotation_type = AnnotationType::Minimized; } else if lo.line != hi.line { - ann.annotation_type = AnnotationType::Multiline { + let ml = MultilineAnnotation { depth: 1, line_start: lo.line, line_end: hi.line, + start_col: lo.col.0, + end_col: hi.col.0, + is_primary: span_label.is_primary, + label: span_label.label.clone(), }; - multiline_annotations.push((lo.file.clone(), ann.clone())); + ann.annotation_type = AnnotationType::Multiline(ml.clone()); + multiline_annotations.push((lo.file.clone(), ml)); }; if !ann.is_multiline() { @@ -200,65 +205,34 @@ impl EmitterWriter { // Find overlapping multiline annotations, put them at different depths multiline_annotations.sort_by(|a, b| { - if let AnnotationType::Multiline { - line_start: a_start, - line_end: a_end, - .. - } = a.1.annotation_type { - if let AnnotationType::Multiline { - line_start: b_start, - line_end: b_end, - .. - } = b.1.annotation_type { - (a_start, a_end).cmp(&(b_start, b_end)) - } else { - panic!("tried to sort multiline annotations, but found `{:?}`", b) - } - } else { - panic!("tried to sort multiline annotations, but found `{:?}`", a) - } + (a.1.line_start, a.1.line_end).cmp(&(b.1.line_start, b.1.line_end)) }); for item in multiline_annotations.clone() { let ann = item.1; - if let AnnotationType::Multiline {line_start, line_end, ..} = ann.annotation_type { - for item in multiline_annotations.iter_mut() { - let ref mut a = item.1; - if let AnnotationType::Multiline { - line_start: start, - line_end: end, - .. - } = a.annotation_type { - // Move all other multiline annotations overlapping with this one - // one level to the right. - if &ann != a && num_overlap(line_start, line_end, start, end, true) { - a.annotation_type.increase_depth(); - } else { - break; - } - } else { - panic!("tried to find depth for multiline annotation, but found `{:?}`", - ann) - }; + for item in multiline_annotations.iter_mut() { + let ref mut a = item.1; + // Move all other multiline annotations overlapping with this one + // one level to the right. + if &ann != a && + num_overlap(ann.line_start, ann.line_end, a.line_start, a.line_end, true) + { + a.increase_depth(); + } else { + break; } - } else { - panic!("tried to find depth for multiline annotation, but found `{:?}`", ann) - }; + } } let mut max_depth = 0; // max overlapping multiline spans for (file, ann) in multiline_annotations { - if let AnnotationType::Multiline {line_start, line_end, depth} = ann.annotation_type { - if depth > max_depth { - max_depth = depth; - } - add_annotation_to_file(&mut output, file.clone(), line_start, ann.as_start()); - for line in line_start + 1..line_end { - add_annotation_to_file(&mut output, file.clone(), line, ann.as_line()); - } - add_annotation_to_file(&mut output, file, line_end, ann.as_end()); - } else { - panic!("non-multiline annotation `{:?}` in `multiline_annotations`!", ann); + if ann.depth > max_depth { + max_depth = ann.depth; } + add_annotation_to_file(&mut output, file.clone(), ann.line_start, ann.as_start()); + for line in ann.line_start + 1..ann.line_end { + add_annotation_to_file(&mut output, file.clone(), line, ann.as_line()); + } + add_annotation_to_file(&mut output, file, ann.line_end, ann.as_end()); } for file_vec in output.iter_mut() { file_vec.multiline_depth = max_depth; @@ -572,7 +546,7 @@ impl EmitterWriter { // | | something about `foo` // | something about `fn foo()` annotations_position.sort_by(|a, b| { - fn len(a: Annotation) -> usize { + fn len(a: &Annotation) -> usize { // Account for usize underflows if a.end_col > a.start_col { a.end_col - a.start_col diff --git a/src/librustc_errors/snippet.rs b/src/librustc_errors/snippet.rs index 3bf428af994a..b8c1726443db 100644 --- a/src/librustc_errors/snippet.rs +++ b/src/librustc_errors/snippet.rs @@ -41,6 +41,57 @@ pub struct Line { pub annotations: Vec, } + +#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] +pub struct MultilineAnnotation { + pub depth: usize, + pub line_start: usize, + pub line_end: usize, + pub start_col: usize, + pub end_col: usize, + pub is_primary: bool, + pub label: Option, +} + +impl MultilineAnnotation { + pub fn increase_depth(&mut self) { + self.depth += 1; + } + + pub fn as_start(&self) -> Annotation { + Annotation { + start_col: self.start_col, + end_col: self.start_col + 1, + is_primary: self.is_primary, + label: Some("starting here...".to_owned()), + annotation_type: AnnotationType::MultilineStart(self.depth) + } + } + + pub fn as_end(&self) -> Annotation { + Annotation { + start_col: self.end_col - 1, + end_col: self.end_col, + is_primary: self.is_primary, + label: match self.label { + Some(ref label) => Some(format!("...ending here: {}", label)), + None => Some("...ending here".to_owned()), + }, + annotation_type: AnnotationType::MultilineEnd(self.depth) + } + } + + pub fn as_line(&self) -> Annotation { + Annotation { + start_col: 0, + end_col: 0, + is_primary: self.is_primary, + label: None, + annotation_type: AnnotationType::MultilineLine(self.depth) + } + } +} + #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] pub enum AnnotationType { /// Annotation under a single line of code @@ -50,11 +101,7 @@ pub enum AnnotationType { Minimized, /// Annotation enclosing the first and last character of a multiline span - Multiline { - depth: usize, - line_start: usize, - line_end: usize, - }, + Multiline(MultilineAnnotation), // The Multiline type above is replaced with the following three in order // to reuse the current label drawing code. @@ -74,24 +121,6 @@ pub enum AnnotationType { MultilineLine(usize), } -impl AnnotationType { - pub fn depth(&self) -> usize { - match self { - &AnnotationType::Multiline {depth, ..} | - &AnnotationType::MultilineStart(depth) | - &AnnotationType::MultilineLine(depth) | - &AnnotationType::MultilineEnd(depth) => depth, - _ => 0, - } - } - - pub fn increase_depth(&mut self) { - if let AnnotationType::Multiline {ref mut depth, ..} = *self { - *depth += 1; - } - } -} - #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] pub struct Annotation { /// Start column, 0-based indexing -- counting *characters*, not @@ -124,39 +153,14 @@ impl Annotation { pub fn is_multiline(&self) -> bool { match self.annotation_type { - AnnotationType::Multiline {..} | - AnnotationType::MultilineStart(_) | - AnnotationType::MultilineLine(_) | - AnnotationType::MultilineEnd(_) => true, + AnnotationType::Multiline(_) | + AnnotationType::MultilineStart(_) | + AnnotationType::MultilineLine(_) | + AnnotationType::MultilineEnd(_) => true, _ => false, } } - pub fn as_start(&self) -> Annotation { - let mut a = self.clone(); - a.annotation_type = AnnotationType::MultilineStart(self.annotation_type.depth()); - a.end_col = a.start_col + 1; - a.label = Some("starting here...".to_owned()); - a - } - - pub fn as_end(&self) -> Annotation { - let mut a = self.clone(); - a.annotation_type = AnnotationType::MultilineEnd(self.annotation_type.depth()); - a.start_col = a.end_col - 1; - a.label = match a.label { - Some(l) => Some(format!("...ending here: {}", l)), - None => Some("..ending here".to_owned()), - }; - a - } - - pub fn as_line(&self) -> Annotation { - let mut a = self.clone(); - a.annotation_type = AnnotationType::MultilineLine(self.annotation_type.depth()); - a.label = None; - a - } } #[derive(Debug)] diff --git a/src/libsyntax/test_snippet.rs b/src/libsyntax/test_snippet.rs index 4ce51076adcf..98e574867b49 100644 --- a/src/libsyntax/test_snippet.rs +++ b/src/libsyntax/test_snippet.rs @@ -444,3 +444,103 @@ error: foo "#); } + +#[test] +fn non_overlaping() { + test_harness(r#" +fn foo() { + X0 Y0 Z0 + X1 Y1 Z1 + X2 Y2 Z2 + X3 Y3 Z3 +} +"#, + vec![ + SpanLabel { + start: Position { + string: "Y0", + count: 1, + }, + end: Position { + string: "X1", + count: 1, + }, + label: "`X` is a good letter", + }, + SpanLabel { + start: Position { + string: "Y2", + count: 1, + }, + end: Position { + string: "Z3", + count: 1, + }, + label: "`Y` is a good letter too", + }, + ], + r#" +error: foo + --> test.rs:3:6 + | +3 | X0 Y0 Z0 + | ______^ starting here... +4 | | X1 Y1 Z1 + | |____^ ...ending here: `X` is a good letter +5 | X2 Y2 Z2 + | ______- starting here... +6 | | X3 Y3 Z3 + | |__________- ...ending here: `Y` is a good letter too + +"#); +} +#[test] +fn overlaping_start_and_end() { + test_harness(r#" +fn foo() { + X0 Y0 Z0 + X1 Y1 Z1 + X2 Y2 Z2 + X3 Y3 Z3 +} +"#, + vec![ + SpanLabel { + start: Position { + string: "Y0", + count: 1, + }, + end: Position { + string: "X1", + count: 1, + }, + label: "`X` is a good letter", + }, + SpanLabel { + start: Position { + string: "Z1", + count: 1, + }, + end: Position { + string: "Z3", + count: 1, + }, + label: "`Y` is a good letter too", + }, + ], + r#" +error: foo + --> test.rs:3:6 + | +3 | X0 Y0 Z0 + | ______^ starting here... +4 | | X1 Y1 Z1 + | |____^____- starting here... + | ||____| + | | ...ending here: `X` is a good letter +5 | | X2 Y2 Z2 +6 | | X3 Y3 Z3 + | |___________- ...ending here: `Y` is a good letter too + +"#); +} diff --git a/src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr b/src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr index b3e72f28d88c..92e2fe8e9367 100644 --- a/src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr +++ b/src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr @@ -8,7 +8,7 @@ error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attri 35 | | // (unsafe to access self.1 due to #[may_dangle] on A) 36 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); } 37 | | } - | |_^ ..ending here + | |_^ ...ending here error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute --> $DIR/dropck-eyepatch-implies-unsafe-impl.rs:38:1 @@ -20,7 +20,7 @@ error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attri 41 | | // (unsafe to access self.1 due to #[may_dangle] on 'a) 42 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); } 43 | | } - | |_^ ..ending here + | |_^ ...ending here error: aborting due to 2 previous errors diff --git a/src/test/ui/lifetimes/consider-using-explicit-lifetime.stderr b/src/test/ui/lifetimes/consider-using-explicit-lifetime.stderr index 1d1bc58805aa..153aaa07833a 100644 --- a/src/test/ui/lifetimes/consider-using-explicit-lifetime.stderr +++ b/src/test/ui/lifetimes/consider-using-explicit-lifetime.stderr @@ -19,7 +19,7 @@ help: consider using an explicit lifetime parameter as shown: fn from_str(path: | _____^ starting here... 26 | | Ok(Foo { field: path }) 27 | | } - | |_____^ ..ending here + | |_____^ ...ending here error: aborting due to 2 previous errors diff --git a/src/test/ui/span/multiline-span-simple.rs b/src/test/ui/span/multiline-span-simple.rs index 16414766f398..451492ba6930 100644 --- a/src/test/ui/span/multiline-span-simple.rs +++ b/src/test/ui/span/multiline-span-simple.rs @@ -20,7 +20,7 @@ fn main() { let x = 1; let y = 2; let z = 3; - foo(1 + + foo(1 as u32 + bar(x, diff --git a/src/test/ui/span/multiline-span-simple.stderr b/src/test/ui/span/multiline-span-simple.stderr index 26acef64c896..b801325114cc 100644 --- a/src/test/ui/span/multiline-span-simple.stderr +++ b/src/test/ui/span/multiline-span-simple.stderr @@ -1,20 +1,19 @@ -error[E0277]: the trait bound `{integer}: std::ops::Add<()>` is not satisfied +error[E0277]: the trait bound `u32: std::ops::Add<()>` is not satisfied --> $DIR/multiline-span-simple.rs:23:9 | -23 | foo(1 + +23 | foo(1 as u32 + | _________^ starting here... 24 | | 25 | | bar(x, 26 | | 27 | | y), - | |______________^ ...ending here: the trait `std::ops::Add<()>` is not implemented for `{integer}` + | |______________^ ...ending here: the trait `std::ops::Add<()>` is not implemented for `u32` | = help: the following implementations were found: = help: = help: <&'a u32 as std::ops::Add> = help: > = help: <&'b u32 as std::ops::Add<&'a u32>> - = help: and 90 others error: aborting due to previous error