Skip to content

Conversation

pommicket
Copy link
Contributor

Fixes #109537

i don't know if this is the best way of fixing this but it works

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 24, 2023
@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 27, 2023

for the error in main.rs, rust gives the column number as a "character index" (i.e. the error is located at the 4th character in the line) which i think is the intended behavior, but for the reference to m.rs, it seems to be treating a tab as 4 columns

Why is the right number reported in main.rs?
The file value wasn't previously kept and the display value should be 7 for main.rs, but the compiler still managed report the correct value 4 somehow. What is the difference between main.rs and m.rs?

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2023
@pommicket
Copy link
Contributor Author

There are multiple places in the code where error line/column references are generated:

So-called "primary" messages are generated here (emitter.rs:1504 in method EmitterWriter::emit_message_default) — this is where the reference to main.rs is generated:

if !self.short_message {
    // remember where we are in the output buffer for easy reference
    let buffer_msg_line_offset = buffer.num_lines();

    buffer.prepend(buffer_msg_line_offset, "--> ", Style::LineNumber);
    buffer.append(
        buffer_msg_line_offset,
        &format!(
            "{}:{}:{}",
            sm.filename_for_diagnostics(&loc.file.name),
            sm.doctest_offset_line(&loc.file.name, loc.line),
            loc.col.0 + 1,
        ),
        Style::LineAndColumn,
    );
    for _ in 0..max_line_num_len {
        buffer.prepend(buffer_msg_line_offset, " ", Style::NoStyle);
    }
} else {
    buffer.prepend(
        0,
        &format!(
            "{}:{}:{}: ",
            sm.filename_for_diagnostics(&loc.file.name),
            sm.doctest_offset_line(&loc.file.name, loc.line),
            loc.col.0 + 1,
        ),
        Style::LineAndColumn,
    );
}

Here we have access to the Loc which includes the correct file column number.

Whereas the m.rs reference comes from here (line 1547):

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)
    } else {
        String::new()
    };
    format!(
        "{}:{}{}",
        sm.filename_for_diagnostics(&annotated_file.file.name),
        sm.doctest_offset_line(&annotated_file.file.name, first_line.line_index),
        col
    )
} else {
    format!("{}", sm.filename_for_diagnostics(&annotated_file.file.name))
};

Here we only have an Annotation, so its start_col member was used even though it actually represents the display column.

Another possibility for fixing this (other than what I've done in my branch) is to somehow recover/keep the column number from msp.span_labels but I'm not sure if it would be any easier or better.

@pommicket
Copy link
Contributor Author

@rustbot reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2023
@petrochenkov
Copy link
Contributor

Ok, sounds reasonable.
r=me after squashing all commits into one, and also rebasing to avoid merge commits.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2023
@pommicket pommicket force-pushed the better-column-numbers-with-hard-tabs branch 2 times, most recently from c0404f7 to 1584b77 Compare March 28, 2023 12:15
@petrochenkov
Copy link
Contributor

There's one more thing I forgot about - could you add the reproduction from #109537 as a test to tests/ui/diagnostic-width?

@rust-log-analyzer

This comment has been minimized.

@pommicket pommicket force-pushed the better-column-numbers-with-hard-tabs branch from 1584b77 to b82608a Compare March 28, 2023 14:14
@pommicket
Copy link
Contributor Author

@rustbot reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 28, 2023
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Mar 28, 2023

📌 Commit b82608a has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#109149 (Improve error message when writer is forgotten in write and writeln macro)
 - rust-lang#109367 (Streamline fast rejection)
 - rust-lang#109548 (AnnotationColumn struct to fix hard tab column numbers in errors)
 - rust-lang#109694 (do not panic on failure to acquire jobserver token)
 - rust-lang#109705 (new solver: check for intercrate mode when accessing the cache)
 - rust-lang#109708 (Specialization involving RPITITs is broken so ignore the diagnostic differences)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6be27b1 into rust-lang:master Mar 29, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 29, 2023
@pommicket pommicket deleted the better-column-numbers-with-hard-tabs branch March 29, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent column numbers when using hard tabs
5 participants