Skip to content
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

Regression in spacing of left margin in diagnostics #65001

Closed
dtolnay opened this issue Oct 2, 2019 · 6 comments · Fixed by #65010
Closed

Regression in spacing of left margin in diagnostics #65001

dtolnay opened this issue Oct 2, 2019 · 6 comments · Fixed by #65010
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.

Comments

@dtolnay
Copy link
Member

dtolnay commented Oct 2, 2019

struct A;
//
//
//
//
//
//
//
fn main() {
    let _: () = A;
    let _: () = B;
}

$ cargo +nightly-2019-10-01 check

This is the correct output.

error[E0425]: cannot find value `B` in this scope
  --> src/main.rs:11:17
   |
11 |     let _: () = B;
   |                 ^ help: a unit struct with a similar name exists: `A`

error[E0308]: mismatched types
  --> src/main.rs:10:17
   |
10 |     let _: () = A;
   |                 ^ expected (), found struct `A`
   |
   = note: expected type `()`
              found type `A`

$ cargo +nightly-2019-10-02 check

Notice that one of the errors has a column of whitespace between the line number and the pipes, the next does not.

error[E0425]: cannot find value `B` in this scope
  --> src/main.rs:11:17
   |
11 |     let _: () = B;
   |                 ^ help: a unit struct with a similar name exists: `A`

error[E0308]: mismatched types
 --> src/main.rs:10:17
  |
10|     let _: () = A;
  |                 ^ expected (), found struct `A`
  |
  = note: expected type `()`
             found type `A`

Commit range: 22bc9e1...702b45e

@dtolnay dtolnay added A-diagnostics Area: Messages for errors, warnings, and lints regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. C-bug Category: This is a bug. labels Oct 2, 2019
@dtolnay
Copy link
Member Author

dtolnay commented Oct 2, 2019

Mentioning @AnthonyMikh @estebank who made changes in librustc_errors. Could this be from #64935?

@dtolnay
Copy link
Member Author

dtolnay commented Oct 2, 2019

It looks like this can cause the wrong line number to be printed. :(

If you bump the problematic line down to line 100, it prints as 10:

error[E0308]: mismatched types
 --> src/main.rs:100:17
  |
10|     let _: () = A;
  |                 ^ expected (), found struct `A`
  |
  = note: expected type `()`
             found type `A`

@estebank
Copy link
Contributor

estebank commented Oct 2, 2019

It seems like it might be.

@AnthonyMikh
Copy link
Contributor

AnthonyMikh commented Oct 2, 2019

Mentioning @AnthonyMikh @estebank who made changes in librustc_errors. Could this be from #64935?

Indeed. Upon closer inspection I've found out that 7b4c5c6 actually changes the semantics of method: after applying this commit, if children is not empty, then max of them is not compared with primary, although primary could actually be greater.

The correct code should rather look like this:

    fn get_max_line_num(&mut self, span: &MultiSpan, children: &[SubDiagnostic]) -> usize {
        let primary = self.get_multispan_max_line_num(span);
        children.iter()
            .map(|sub| self.get_multispan_max_line_num(&sub.span))
            .max()
            .unwrap_or(0)
            .max(primary)
    }

Although it might be not the bug responsible for this issue, it definetely should be fixed. I'll make a PR with fix ASAP.

EDIT: Here it is

@estebank
Copy link
Contributor

estebank commented Oct 2, 2019

Subtle. That seems about right.

@AnthonyMikh
Copy link
Contributor

For every problem there is a solution that is simple, neat—and wrong.

Manishearth added a commit to Manishearth/rust that referenced this issue Oct 2, 2019
Compare `primary` with maximum of `children`s' line num instead of dropping it

Fix rust-lang#65001.
Centril added a commit to Centril/rust that referenced this issue Oct 2, 2019
Compare `primary` with maximum of `children`s' line num instead of dropping it

Fix rust-lang#65001.
@bors bors closed this as completed in d5a0765 Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants