Regression in spacing of left margin in diagnostics

[dtolnay]

struct MyError;
impl std::error::Error for MyError {}

$ cargo +nightly-2020-04-19 check
error[E0277]: `MyError` doesn't implement `std::fmt::Display`
 --> src/main.rs:2:6
  |
2 | impl std::error::Error for MyError {}
  |      ^^^^^^^^^^^^^^^^^ `MyError` cannot be formatted with the default formatter
  |
  = help: the trait `std::fmt::Display` is not implemented for `MyError`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead

$ cargo +nightly-2020-04-20 check
error[E0277]: `MyError` doesn't implement `std::fmt::Display`
  --> src/main.rs:2:6
   |
2  | impl std::error::Error for MyError {}
   |      ^^^^^^^^^^^^^^^^^ `MyError` cannot be formatted with the default formatter
   |
   = help: the trait `std::fmt::Display` is not implemented for `MyError`
   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead

Notice the extra one column of indentation in front of the pipes. I presume this is unintentional because almost any other error is still printed without such extra indentation; for example:

error: expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `;`
 --> src/main.rs:2:35
  |
2 | impl std::error::Error for MyError;
  |                                   ^ expected one of 7 possible tokens

The relevant commit range is 52fa23a...dbf8b6b.
Mentioning @estebank @eddyb because #69745 looks like it touches error messages vaguely along the same lines.

[estebank]

Could you show the output of --message-format=json? The only thing I can think of is that there's a DUMMY_SP span that isn't being displayed.

[dtolnay]

cargo +nightly-2020-04-20 check --message-format=json | jq

