cargo fix --edition failure (probably macros related)

[dignifiedquire]

Tried converting my project using cargo fix --edition, but go the following output.

• source code: https://github.com/dignifiedquire/rpgp/tree/06458a8cf35927f165a2930ddcbd57e535148f5c
• rustc: rustc 1.34.0-nightly (aadbc459b 2019-02-23)

$ cargo fix --edition
    Checking pgp v0.1.0 (/Users/dignifiedquire/opensource/rpgp)
warning: failed to automatically apply fixes suggested by rustc to crate `pgp`

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

  * src/composed/shared.rs
  * src/composed/signed_key/key_parser_macros.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 one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `Iterator`
   --> src/composed/signed_key/key_parser_macros.rs:176:28
    |
176 |               ) -> Box<r#dyn Iterator<Item = $crate::errors::Result<Self>> + 'a> {
    |                              ^^^^^^^^ expected one of 7 possible tokens here
    |
   ::: src/composed/signed_key/public.rs:21:1
    |

Full log: https://gist.github.com/dignifiedquire/d75bb480b0f51645dcb74b95bb88c10d

[pnkfelix]

discussed in T-compiler meeting. P-high. still needs assignment of someone to investigate though.

[nikomatsakis]

@dignifiedquire do you have any idea if this is a regression -- i.e., did it work with some older nightly builds? Is your crate nightly only?

[nikomatsakis]

If it is, it'd be great to try and bisect it.

[pnkfelix]

here is a reduced test case, built by just taking the Box<dyn Iterator> type under a macro definition and putting it into a freshly made crate:

% cargo --version
cargo 1.35.0-nightly (95b45eca1 2019-03-06)
% rustc --version
rustc 1.35.0-nightly (9d71ec135 2019-03-10)
% cargo new demo --edition 2015
     Created binary (application) `demo` package
% cat >> demo/src/main.rs
macro_rules! m {
    () => {
        Box<dyn Iterator<Item = u32>>
    }
}

pub fn foo() -> m!() {
    Box::new(None.into_iter())
}
^D
% cd demo
% cargo build
   Compiling demo v0.1.0 (/home/pnkfelix/Dev/Mozilla/issue59065/demo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.24s
% cargo fix --edition
    Checking demo v0.1.0 (/home/pnkfelix/Dev/Mozilla/issue59065/demo)
warning: failed to automatically apply fixes suggested by rustc to crate `demo`

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

  * src/main.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 one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `Iterator`
  --> src/main.rs:6:19
   |
6  |         Box<r#dyn Iterator<Item = u32>>
   |                   ^^^^^^^^ expected one of 7 possible tokens here
...
10 | pub fn foo() -> m!() {
   |                 ---- in this macro invocation

error: aborting due to previous error

Original diagnostics will follow.

warning: `dyn` is a keyword in the 2018 edition
 --> src/main.rs:6:13
  |
6 |         Box<dyn Iterator<Item = u32>>
  |             ^^^ help: you can use a raw identifier to stay compatible: `r#dyn`
  |
  = note: `-W keyword-idents` implied by `-W rust-2018-compatibility`
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
  = note: for more information, see issue #49716 <https://github.com/rust-lang/rust/issues/49716>

warning: `dyn` is a keyword in the 2018 edition
 --> src/main.rs:6:13
  |
6 |         Box<dyn Iterator<Item = u32>>
  |             ^^^ help: you can use a raw identifier to stay compatible: `r#dyn`
  |
  = note: `-W keyword-idents` implied by `-W rust-2018-compatibility`
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
  = note: for more information, see issue #49716 <https://github.com/rust-lang/rust/issues/49716>

    Finished dev [unoptimized + debuginfo] target(s) in 0.21s

[pnkfelix]

(I also don't know how we should actually fix this. I assume the problem is arising because rustfix is trying to rewrite the macro definition itself without actually knowing the roles of the identifiers that appear within the macro body.)

[pnkfelix]

(And in cases its not obvious, you can use any trait, e.g. std::fmt::Debug, if for some reason you'd like to reduce the test case further.)

[pnkfelix]

one other note: if the macro is unused in the crate (or if for some reason changing dyn to r#dyn happens to compile successfully), rustfix will perform the macro rewrite without error and commit the result to the disk.

This may be even worse than the error given above.

(But again, I am not sure how best to address this. Hypothetically there could be macros out there that do use dyn as a normal identifier, and those would want to be rewritten to use r#dyn...)

[pnkfelix]

I suppose the most direct thing to do is to try to fix this (play):

#![warn(keyword_idents)]

pub trait Trait { }

impl Trait for &'static str { }

macro_rules! m { () => { dyn Trait } }

fn foo() -> Box<m!()> { Box::new("from foo") }

fn bar() -> Box<dyn Trait> { Box::new("from bar") }

fn main() { let _ = (foo(), bar()); }

which when compiled under 2015 edition currently emits this diagnostic:

warning: `dyn` is a keyword in the 2018 edition
 --> src/main.rs:7:26
  |
7 | macro_rules! m { () => { dyn Trait } }
  |                          ^^^ help: you can use a raw identifier to stay compatible: `r#dyn`
  |
note: lint level defined here
 --> src/main.rs:1:9
  |
1 | #![warn(keyword_idents)]
  |         ^^^^^^^^^^^^^^
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
  = note: for more information, see issue #49716 <https://github.com/rust-lang/rust/issues/49716>

cc #49716

[pnkfelix]

(Hmm that lint points to #49716 but I think that tracking issue is solely about linting the async identifier ... there should be a separate issue for dyn?)

[pnkfelix]

Ah this issue was known about back in November, see #56327 !

[pnkfelix]

okay i don't think there's anything useful in this ticket beyond what was already reported in #56327.

I'm going to close this as a duplicate of #56327 (but I'm also going to migrate some tags from this issue to that one, so that we'll discuss how to address this in the next compiler meeting.)