Skip to content

AnnotationColumn struct to fix hard tab column numbers in errors #109548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ impl AnnotateSnippetEmitterWriter {
annotations: annotations
.iter()
.map(|annotation| SourceAnnotation {
range: (annotation.start_col, annotation.end_col),
range: (
annotation.start_col.display,
annotation.end_col.display,
),
label: annotation.label.as_deref().unwrap_or_default(),
annotation_type: annotation_type_for_level(*level),
})
Expand Down
51 changes: 30 additions & 21 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ use Destination::*;
use rustc_span::source_map::SourceMap;
use rustc_span::{FileLines, SourceFile, Span};

use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
use crate::snippet::{
Annotation, AnnotationColumn, AnnotationType, Line, MultilineAnnotation, Style, StyledString,
};
use crate::styled_buffer::StyledBuffer;
use crate::translation::{to_fluent_args, Translate};
use crate::{
Expand Down Expand Up @@ -858,7 +860,7 @@ impl EmitterWriter {
let mut short_start = true;
for ann in &line.annotations {
if let AnnotationType::MultilineStart(depth) = ann.annotation_type {
if source_string.chars().take(ann.start_col).all(|c| c.is_whitespace()) {
if source_string.chars().take(ann.start_col.display).all(|c| c.is_whitespace()) {
let style = if ann.is_primary {
Style::UnderlinePrimary
} else {
Expand Down Expand Up @@ -1093,15 +1095,15 @@ impl EmitterWriter {
'_',
line_offset + pos,
width_offset + depth,
(code_offset + annotation.start_col).saturating_sub(left),
(code_offset + annotation.start_col.display).saturating_sub(left),
style,
);
}
_ if self.teach => {
buffer.set_style_range(
line_offset,
(code_offset + annotation.start_col).saturating_sub(left),
(code_offset + annotation.end_col).saturating_sub(left),
(code_offset + annotation.start_col.display).saturating_sub(left),
(code_offset + annotation.end_col.display).saturating_sub(left),
style,
annotation.is_primary,
);
Expand Down Expand Up @@ -1133,7 +1135,7 @@ impl EmitterWriter {
for p in line_offset + 1..=line_offset + pos {
buffer.putc(
p,
(code_offset + annotation.start_col).saturating_sub(left),
(code_offset + annotation.start_col.display).saturating_sub(left),
'|',
style,
);
Expand Down Expand Up @@ -1169,9 +1171,9 @@ impl EmitterWriter {
let style =
if annotation.is_primary { Style::LabelPrimary } else { Style::LabelSecondary };
let (pos, col) = if pos == 0 {
(pos + 1, (annotation.end_col + 1).saturating_sub(left))
(pos + 1, (annotation.end_col.display + 1).saturating_sub(left))
} else {
(pos + 2, annotation.start_col.saturating_sub(left))
(pos + 2, annotation.start_col.display.saturating_sub(left))
};
if let Some(ref label) = annotation.label {
buffer.puts(line_offset + pos, code_offset + col, label, style);
Expand Down Expand Up @@ -1208,7 +1210,7 @@ impl EmitterWriter {
} else {
('-', Style::UnderlineSecondary)
};
for p in annotation.start_col..annotation.end_col {
for p in annotation.start_col.display..annotation.end_col.display {
buffer.putc(
line_offset + 1,
(code_offset + p).saturating_sub(left),
Expand Down Expand Up @@ -1459,7 +1461,7 @@ impl EmitterWriter {
&annotated_file.file.name,
line.line_index
),
annotations[0].start_col + 1,
annotations[0].start_col.file + 1,
),
Style::LineAndColumn,
);
Expand Down Expand Up @@ -1546,7 +1548,7 @@ impl EmitterWriter {
buffer.prepend(buffer_msg_line_offset + 1, "::: ", Style::LineNumber);
let loc = if let Some(first_line) = annotated_file.lines.first() {
let col = if let Some(first_annotation) = first_line.annotations.first() {
format!(":{}", first_annotation.start_col + 1)
format!(":{}", first_annotation.start_col.file + 1)
} else {
String::new()
};
Expand Down Expand Up @@ -1607,8 +1609,8 @@ impl EmitterWriter {
let mut span_left_margin = usize::MAX;
for line in &annotated_file.lines {
for ann in &line.annotations {
span_left_margin = min(span_left_margin, ann.start_col);
span_left_margin = min(span_left_margin, ann.end_col);
span_left_margin = min(span_left_margin, ann.start_col.display);
span_left_margin = min(span_left_margin, ann.end_col.display);
}
}
if span_left_margin == usize::MAX {
Expand All @@ -1625,11 +1627,12 @@ impl EmitterWriter {
annotated_file.file.get_line(line.line_index - 1).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);
span_right_margin = max(span_right_margin, ann.start_col.display);
span_right_margin = max(span_right_margin, ann.end_col.display);
// FIXME: account for labels not in the same line
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);
label_right_margin =
max(label_right_margin, ann.end_col.display + label_right);
}
}

Expand Down Expand Up @@ -2352,17 +2355,17 @@ impl FileWithAnnotatedLines {
depth: 1,
line_start: lo.line,
line_end: hi.line,
start_col: lo.col_display,
end_col: hi.col_display,
start_col: AnnotationColumn::from_loc(&lo),
end_col: AnnotationColumn::from_loc(&hi),
is_primary,
label,
overlaps_exactly: false,
};
multiline_annotations.push((lo.file, ml));
} else {
let ann = Annotation {
start_col: lo.col_display,
end_col: hi.col_display,
start_col: AnnotationColumn::from_loc(&lo),
end_col: AnnotationColumn::from_loc(&hi),
is_primary,
label,
annotation_type: AnnotationType::Singleline,
Expand Down Expand Up @@ -2551,7 +2554,13 @@ fn num_overlap(
(b_start..b_end + extra).contains(&a_start) || (a_start..a_end + extra).contains(&b_start)
}
fn overlaps(a1: &Annotation, a2: &Annotation, padding: usize) -> bool {
num_overlap(a1.start_col, a1.end_col + padding, a2.start_col, a2.end_col, false)
num_overlap(
a1.start_col.display,
a1.end_col.display + padding,
a2.start_col.display,
a2.end_col.display,
false,
)
}

fn emit_to_destination(
Expand Down
65 changes: 51 additions & 14 deletions compiler/rustc_errors/src/snippet.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,46 @@
// Code for annotating snippets.

use crate::Level;
use crate::{Level, Loc};

#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub struct Line {
pub line_index: usize,
pub annotations: Vec<Annotation>,
}

#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Default)]
pub struct AnnotationColumn {
/// the (0-indexed) column for *display* purposes, counted in characters, not utf-8 bytes
pub display: usize,
/// the (0-indexed) column in the file, counted in characters, not utf-8 bytes.
///
/// this may be different from `self.display`,
/// e.g. if the file contains hard tabs, because we convert tabs to spaces for error messages.
///
/// for example:
/// ```text
/// (hard tab)hello
/// ^ this is display column 4, but file column 1
/// ```
///
/// we want to keep around the correct file offset so that column numbers in error messages
/// are correct. (motivated by <https://github.com/rust-lang/rust/issues/109537>)
pub file: usize,
}

impl AnnotationColumn {
pub fn from_loc(loc: &Loc) -> AnnotationColumn {
AnnotationColumn { display: loc.col_display, file: loc.col.0 }
}
}

#[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 start_col: AnnotationColumn,
pub end_col: AnnotationColumn,
pub is_primary: bool,
pub label: Option<String>,
pub overlaps_exactly: bool,
Expand All @@ -36,7 +62,12 @@ impl MultilineAnnotation {
pub fn as_start(&self) -> Annotation {
Annotation {
start_col: self.start_col,
end_col: self.start_col + 1,
end_col: AnnotationColumn {
// these might not correspond to the same place anymore,
// but that's okay for our purposes
display: self.start_col.display + 1,
file: self.start_col.file + 1,
},
is_primary: self.is_primary,
label: None,
annotation_type: AnnotationType::MultilineStart(self.depth),
Expand All @@ -45,7 +76,12 @@ impl MultilineAnnotation {

pub fn as_end(&self) -> Annotation {
Annotation {
start_col: self.end_col.saturating_sub(1),
start_col: AnnotationColumn {
// these might not correspond to the same place anymore,
// but that's okay for our purposes
display: self.end_col.display.saturating_sub(1),
file: self.end_col.file.saturating_sub(1),
},
end_col: self.end_col,
is_primary: self.is_primary,
label: self.label.clone(),
Expand All @@ -55,8 +91,8 @@ impl MultilineAnnotation {

pub fn as_line(&self) -> Annotation {
Annotation {
start_col: 0,
end_col: 0,
start_col: Default::default(),
end_col: Default::default(),
is_primary: self.is_primary,
label: None,
annotation_type: AnnotationType::MultilineLine(self.depth),
Expand Down Expand Up @@ -92,14 +128,14 @@ pub enum AnnotationType {

#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub struct Annotation {
/// Start column, 0-based indexing -- counting *characters*, not
/// utf-8 bytes. Note that it is important that this field goes
/// Start column.
/// Note that it is important that this field goes
/// first, so that when we sort, we sort orderings by start
/// column.
pub start_col: usize,
pub start_col: AnnotationColumn,

/// End column within the line (exclusive)
pub end_col: usize,
pub end_col: AnnotationColumn,

/// Is this annotation derived from primary span
pub is_primary: bool,
Expand All @@ -118,12 +154,13 @@ impl Annotation {
matches!(self.annotation_type, AnnotationType::MultilineLine(_))
}

/// Length of this annotation as displayed in the stderr output
pub fn len(&self) -> usize {
// Account for usize underflows
if self.end_col > self.start_col {
self.end_col - self.start_col
if self.end_col.display > self.start_col.display {
self.end_col.display - self.start_col.display
} else {
self.start_col - self.end_col
self.start_col.display - self.end_col.display
}
}

Expand Down
6 changes: 6 additions & 0 deletions tests/ui/diagnostic-width/auxiliary/tab_column_numbers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// ignore-tidy-tab

pub struct S;
impl S {
fn method(&self) {}
}
12 changes: 12 additions & 0 deletions tests/ui/diagnostic-width/tab-column-numbers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Test for #109537: ensure that column numbers are correctly generated when using hard tabs.
// aux-build:tab_column_numbers.rs

// ignore-tidy-tab

extern crate tab_column_numbers;

fn main() {
let s = tab_column_numbers::S;
s.method();
//~^ ERROR method `method` is private
}
14 changes: 14 additions & 0 deletions tests/ui/diagnostic-width/tab-column-numbers.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0624]: method `method` is private
--> $DIR/tab-column-numbers.rs:10:4
|
LL | s.method();
| ^^^^^^ private method
|
::: $DIR/auxiliary/tab_column_numbers.rs:5:3
|
LL | fn method(&self) {}
| ---------------- private method defined here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0624`.