Incorrect span / broken rustfix: help: use dyn: dyn #[dom_struct]

[SimonSapin]

One of the many new (or newly enabled by default) warnings in Servo in today’s Nightly:

warning: trait objects without an explicit `dyn` are deprecated
   --> components/script/dom/window.rs:169:1
    |
169 | #[dom_struct]
    | ^^^^^^^^^^^^^ help: use `dyn`: `dyn #[dom_struct]`

The suggestion is wrong, which means cargo fix also fails and therefore does not apply fixes for (the many) other warnings where the suggestion is correct.

warning: failed to automatically apply fixes suggested by rustc to crate `script`

after fixes were automatically applied the compiler reported errors within these files:

  * components/script/dom/dedicatedworkerglobalscope.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error: expected item, found keyword `dyn`
   --> components/script/dom/dedicatedworkerglobalscope.rs:164:1
    |
164 | dyn #[dom_struct]
    | ^^^ expected item

error: aborting due to previous error

Original diagnostics will follow.

[Centril]

Triage: Preliminarily assigning P-high before Felix reviews this.

[Centril]

Also cc @pnkfelix since iirc you did some macro related work here.

[SimonSapin]

Possible solution: when a warning’s span is exactly a macro invocation, don’t emit a “suggested fix” that is likely wrong. (But still emit a warning.)

[estebank]

We have that check for most suggestions, but it might be missing for dyn. Was this in latest nightly? I know a couple of these have been fixed in the past few weeks.

[SimonSapin]

This is in rustc 1.37.0-nightly (04a3dd8 2019-06-18).

#![allow(bare_trait_objects)] works around this and allows rustfix’ing the rest of the warnings (namely old syntax for inclusive ranges) or warnings in other crates.

[SimonSapin]

Oh. I just noticed the --broken-code suggestion. I’d missed it in that blob of text.

[davidtwco]

Apologies for the time this has taken - I've struggled to find time to dig in to this. So far, I've managed to make a smaller example (it still depends on syn, quote and proc-macro2) that reproduces the issue, you can find it in davidtwco/rust-reproduction-issue-61963 (run cargo build in the repro-crate directory).

[davidtwco]

Thanks to some help from @lqd, I've now got a minimal test case w/out any dependencies that outputs the same tokens that the reduced example from servo did (you can see it in 732507b).

When inspecting the span that the lint is emitted for, it isn't marked as coming from a macro expansion, and it appears that it was created that way during the macro expansion.

I'd appreciate any opinions on what the correct fix is here - is it a bug in syn/quote that they output what appears to be incorrect spans? Should rustc be able to handle this case? If so, where should I be looking to mark the span or what check should I be performing to identify it as a result of macro expansion?

[Centril]

cc @dtolnay @petrochenkov

[dtolnay]

I believe this is a compiler bug, not something that needs a fix in syn or quote. #0 is a call_site span. It is the span of tokens in macro output that are not attributable to any particular input token. Usually that will be most of the tokens in any macro output; diagnostics need to take this into account.