Skip to content

Commit b7982bb

Browse files
estebankestebank
committed
review comments
1 parent eb53ca3 commit b7982bb

7 files changed

+191
-114
lines changed

‎src/librustc_errors/emitter.rs

+28-54
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use syntax_pos::{COMMAND_LINE_SP, DUMMY_SP, FileMap, Span, MultiSpan, CharPos};
1414

1515
use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapper};
1616
use RenderSpan::*;
17-
use snippet::{Annotation, AnnotationType, Line, StyledString, Style};
17+
use snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style};
1818
use styled_buffer::StyledBuffer;
1919

2020
use std::io::prelude::*;
@@ -181,12 +181,17 @@ impl EmitterWriter {
181181
if is_minimized {
182182
ann.annotation_type = AnnotationType::Minimized;
183183
} else if lo.line != hi.line {
184-
ann.annotation_type = AnnotationType::Multiline {
184+
let ml = MultilineAnnotation {
185185
depth: 1,
186186
line_start: lo.line,
187187
line_end: hi.line,
188+
start_col: lo.col.0,
189+
end_col: hi.col.0,
190+
is_primary: span_label.is_primary,
191+
label: span_label.label.clone(),
188192
};
189-
multiline_annotations.push((lo.file.clone(), ann.clone()));
193+
ann.annotation_type = AnnotationType::Multiline(ml.clone());
194+
multiline_annotations.push((lo.file.clone(), ml));
190195
};
191196

192197
if !ann.is_multiline() {
@@ -200,65 +205,34 @@ impl EmitterWriter {
200205

201206
// Find overlapping multiline annotations, put them at different depths
202207
multiline_annotations.sort_by(|a, b| {
203-
if let AnnotationType::Multiline {
204-
line_start: a_start,
205-
line_end: a_end,
206-
..
207-
} = a.1.annotation_type {
208-
if let AnnotationType::Multiline {
209-
line_start: b_start,
210-
line_end: b_end,
211-
..
212-
} = b.1.annotation_type {
213-
(a_start, a_end).cmp(&(b_start, b_end))
214-
} else {
215-
panic!("tried to sort multiline annotations, but found `{:?}`", b)
216-
}
217-
} else {
218-
panic!("tried to sort multiline annotations, but found `{:?}`", a)
219-
}
208+
(a.1.line_start, a.1.line_end).cmp(&(b.1.line_start, b.1.line_end))
220209
});
221210
for item in multiline_annotations.clone() {
222211
let ann = item.1;
223-
if let AnnotationType::Multiline {line_start, line_end, ..} = ann.annotation_type {
224-
for item in multiline_annotations.iter_mut() {
225-
let ref mut a = item.1;
226-
if let AnnotationType::Multiline {
227-
line_start: start,
228-
line_end: end,
229-
..
230-
} = a.annotation_type {
231-
// Move all other multiline annotations overlapping with this one
232-
// one level to the right.
233-
if &ann != a && num_overlap(line_start, line_end, start, end, true) {
234-
a.annotation_type.increase_depth();
235-
} else {
236-
break;
237-
}
238-
} else {
239-
panic!("tried to find depth for multiline annotation, but found `{:?}`",
240-
ann)
241-
};
212+
for item in multiline_annotations.iter_mut() {
213+
let ref mut a = item.1;
214+
// Move all other multiline annotations overlapping with this one
215+
// one level to the right.
216+
if &ann != a &&
217+
num_overlap(ann.line_start, ann.line_end, a.line_start, a.line_end, true)
218+
{
219+
a.increase_depth();
220+
} else {
221+
break;
242222
}
243-
} else {
244-
panic!("tried to find depth for multiline annotation, but found `{:?}`", ann)
245-
};
223+
}
246224
}
247225

248226
let mut max_depth = 0; // max overlapping multiline spans
249227
for (file, ann) in multiline_annotations {
250-
if let AnnotationType::Multiline {line_start, line_end, depth} = ann.annotation_type {
251-
if depth > max_depth {
252-
max_depth = depth;
253-
}
254-
add_annotation_to_file(&mut output, file.clone(), line_start, ann.as_start());
255-
for line in line_start + 1..line_end {
256-
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
257-
}
258-
add_annotation_to_file(&mut output, file, line_end, ann.as_end());
259-
} else {
260-
panic!("non-multiline annotation `{:?}` in `multiline_annotations`!", ann);
228+
if ann.depth > max_depth {
229+
max_depth = ann.depth;
230+
}
231+
add_annotation_to_file(&mut output, file.clone(), ann.line_start, ann.as_start());
232+
for line in ann.line_start + 1..ann.line_end {
233+
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
261234
}
235+
add_annotation_to_file(&mut output, file, ann.line_end, ann.as_end());
262236
}
263237
for file_vec in output.iter_mut() {
264238
file_vec.multiline_depth = max_depth;
@@ -572,7 +546,7 @@ impl EmitterWriter {
572546
// | | something about `foo`
573547
// | something about `fn foo()`
574548
annotations_position.sort_by(|a, b| {
575-
fn len(a: Annotation) -> usize {
549+
fn len(a: &Annotation) -> usize {
576550
// Account for usize underflows
577551
if a.end_col > a.start_col {
578552
a.end_col - a.start_col

‎src/librustc_errors/snippet.rs

+56-52
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,57 @@ pub struct Line {
4141
pub annotations: Vec<Annotation>,
4242
}
4343

44+
45+
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
46+
pub struct MultilineAnnotation {
47+
pub depth: usize,
48+
pub line_start: usize,
49+
pub line_end: usize,
50+
pub start_col: usize,
51+
pub end_col: usize,
52+
pub is_primary: bool,
53+
pub label: Option<String>,
54+
}
55+
56+
impl MultilineAnnotation {
57+
pub fn increase_depth(&mut self) {
58+
self.depth += 1;
59+
}
60+
61+
pub fn as_start(&self) -> Annotation {
62+
Annotation {
63+
start_col: self.start_col,
64+
end_col: self.start_col + 1,
65+
is_primary: self.is_primary,
66+
label: Some("starting here...".to_owned()),
67+
annotation_type: AnnotationType::MultilineStart(self.depth)
68+
}
69+
}
70+
71+
pub fn as_end(&self) -> Annotation {
72+
Annotation {
73+
start_col: self.end_col - 1,
74+
end_col: self.end_col,
75+
is_primary: self.is_primary,
76+
label: match self.label {
77+
Some(ref label) => Some(format!("...ending here: {}", label)),
78+
None => Some("...ending here".to_owned()),
79+
},
80+
annotation_type: AnnotationType::MultilineEnd(self.depth)
81+
}
82+
}
83+
84+
pub fn as_line(&self) -> Annotation {
85+
Annotation {
86+
start_col: 0,
87+
end_col: 0,
88+
is_primary: self.is_primary,
89+
label: None,
90+
annotation_type: AnnotationType::MultilineLine(self.depth)
91+
}
92+
}
93+
}
94+
4495
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
4596
pub enum AnnotationType {
4697
/// Annotation under a single line of code
@@ -50,11 +101,7 @@ pub enum AnnotationType {
50101
Minimized,
51102

52103
/// Annotation enclosing the first and last character of a multiline span
53-
Multiline {
54-
depth: usize,
55-
line_start: usize,
56-
line_end: usize,
57-
},
104+
Multiline(MultilineAnnotation),
58105

59106
// The Multiline type above is replaced with the following three in order
60107
// to reuse the current label drawing code.
@@ -74,24 +121,6 @@ pub enum AnnotationType {
74121
MultilineLine(usize),
75122
}
76123

77-
impl AnnotationType {
78-
pub fn depth(&self) -> usize {
79-
match self {
80-
&AnnotationType::Multiline {depth, ..} |
81-
&AnnotationType::MultilineStart(depth) |
82-
&AnnotationType::MultilineLine(depth) |
83-
&AnnotationType::MultilineEnd(depth) => depth,
84-
_ => 0,
85-
}
86-
}
87-
88-
pub fn increase_depth(&mut self) {
89-
if let AnnotationType::Multiline {ref mut depth, ..} = *self {
90-
*depth += 1;
91-
}
92-
}
93-
}
94-
95124
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
96125
pub struct Annotation {
97126
/// Start column, 0-based indexing -- counting *characters*, not
@@ -124,39 +153,14 @@ impl Annotation {
124153

125154
pub fn is_multiline(&self) -> bool {
126155
match self.annotation_type {
127-
AnnotationType::Multiline {..} |
128-
AnnotationType::MultilineStart(_) |
129-
AnnotationType::MultilineLine(_) |
130-
AnnotationType::MultilineEnd(_) => true,
156+
AnnotationType::Multiline(_) |
157+
AnnotationType::MultilineStart(_) |
158+
AnnotationType::MultilineLine(_) |
159+
AnnotationType::MultilineEnd(_) => true,
131160
_ => false,
132161
}
133162
}
134163

135-
pub fn as_start(&self) -> Annotation {
136-
let mut a = self.clone();
137-
a.annotation_type = AnnotationType::MultilineStart(self.annotation_type.depth());
138-
a.end_col = a.start_col + 1;
139-
a.label = Some("starting here...".to_owned());
140-
a
141-
}
142-
143-
pub fn as_end(&self) -> Annotation {
144-
let mut a = self.clone();
145-
a.annotation_type = AnnotationType::MultilineEnd(self.annotation_type.depth());
146-
a.start_col = a.end_col - 1;
147-
a.label = match a.label {
148-
Some(l) => Some(format!("...ending here: {}", l)),
149-
None => Some("..ending here".to_owned()),
150-
};
151-
a
152-
}
153-
154-
pub fn as_line(&self) -> Annotation {
155-
let mut a = self.clone();
156-
a.annotation_type = AnnotationType::MultilineLine(self.annotation_type.depth());
157-
a.label = None;
158-
a
159-
}
160164
}
161165

162166
#[derive(Debug)]

‎src/libsyntax/test_snippet.rs

+100
Original file line numberDiff line numberDiff line change
@@ -444,3 +444,103 @@ error: foo
444444
445445
"#);
446446
}
447+
448+
#[test]
449+
fn non_overlaping() {
450+
test_harness(r#"
451+
fn foo() {
452+
X0 Y0 Z0
453+
X1 Y1 Z1
454+
X2 Y2 Z2
455+
X3 Y3 Z3
456+
}
457+
"#,
458+
vec![
459+
SpanLabel {
460+
start: Position {
461+
string: "Y0",
462+
count: 1,
463+
},
464+
end: Position {
465+
string: "X1",
466+
count: 1,
467+
},
468+
label: "`X` is a good letter",
469+
},
470+
SpanLabel {
471+
start: Position {
472+
string: "Y2",
473+
count: 1,
474+
},
475+
end: Position {
476+
string: "Z3",
477+
count: 1,
478+
},
479+
label: "`Y` is a good letter too",
480+
},
481+
],
482+
r#"
483+
error: foo
484+
--> test.rs:3:6
485+
|
486+
3 | X0 Y0 Z0
487+
| ______^ starting here...
488+
4 | | X1 Y1 Z1
489+
| |____^ ...ending here: `X` is a good letter
490+
5 | X2 Y2 Z2
491+
| ______- starting here...
492+
6 | | X3 Y3 Z3
493+
| |__________- ...ending here: `Y` is a good letter too
494+
495+
"#);
496+
}
497+
#[test]
498+
fn overlaping_start_and_end() {
499+
test_harness(r#"
500+
fn foo() {
501+
X0 Y0 Z0
502+
X1 Y1 Z1
503+
X2 Y2 Z2
504+
X3 Y3 Z3
505+
}
506+
"#,
507+
vec![
508+
SpanLabel {
509+
start: Position {
510+
string: "Y0",
511+
count: 1,
512+
},
513+
end: Position {
514+
string: "X1",
515+
count: 1,
516+
},
517+
label: "`X` is a good letter",
518+
},
519+
SpanLabel {
520+
start: Position {
521+
string: "Z1",
522+
count: 1,
523+
},
524+
end: Position {
525+
string: "Z3",
526+
count: 1,
527+
},
528+
label: "`Y` is a good letter too",
529+
},
530+
],
531+
r#"
532+
error: foo
533+
--> test.rs:3:6
534+
|
535+
3 | X0 Y0 Z0
536+
| ______^ starting here...
537+
4 | | X1 Y1 Z1
538+
| |____^____- starting here...
539+
| ||____|
540+
| | ...ending here: `X` is a good letter
541+
5 | | X2 Y2 Z2
542+
6 | | X3 Y3 Z3
543+
| |___________- ...ending here: `Y` is a good letter too
544+
545+
"#);
546+
}

‎src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attri
88
35 | | // (unsafe to access self.1 due to #[may_dangle] on A)
99
36 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
1010
37 | | }
11-
| |_^ ..ending here
11+
| |_^ ...ending here
1212

1313
error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute
1414
--> $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
2020
41 | | // (unsafe to access self.1 due to #[may_dangle] on 'a)
2121
42 | | fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
2222
43 | | }
23-
| |_^ ..ending here
23+
| |_^ ...ending here
2424

2525
error: aborting due to 2 previous errors
2626

‎src/test/ui/lifetimes/consider-using-explicit-lifetime.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ help: consider using an explicit lifetime parameter as shown: fn from_str(path:
1919
| _____^ starting here...
2020
26 | | Ok(Foo { field: path })
2121
27 | | }
22-
| |_____^ ..ending here
22+
| |_____^ ...ending here
2323

2424
error: aborting due to 2 previous errors
2525

0 commit comments

Comments
 (0)