review comments

7 files changed, 191 insertions, 114 deletions

diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
index 808a1683b84..808fe504b95 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 3bf428af994..b8c1726443d 100644
--- a/src/librustc_errors/snippet.rs
+++ b/src/librustc_errors/snippet.rs
@@ -41,6 +41,57 @@ pub struct Line {
     pub annotations: Vec<Annotation>,
 }
 
+
+#[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<String>,
+}
+
+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 4ce51076adc..98e574867b4 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 b3e72f28d88..92e2fe8e936 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 1d1bc58805a..153aaa07833 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 16414766f39..451492ba693 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 26acef64c89..b801325114c 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:   <u32 as std::ops::Add>
    = help:   <&'a u32 as std::ops::Add<u32>>
    = help:   <u32 as std::ops::Add<&'a u32>>
    = help:   <&'b u32 as std::ops::Add<&'a u32>>
-   = help: and 90 others
 
 error: aborting due to previous error