From acf6344b42b86bf63c73632620438fc4836b0d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 10 Nov 2024 17:51:37 +0100 Subject: [PATCH] Address review comments --- compiler/rustc_errors/src/emitter.rs | 51 ++++++++++++++-------------- compiler/rustc_session/src/config.rs | 30 ++++++++-------- compiler/rustc_span/src/lib.rs | 4 +++ 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 120f5ba7d48..06c961c3304 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -22,7 +22,7 @@ use rustc_error_messages::{FluentArgs, SpanLabel}; use rustc_lint_defs::pluralize; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::SourceMap; -use rustc_span::{FileLines, FileName, SourceFile, Span, char_width}; +use rustc_span::{FileLines, FileName, SourceFile, Span, char_width, str_width}; use termcolor::{Buffer, BufferWriter, Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; use tracing::{debug, instrument, trace, warn}; @@ -113,8 +113,12 @@ impl Margin { fn was_cut_right(&self, line_len: usize) -> bool { let right = if self.computed_right == self.span_right || self.computed_right == self.label_right { + // FIXME: This comment refers to the only callsite of this method. + // Rephrase it or refactor it, so it can stand on its own. // Account for the "..." padding given above. Otherwise we end up with code lines // that do fit but end in "..." as if they were trimmed. + // FIXME: Don't hard-code this offset. Is this meant to represent + // `2 * str_width(self.margin())`? self.computed_right - 6 } else { self.computed_right @@ -673,6 +677,7 @@ impl HumanEmitter { // Create the source line we will highlight. let left = margin.left(line_len); let right = margin.right(line_len); + // FIXME: The following code looks fishy. See #132860. // On long lines, we strip the source line, accounting for unicode. let mut taken = 0; let code: String = source_string @@ -695,7 +700,7 @@ impl HumanEmitter { buffer.puts(line_offset, code_offset, placeholder, Style::LineNumber); } if margin.was_cut_right(line_len) { - let padding: usize = placeholder.chars().map(|ch| char_width(ch)).sum(); + let padding = str_width(placeholder); // We have stripped some code after the rightmost span end, make it clear we did so. buffer.puts(line_offset, code_offset + taken - padding, placeholder, Style::LineNumber); } @@ -744,6 +749,7 @@ impl HumanEmitter { // Left trim let left = margin.left(source_string.len()); + // FIXME: This looks fishy. See #132860. // Account for unicode characters of width !=0 that were removed. let left = source_string.chars().take(left).map(|ch| char_width(ch)).sum(); @@ -1068,21 +1074,15 @@ impl HumanEmitter { if pos > 1 && (annotation.has_label() || annotation.takes_space()) { for p in line_offset + 1..=line_offset + pos { - if let AnnotationType::MultilineLine(_) = annotation.annotation_type { - buffer.putc( - p, - (code_offset + annotation.start_col.display).saturating_sub(left), - underline.multiline_vertical, - underline.style, - ); - } else { - buffer.putc( - p, - (code_offset + annotation.start_col.display).saturating_sub(left), - underline.vertical_text_line, - underline.style, - ); - } + buffer.putc( + p, + (code_offset + annotation.start_col.display).saturating_sub(left), + match annotation.annotation_type { + AnnotationType::MultilineLine(_) => underline.multiline_vertical, + _ => underline.vertical_text_line, + }, + underline.style, + ); } if let AnnotationType::MultilineStart(_) = annotation.annotation_type { buffer.putc( @@ -2134,7 +2134,7 @@ impl HumanEmitter { } let placeholder = self.margin(); - let padding: usize = placeholder.chars().map(|ch| char_width(ch)).sum(); + let padding = str_width(placeholder); buffer.puts( row_num, max_line_num_len.saturating_sub(padding), @@ -2224,11 +2224,11 @@ impl HumanEmitter { }; // ...or trailing spaces. Account for substitutions containing unicode // characters. - let sub_len: usize = - if is_whitespace_addition { &part.snippet } else { part.snippet.trim() } - .chars() - .map(|ch| char_width(ch)) - .sum(); + let sub_len: usize = str_width(if is_whitespace_addition { + &part.snippet + } else { + part.snippet.trim() + }); let offset: isize = offsets .iter() @@ -2266,8 +2266,7 @@ impl HumanEmitter { } // length of the code after substitution - let full_sub_len = - part.snippet.chars().map(|ch| char_width(ch)).sum::() as isize; + let full_sub_len = str_width(&part.snippet) as isize; // length of the code to be substituted let snippet_len = span_end_pos as isize - span_start_pos as isize; @@ -2282,7 +2281,7 @@ impl HumanEmitter { // if we elided some lines, add an ellipsis if lines.next().is_some() { let placeholder = self.margin(); - let padding: usize = placeholder.chars().map(|ch| char_width(ch)).sum(); + let padding = str_width(placeholder); buffer.puts( row_num, max_line_num_len.saturating_sub(padding), diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 352152a1bd4..114ee7ac9de 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1772,8 +1772,8 @@ pub fn parse_error_format( color, )); early_dcx.early_fatal(format!( - "argument for `--error-format` must be `human`, `json` or \ - `short` (instead was `{arg}`)" + "argument for `--error-format` must be `human`, `human-annotate-rs`, \ + `human-unicode`, `json`, `pretty-json` or `short` (instead was `{arg}`)" )) } } @@ -1826,21 +1826,21 @@ pub fn parse_crate_edition(early_dcx: &EarlyDiagCtxt, matches: &getopts::Matches fn check_error_format_stability( early_dcx: &EarlyDiagCtxt, unstable_opts: &UnstableOptions, - error_format: ErrorOutputType, + format: ErrorOutputType, ) { - if !unstable_opts.unstable_options { - if let ErrorOutputType::Json { pretty: true, .. } = error_format { - early_dcx.early_fatal("`--error-format=pretty-json` is unstable"); - } - if let ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateSnippet, _) = - error_format - { - early_dcx.early_fatal("`--error-format=human-annotate-rs` is unstable"); - } - if let ErrorOutputType::HumanReadable(HumanReadableErrorType::Unicode, _) = error_format { - early_dcx.early_fatal("`--error-format=human-unicode` is unstable"); - } + if unstable_opts.unstable_options { + return; } + let format = match format { + ErrorOutputType::Json { pretty: true, .. } => "pretty-json", + ErrorOutputType::HumanReadable(format, _) => match format { + HumanReadableErrorType::AnnotateSnippet => "human-annotate-rs", + HumanReadableErrorType::Unicode => "human-unicode", + _ => return, + }, + _ => return, + }; + early_dcx.early_fatal(format!("`--error-format={format}` is unstable")) } fn parse_output_types( diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 5b1be5bca05..ef775068515 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -2223,6 +2223,10 @@ pub fn char_width(ch: char) -> usize { } } +pub fn str_width(s: &str) -> usize { + s.chars().map(char_width).sum() +} + /// Normalizes the source code and records the normalizations. fn normalize_src(src: &mut String) -> Vec { let mut normalized_pos = vec![];