{
  "reason": "compiler-message",
  "package_id": "testing 0.0.0 (path+file:///git/testing)",
  "target": {
    "kind": [
      "bin"
    ],
    "crate_types": [
      "bin"
    ],
    "name": "testing",
    "src_path": "/git/testing/src/main.rs",
    "edition": "2018",
    "doctest": false
  },
  "message": {
    "rendered": "error[E0277]: `MyError` doesn't implement `std::fmt::Display`\n  --> src/main.rs:2:6\n   |\n2  | impl std::error::Error for MyError {}\n   |      ^^^^^^^^^^^^^^^^^ `MyError` cannot be formatted with the default formatter\n   |\n   = help: the trait `std::fmt::Display` is not implemented for `MyError`\n   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead\n\n",
    "children": [
      {
        "children": [],
        "code": null,
        "level": "help",
        "message": "the trait `std::fmt::Display` is not implemented for `MyError`",
        "rendered": null,
        "spans": []
      },
      {
        "children": [],
        "code": null,
        "level": "note",
        "message": "in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead",
        "rendered": null,
        "spans": []
      }
    ],
    "code": {
      "code": "E0277",
      "explanation": "You tried to use a type which doesn't implement some trait in a place which\nexpected that trait.\n\nErroneous code example:\n\n```compile_fail,E0277\n// here we declare the Foo trait with a bar method\ntrait Foo {\n    fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func<T: Foo>(foo: T) {\n    foo.bar();\n}\n\nfn main() {\n    // we now call the method with the i32 type, which doesn't implement\n    // the Foo trait\n    some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n    fn bar(&self);\n}\n\nfn some_func<T: Foo>(foo: T) {\n    foo.bar(); // we can now use this method since i32 implements the\n               // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n    fn bar(&self) {}\n}\n\nfn main() {\n    some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func<T>(foo: T) {\n    println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n                           //        implemented for the type `T`\n}\n\nfn main() {\n    // We now call the method with the i32 type,\n    // which *does* implement the Debug trait.\n    some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func<T: fmt::Debug>(foo: T) {\n    println!(\"{:?}\", foo);\n}\n\nfn main() {\n    // Calling the method is still fine, as i32 implements Debug.\n    some_func(5i32);\n\n    // This would fail to compile now:\n    // struct WithoutDebug;\n    // some_func(WithoutDebug);\n}\n```\n\nRust only looks at the signature of the called function, as such it must\nalready specify all requirements that will be used for every type parameter.\n"
    },
    "level": "error",
    "message": "`MyError` doesn't implement `std::fmt::Display`",
    "spans": [
      {
        "byte_end": 38,
        "byte_start": 21,
        "column_end": 23,
        "column_start": 6,
        "expansion": null,
        "file_name": "src/main.rs",
        "is_primary": true,
        "label": "`MyError` cannot be formatted with the default formatter",
        "line_end": 2,
        "line_start": 2,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": [
          {
            "highlight_end": 23,
            "highlight_start": 6,
            "text": "impl std::error::Error for MyError {}"
          }
        ]
      },
      {
        "byte_end": 1885,
        "byte_start": 1878,
        "column_end": 33,
        "column_start": 26,
        "expansion": null,
        "file_name": "/rustc/dbf8b6bf116c7bece2987ff4bd2792f008a6ee77/src/libstd/error.rs",
        "is_primary": false,
        "label": "required by this bound in `std::error::Error`",
        "line_end": 47,
        "line_start": 47,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": []
      }
    ]
  }
}
{
  "reason": "compiler-message",
  "package_id": "testing 0.0.0 (path+file:///git/testing)",
  "target": {
    "kind": [
      "bin"
    ],
    "crate_types": [
      "bin"
    ],
    "name": "testing",
    "src_path": "/git/testing/src/main.rs",
    "edition": "2018",
    "doctest": false
  },
  "message": {
    "rendered": "error[E0277]: `MyError` doesn't implement `std::fmt::Debug`\n  --> src/main.rs:2:6\n   |\n2  | impl std::error::Error for MyError {}\n   |      ^^^^^^^^^^^^^^^^^ `MyError` cannot be formatted using `{:?}`\n   |\n   = help: the trait `std::fmt::Debug` is not implemented for `MyError`\n   = note: add `#[derive(Debug)]` or manually implement `std::fmt::Debug`\n\n",
    "children": [
      {
        "children": [],
        "code": null,
        "level": "help",
        "message": "the trait `std::fmt::Debug` is not implemented for `MyError`",
        "rendered": null,
        "spans": []
      },
      {
        "children": [],
        "code": null,
        "level": "note",
        "message": "add `#[derive(Debug)]` or manually implement `std::fmt::Debug`",
        "rendered": null,
        "spans": []
      }
    ],
    "code": {
      "code": "E0277",
      "explanation": "You tried to use a type which doesn't implement some trait in a place which\nexpected that trait.\n\nErroneous code example:\n\n```compile_fail,E0277\n// here we declare the Foo trait with a bar method\ntrait Foo {\n    fn bar(&self);\n}\n\n// we now declare a function which takes an object implementing the Foo trait\nfn some_func<T: Foo>(foo: T) {\n    foo.bar();\n}\n\nfn main() {\n    // we now call the method with the i32 type, which doesn't implement\n    // the Foo trait\n    some_func(5i32); // error: the trait bound `i32 : Foo` is not satisfied\n}\n```\n\nIn order to fix this error, verify that the type you're using does implement\nthe trait. Example:\n\n```\ntrait Foo {\n    fn bar(&self);\n}\n\nfn some_func<T: Foo>(foo: T) {\n    foo.bar(); // we can now use this method since i32 implements the\n               // Foo trait\n}\n\n// we implement the trait on the i32 type\nimpl Foo for i32 {\n    fn bar(&self) {}\n}\n\nfn main() {\n    some_func(5i32); // ok!\n}\n```\n\nOr in a generic context, an erroneous code example would look like:\n\n```compile_fail,E0277\nfn some_func<T>(foo: T) {\n    println!(\"{:?}\", foo); // error: the trait `core::fmt::Debug` is not\n                           //        implemented for the type `T`\n}\n\nfn main() {\n    // We now call the method with the i32 type,\n    // which *does* implement the Debug trait.\n    some_func(5i32);\n}\n```\n\nNote that the error here is in the definition of the generic function: Although\nwe only call it with a parameter that does implement `Debug`, the compiler\nstill rejects the function: It must work with all possible input types. In\norder to make this example compile, we need to restrict the generic type we're\naccepting:\n\n```\nuse std::fmt;\n\n// Restrict the input type to types that implement Debug.\nfn some_func<T: fmt::Debug>(foo: T) {\n    println!(\"{:?}\", foo);\n}\n\nfn main() {\n    // Calling the method is still fine, as i32 implements Debug.\n    some_func(5i32);\n\n    // This would fail to compile now:\n    // struct WithoutDebug;\n    // some_func(WithoutDebug);\n}\n```\n\nRust only looks at the signature of the called function, as such it must\nalready specify all requirements that will be used for every type parameter.\n"
    },
    "level": "error",
    "message": "`MyError` doesn't implement `std::fmt::Debug`",
    "spans": [
      {
        "byte_end": 38,
        "byte_start": 21,
        "column_end": 23,
        "column_start": 6,
        "expansion": null,
        "file_name": "src/main.rs",
        "is_primary": true,
        "label": "`MyError` cannot be formatted using `{:?}`",
        "line_end": 2,
        "line_start": 2,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": [
          {
            "highlight_end": 23,
            "highlight_start": 6,
            "text": "impl std::error::Error for MyError {}"
          }
        ]
      },
      {
        "byte_end": 1875,
        "byte_start": 1870,
        "column_end": 23,
        "column_start": 18,
        "expansion": null,
        "file_name": "/rustc/dbf8b6bf116c7bece2987ff4bd2792f008a6ee77/src/libstd/error.rs",
        "is_primary": false,
        "label": "required by this bound in `std::error::Error`",
        "line_end": 47,
        "line_start": 47,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": []
      }
    ]
  }
}

[estebank]

I'm pretty sure it's happening because of this

      {
        "byte_end": 1875,
        "byte_start": 1870,
        "column_end": 23,
        "column_start": 18,
        "expansion": null,
        "file_name": "/rustc/dbf8b6bf116c7bece2987ff4bd2792f008a6ee77/src/libstd/error.rs",
        "is_primary": false,
        "label": "required by this bound in `std::error::Error`",
        "line_end": 47,
        "line_start": 47,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": []
      }

It is weird because that file should be accessible to rustc in all environments now. I think we can get away with some well placed if sp == DUMMY_SP { return; } in a few places.

[spastorino]

It maybe be nice if someone can find where this regressed ...

@rustbot ping cleanup

[rustbot]

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @jakevossen5 @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

[spastorino]

Assigning P-low as discussed as part of the Prioritization Working Group process.

[chrissimpkins]

cargo bisect-rustc indicates that the regression is @ e7497a8 / PR #69793

cc @estebank