Some macro errors now include file names into the standard library (JSON).

[ehuss]

The error message for the following has changed so that that JSON spans include file names into the standard library:

struct S;

pub fn f() {
    let x = vec![1,2,3];
    let x = vec![S; x.len()];
}

With JSON output, previously this had a span that looked like this:

// Rust 1.43 output
{
  "byte_end": 60,
  "byte_start": 33,
  "column_end": 61,
  "column_start": 34,
  "expansion": {
    "def_site_span": {
      "byte_end": 198,
      "byte_start": 0,
      "column_end": 55,
      "column_start": 1,
      "expansion": null,
      "file_name": "<::alloc::macros::vec macros>",
      "is_primary": false,
      "label": null,
      "line_end": 3,
      "line_start": 1,
      "suggested_replacement": null,
      "suggestion_applicability": null,
      "text": [
        {
          "highlight_end": 78,
          "highlight_start": 1,
          "text": "($ elem : expr ; $ n : expr) => ($ crate :: vec :: from_elem ($ elem, $ n)) ;"
        },
        {
          "highlight_end": 66,
          "highlight_start": 1,
          "text": "($ ($ x : expr), *) => (< [_] > :: into_vec (box [$ ($ x), *])) ;"
        },
        {
          "highlight_end": 55,
          "highlight_start": 1,
          "text": "($ ($ x : expr,) *) => ($ crate :: vec ! [$ ($ x), *])"
        }
      ]
    },
    "macro_decl_name": "vec!",
    "span": {
      "byte_end": 79,
      "byte_start": 62,
      "column_end": 30,
      "column_start": 13,
      "expansion": null,
      "file_name": "src/lib.rs",
      "is_primary": false,
      "label": null,
      "line_end": 5,
      "line_start": 5,
      "suggested_replacement": null,
      "suggestion_applicability": null,
      "text": [
        {
          "highlight_end": 30,
          "highlight_start": 13,
          "text": "    let x = vec![S; x.len()];"
        }
      ]
    }
  },
  "file_name": "<::alloc::macros::vec macros>",
  "is_primary": true,
  "label": "the trait `std::clone::Clone` is not implemented for `S`",
  "line_end": 1,
  "line_start": 1,
  "suggested_replacement": null,
  "suggestion_applicability": null,
  "text": [
    {
      "highlight_end": 61,
      "highlight_start": 34,
      "text": "($ elem : expr ; $ n : expr) => ($ crate :: vec :: from_elem ($ elem, $ n)) ;"
    }
  ]
},

Pay attention to the "file_name" field, it now includes a span that references macros.rs instead of <::alloc::macros::vec macros>. There are other differences, such as missing text rendering. The new output is:

// Rust 1.44 (nightly) output
{
  "byte_end": 1284,
  "byte_start": 1262,
  "column_end": 31,
  "column_start": 9,
  "expansion": {
    "def_site_span": {
      "byte_end": 1420,
      "byte_start": 1204,
      "column_end": 2,
      "column_start": 1,
      "expansion": null,
      "file_name": "/rustc/38114ff16e7856f98b2b4be7ab4cd29b38bed59a/src/liballoc/macros.rs",
      "is_primary": false,
      "label": null,
      "line_end": 46,
      "line_start": 38,
      "suggested_replacement": null,
      "suggestion_applicability": null,
      "text": []
    },
    "macro_decl_name": "vec!",
    "span": {
      "byte_end": 79,
      "byte_start": 62,
      "column_end": 30,
      "column_start": 13,
      "expansion": null,
      "file_name": "src/lib.rs",
      "is_primary": false,
      "label": null,
      "line_end": 5,
      "line_start": 5,
      "suggested_replacement": null,
      "suggestion_applicability": null,
      "text": [
        {
          "highlight_end": 30,
          "highlight_start": 13,
          "text": "    let x = vec![S; x.len()];"
        }
      ]
    }
  },
  "file_name": "/rustc/38114ff16e7856f98b2b4be7ab4cd29b38bed59a/src/liballoc/macros.rs",
  "is_primary": true,
  "label": "the trait `std::clone::Clone` is not implemented for `S`",
  "line_end": 40,
  "line_start": 40,
  "suggested_replacement": null,
  "suggestion_applicability": null,
  "text": []
},

