From 2cb460e6259af63924a719beaf91c5804dd0a789 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Wed, 25 Sep 2019 22:55:04 +0300 Subject: [PATCH 01/23] Use `saturating_sub` where applicable --- src/librustc_errors/emitter.rs | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 2a89c94652a24..c5b03acb622f6 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -99,8 +99,8 @@ impl Margin { // ``` let mut m = Margin { - whitespace_left: if whitespace_left >= 6 { whitespace_left - 6 } else { 0 }, - span_left: if span_left >= 6 { span_left - 6 } else { 0 }, + whitespace_left: whitespace_left.saturating_sub(6), + span_left: span_left.saturating_sub(6), span_right: span_right + 6, computed_left: 0, computed_right: 0, @@ -167,7 +167,7 @@ impl Margin { } fn right(&self, line_len: usize) -> usize { - if max(line_len, self.computed_left) - self.computed_left <= self.column_width { + if line_len.saturating_sub(self.computed_left) <= self.column_width { line_len } else if self.computed_right > line_len { line_len @@ -941,17 +941,9 @@ impl EmitterWriter { Style::LabelSecondary }; let (pos, col) = if pos == 0 { - (pos + 1, if annotation.end_col + 1 > left { - annotation.end_col + 1 - left - } else { - 0 - }) + (pos + 1, (annotation.end_col + 1).saturating_sub(left)) } else { - (pos + 2, if annotation.start_col > left { - annotation.start_col - left - } else { - 0 - }) + (pos + 2, annotation.start_col.saturating_sub(left)) }; if let Some(ref label) = annotation.label { buffer.puts(line_offset + pos, code_offset + col, &label, style); @@ -991,11 +983,7 @@ impl EmitterWriter { for p in annotation.start_col..annotation.end_col { buffer.putc( line_offset + 1, - if code_offset + p > left { - code_offset + p - left - } else { - 0 - }, + (code_offset + p).saturating_sub(left), underline, style, ); From e9a93be53a6ccec5ba76f17333acf3c198313c61 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Wed, 25 Sep 2019 23:08:09 +0300 Subject: [PATCH 02/23] Use `max` instead of `if`s --- src/librustc_errors/emitter.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index c5b03acb622f6..45c3dd8f4d825 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -169,10 +169,8 @@ impl Margin { fn right(&self, line_len: usize) -> usize { if line_len.saturating_sub(self.computed_left) <= self.column_width { line_len - } else if self.computed_right > line_len { - line_len } else { - self.computed_right + min(line_len, self.computed_right) } } } @@ -797,9 +795,7 @@ impl EmitterWriter { } } } - if line_len < p { - line_len = p; - } + line_len = max(line_len, p); } if line_len != 0 { @@ -1772,9 +1768,7 @@ impl FileWithAnnotatedLines { let mut max_depth = 0; // max overlapping multiline spans for (file, ann) in multiline_annotations { - if ann.depth > max_depth { - max_depth = ann.depth; - } + max_depth = max(max_depth, ann.depth); let mut end_ann = ann.as_end(); if !ann.overlaps_exactly { // avoid output like From 4fc5650d17794c00fbf03597b35979de7094d386 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Wed, 25 Sep 2019 23:13:19 +0300 Subject: [PATCH 03/23] Simplify Unicode-aware trimming --- src/librustc_errors/emitter.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 45c3dd8f4d825..151c06ddbbd4c 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -529,18 +529,21 @@ impl EmitterWriter { let left = margin.left(line_len); let right = margin.right(line_len); // On long lines, we strip the source line, accounting for unicode. - let mut taken = 0; - let code: String = source_string.chars().skip(left).take_while(|ch| { - // Make sure that the trimming on the right will fall within the terminal width. - // FIXME: `unicode_width` sometimes disagrees with terminals on how wide a `char` is. - // For now, just accept that sometimes the code line will be longer than desired. - let next = unicode_width::UnicodeWidthChar::width(*ch).unwrap_or(1); - if taken + next > right - left { - return false; - } - taken += next; - true - }).collect(); + // Make sure that the trimming on the right will fall within the terminal width. + // FIXME: `unicode_width` sometimes disagrees with terminals on how wide a `char` is. + // For now, just accept that sometimes the code line will be longer than desired. + let code: String = source_string.chars().skip(left) + .map(|ch| { + let width = unicode_width::UnicodeWidthChar::width(*ch).unwrap_or(1); + (width, ch) + }) + .scan(0, |len, (width, ch)| { + *len += width; + Some(*len, ch) + }) + .take_while(|&(prefix_len, _ch)| prefix_len <= right - left) + .map(|(_prefix_len, ch)| ch) + .collect(); buffer.puts(line_offset, code_offset, &code, Style::Quotation); if margin.was_cut_left() { // We have stripped some code/whitespace from the beginning, make it clear. From d6327e8f12cc51254d62e6755f67ea580fc4dd21 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Wed, 25 Sep 2019 23:23:19 +0300 Subject: [PATCH 04/23] Use map + sum instead of fold for computing Unicode width --- src/librustc_errors/emitter.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 151c06ddbbd4c..17a5fac6da75e 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -594,9 +594,9 @@ impl EmitterWriter { let left = margin.left(source_string.len()); // Left trim // Account for unicode characters of width !=0 that were removed. - let left = source_string.chars().take(left).fold(0, |acc, ch| { - acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1) - }); + let left = source_string.chars().take(left) + .map(|ch| unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1)) + .sum(); self.draw_line( buffer, @@ -1512,9 +1512,9 @@ impl EmitterWriter { .saturating_sub(part.snippet.trim_start().len()); // ...or trailing spaces. Account for substitutions containing unicode // characters. - let sub_len = part.snippet.trim().chars().fold(0, |acc, ch| { - acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1) - }); + let sub_len = part.snippet.trim().chars() + .map(|ch| unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1)) + .sum(); let underline_start = (span_start_pos + start) as isize + offset; let underline_end = (span_start_pos + start + sub_len) as isize + offset; @@ -1535,9 +1535,9 @@ impl EmitterWriter { } // length of the code after substitution - let full_sub_len = part.snippet.chars().fold(0, |acc, ch| { - acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1) as isize - }); + let full_sub_len = part.snippet.chars() + .map(|ch| acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1)) + .sum() as isize; // length of the code to be substituted let snippet_len = span_end_pos as isize - span_start_pos as isize; From aef169b4e637fe417f05fa50450b90e403023a5f Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Wed, 25 Sep 2019 23:41:27 +0300 Subject: [PATCH 05/23] Use Option::map_or where applicable --- src/librustc_errors/emitter.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 17a5fac6da75e..61bd4899f3c54 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -764,11 +764,7 @@ impl EmitterWriter { annotations_position.push((p, annotation)); for (j, next) in annotations.iter().enumerate() { if j > i { - let l = if let Some(ref label) = next.label { - label.len() + 2 - } else { - 0 - }; + let l = next.label.map_or(0, |label| label.len() + 2); if (overlaps(next, annotation, l) // Do not allow two labels to be in the same // line if they overlap including padding, to // avoid situations like: @@ -1311,13 +1307,12 @@ impl EmitterWriter { for line in &annotated_file.lines { max_line_len = max(max_line_len, annotated_file.file .get_line(line.line_index - 1) - .map(|s| s.len()) - .unwrap_or(0)); + .map_or(0, |s| s.len()); for ann in &line.annotations { span_right_margin = max(span_right_margin, ann.start_col); span_right_margin = max(span_right_margin, ann.end_col); // FIXME: account for labels not in the same line - let label_right = ann.label.as_ref().map(|l| l.len() + 1).unwrap_or(0); + let label_right = ann.label.as_ref().map_or(0, |l| l.len() + 1); label_right_margin = max(label_right_margin, ann.end_col + label_right); } } From 9b447e2f9fb9b0d3366de90b2de72541b195e99d Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Wed, 25 Sep 2019 23:46:33 +0300 Subject: [PATCH 06/23] Unify order of variables in chained comparison --- src/librustc_errors/emitter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 61bd4899f3c54..362e28661d919 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -125,7 +125,7 @@ impl Margin { } else { self.computed_right }; - right < line_len && line_len > self.computed_left + self.column_width + right < line_len && self.computed_left + self.column_width < line_len } fn compute(&mut self, max_line_len: usize) { From f93827f9e446e52619953c3ad2aa6604dfc9f986 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Thu, 26 Sep 2019 00:31:16 +0300 Subject: [PATCH 07/23] Simplify `Emitter::fix_multispan_in_std_macros` 1. Rewrite `if let` into `match` to return earl and avoid indenting giant block 2. Assign `spans_updated` only once --- src/librustc_errors/emitter.rs | 129 +++++++++++++++++---------------- 1 file changed, 65 insertions(+), 64 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 362e28661d919..efb5c63bd0847 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -295,81 +295,82 @@ pub trait Emitter { source_map: &Option>, span: &mut MultiSpan, always_backtrace: bool) -> bool { - let mut spans_updated = false; + let sm = match source_map { + Some(ref sm) => sm, + None => return false, + }; - if let Some(ref sm) = source_map { - let mut before_after: Vec<(Span, Span)> = vec![]; - let mut new_labels: Vec<(Span, String)> = vec![]; + let mut before_after: Vec<(Span, Span)> = vec![]; + let mut new_labels: Vec<(Span, String)> = vec![]; - // First, find all the spans in <*macros> and point instead at their use site - for sp in span.primary_spans() { - if sp.is_dummy() { + // First, find all the spans in <*macros> and point instead at their use site + for sp in span.primary_spans() { + if sp.is_dummy() { + continue; + } + let call_sp = sm.call_span_if_macro(*sp); + if call_sp != *sp && !always_backtrace { + before_after.push((*sp, call_sp)); + } + let backtrace_len = sp.macro_backtrace().len(); + for (i, trace) in sp.macro_backtrace().iter().rev().enumerate() { + // Only show macro locations that are local + // and display them like a span_note + if trace.def_site_span.is_dummy() { continue; } - let call_sp = sm.call_span_if_macro(*sp); - if call_sp != *sp && !always_backtrace { - before_after.push((*sp, call_sp)); + if always_backtrace { + new_labels.push((trace.def_site_span, + format!("in this expansion of `{}`{}", + trace.macro_decl_name, + if backtrace_len > 2 { + // if backtrace_len == 1 it'll be pointed + // at by "in this macro invocation" + format!(" (#{})", i + 1) + } else { + String::new() + }))); } - let backtrace_len = sp.macro_backtrace().len(); - for (i, trace) in sp.macro_backtrace().iter().rev().enumerate() { - // Only show macro locations that are local - // and display them like a span_note - if trace.def_site_span.is_dummy() { - continue; - } - if always_backtrace { - new_labels.push((trace.def_site_span, - format!("in this expansion of `{}`{}", - trace.macro_decl_name, - if backtrace_len > 2 { - // if backtrace_len == 1 it'll be pointed - // at by "in this macro invocation" - format!(" (#{})", i + 1) - } else { - String::new() - }))); - } - // Check to make sure we're not in any <*macros> - if !sm.span_to_filename(trace.def_site_span).is_macros() && - !trace.macro_decl_name.starts_with("desugaring of ") && - !trace.macro_decl_name.starts_with("#[") || - always_backtrace { - new_labels.push((trace.call_site, - format!("in this macro invocation{}", - if backtrace_len > 2 && always_backtrace { - // only specify order when the macro - // backtrace is multiple levels deep - format!(" (#{})", i + 1) - } else { - String::new() - }))); - if !always_backtrace { - break; - } + // Check to make sure we're not in any <*macros> + if !sm.span_to_filename(trace.def_site_span).is_macros() && + !trace.macro_decl_name.starts_with("desugaring of ") && + !trace.macro_decl_name.starts_with("#[") || + always_backtrace { + new_labels.push((trace.call_site, + format!("in this macro invocation{}", + if backtrace_len > 2 && always_backtrace { + // only specify order when the macro + // backtrace is multiple levels deep + format!(" (#{})", i + 1) + } else { + String::new() + }))); + if !always_backtrace { + break; } } } - for (label_span, label_text) in new_labels { - span.push_span_label(label_span, label_text); + } + for (label_span, label_text) in new_labels { + span.push_span_label(label_span, label_text); + } + for sp_label in span.span_labels() { + if sp_label.span.is_dummy() { + continue; } - for sp_label in span.span_labels() { - if sp_label.span.is_dummy() { - continue; - } - if sm.span_to_filename(sp_label.span.clone()).is_macros() && - !always_backtrace - { - let v = sp_label.span.macro_backtrace(); - if let Some(use_site) = v.last() { - before_after.push((sp_label.span.clone(), use_site.call_site.clone())); - } + if sm.span_to_filename(sp_label.span.clone()).is_macros() && + !always_backtrace + { + let v = sp_label.span.macro_backtrace(); + if let Some(use_site) = v.last() { + before_after.push((sp_label.span.clone(), use_site.call_site.clone())); } } - // After we have them, make sure we replace these 'bad' def sites with their use sites - for (before, after) in before_after { - span.replace(before, after); - spans_updated = true; - } + } + // After we have them, make sure we replace these 'bad' def sites with their use sites + let spans_updated = !before_after.is_empty(); + for (before, after) in before_after { + span.replace(before, after); } spans_updated From ea328625039229f9ad0002a68f1a86691a065cf6 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Thu, 26 Sep 2019 00:38:36 +0300 Subject: [PATCH 08/23] Use `sort_by_key` rather than `sort_by` --- src/librustc_errors/emitter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index efb5c63bd0847..7513a9203fe25 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -954,9 +954,9 @@ impl EmitterWriter { // | | | // | | something about `foo` // | something about `fn foo()` - annotations_position.sort_by(|a, b| { + annotations_position.sort_by_key(|x| { // Decreasing order. When `a` and `b` are the same length, prefer `Primary`. - (a.1.len(), !a.1.is_primary).cmp(&(b.1.len(), !b.1.is_primary)).reverse() + (Reverse(a.1.len()), a.1.is_primary) }); // Write the underlines. From 7a0725fdaf964fa1bf60eefb0cc5846b687426bf Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Thu, 26 Sep 2019 00:42:55 +0300 Subject: [PATCH 09/23] Simplify `style_or_override` --- src/librustc_errors/emitter.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 7513a9203fe25..b8b7488d8ae45 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -1066,13 +1066,11 @@ impl EmitterWriter { let padding = " ".repeat(padding + label.len() + 5); /// Returns `true` if `style`, or the override if present and the style is `NoStyle`. - fn style_or_override(style: Style, override_style: Option