This is a problem because the path /rustc/38114ff16e7856f98b2b4be7ab4cd29b38bed59a doesn't exist, so there is nothing to point to. Additionally, tools may be expecting the previous behavior of a … macros> value in the "file_name".

Note that this seems particular to using a const expression in the example above (a literal such as 3 doesn't have the same effect).

This seems to have started with #66364 (cc @Centril).

rustc 1.44.0-nightly (38114ff16 2020-03-21)

[spastorino]

pre-triage: prioritized as P-medium given that this is unlikely to break important stuff badly.

@Centril should we assign to you?.

[Centril]

Let's not assign it to me, but let's cc @eddyb and @petrochenkov :)

[eddyb]

This is a problem because the path /rustc/38114ff16e7856f98b2b4be7ab4cd29b38bed59a doesn't exist, so there is nothing to point to.

This is #53486, maybe we should prioritize it?

Additionally, tools may be expecting the previous behavior of a … macros> value in the "file_name".

I hope not, it was always a hack that we've been wanting to get rid for years (#49511) and even used to be even more useless (e.g. <std macros> i.e. crate name, that's why it was macros with an s).

We shouldn't treat this as a regression unless we have some evidence of misuse in the wild.

[petrochenkov]

path /rustc/38114ff16e7856f98b2b4be7ab4cd29b38bed59a doesn't exist

Hmm, I'm pretty sure that I've seen paths like this before #66364, but probably in debuginfo rather than in spans.

It still seems to be better than no paths at all because some tools have an option to remap paths like this to a different directory with actual sources, or /rustc/38114ff16e7856f98b2b4be7ab4cd29b38bed59a can be symlinked to the actual source directory in the worst case.

[ehuss]

We shouldn't treat this as a regression unless we have some evidence of misuse in the wild.

Just a quick perusal of projects:

• Sublime: https://github.com/rust-lang/rust-enhanced/blob/7acf8a9914906efb2431ba475f2148f1c16e2ae4/rust/messages.py#L988-L1007
• RLS: https://github.com/rust-lang/rls/blob/782b7962546ab584097da6d5c80fcff9d4f19c7a/rls/src/actions/diagnostics.rs#L122-L130
• Rust-analyzer: https://github.com/rust-analyzer/rust-analyzer/blob/402bf53a09a3edbb0ff4c94bf22d94d484f81eb9/crates/ra_cargo_watch/src/conv.rs#L34-L36

I imagine there are others. I wouldn't call it "misuse". It is how stable rustc behaves, and tools have to deal with it.

The immediate issue for me, is that I need to find the invocation site. It was previously easy to skip over the macros> entries. Now, I guess I'll need to check if paths exist? I'm not sure, I haven't thought about it too much.

Nor do I understand under which scenarios this happens. If someone could explain in clear terms what the change is, that would be helpful. I can probably update the documentation if this really is a permanent change.

It was also useful for emitting the message "this error originates in a macro outside of the current crate", but now I don't see a way to detect that?

Regardless, this is a breaking change. I'm not so sensitive about them, I can adapt, but it will likely have some impact on other projects, too.

[spastorino]

hmm I wonder if we should bump the priority to P-high then. Thoughts?.

[Centril]

Regardless, this is a breaking change. I'm not so sensitive about them, I can adapt, but it will likely have some impact on other projects, too.

(I want to push back on the notion of "breaking change" here, as it is typically associated with "you broke some stability guarantees", which isn't the case here AFAIK, and it would be pretty dangerous if we would have given such guarantees. That said, it would be nice to make more robust improvements here, but I agree with @petrochenkov re. "better than nothing".)

For now I think P-medium is appropriate.