Unexpected Regression in Rust 1.47 for Custom Derives

[mitsuhiko]

This is a meta issue that the scroll crate changed behavior in 1.47 which broke the minidump crate. There is a pull request to the scroll crate to fix the issue here: m4b/scroll#75

There is also a repo with a repro case: https://github.com/mitsuhiko/rustc-147-macro-regression

I was surprised to see that a stable update to Rust would do this. Apparently macro expansion of array types in struct fields creates invisible type groups which were previously not emitted.

I'm not sure if this issue is valid but I figured I might surface it.

Meta

rustc --version --verbose:

rustc 1.47.0 (18bf6b4f0 2020-10-07)
binary: rustc
commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
commit-date: 2020-10-07
host: x86_64-apple-darwin
release: 1.47.0
LLVM version: 11.0

[Aaron1011]

This is mentioned in the release notes: https://github.com/rust-lang/rust/blob/master/RELEASES.md#compatibility-notes

Added changes to how proc-macros are expanded in macro_rules! that should help to preserve more span information. These changes may cause compiliation errors if your macro was unhygenic or didn't correctly handle Delimiter::None.

However, it might be a good idea to feature this more prominently.

@mitsuhiko: I'm sorry about the breakage that this has caused. This is an unfortunate consequence of making progress on #43081 - as we preserve tokens more precisely, we will start to pass new None-delimited groups to proc-macros.

We did a Crater run before landing the change, but it looks like Crater didn't cover any code that uses #[derive(Pread)] in a macro_rules! macro.

[mitsuhiko]

No worries. Indeed linked from the PR in the changelog (#73084) is a similar fix: rustwasm/wasm-bindgen@3dd8f3d#diff-72d47f188a68cf3b429a932845476c3bR379-R386

We did a Crater run before landing the change, but it looks like Crater didn't cover any code that uses #[derive(Pread)] in a macro_rules! macro.

It only breaks when you use array types it seems.

[jan-auer]

If it helps, we found this in https://github.com/luser/rust-minidump.

[Aaron1011]

It looks like the crates.io version of minidump got included in the Crater run, but the Github repository didn't. I'll try to figure out how that happened.

[jan-auer]

That makes sense. The minidump crate has not been published for a while. Now that you're mentioning it, I wasn't even aware that crater also checks for the latest master of certain crates.

[Aaron1011]

Crater theoretically checks all Rust repositories on Github - see https://github.com/rust-lang/rust-repos/

The rust-minidump repository is included in the latest data/github.csv, and in the github.csv at the time of the Crater run (see #73084 (comment))

However, the rust-minidump repository doesn't show up anywhere in the Crater report.

cc @rust-lang/infra : Does anyone know why a Github repository (https://github.com/luser/rust-minidump) that's include in data/github.csv would have been skipped entirely in a Crater run? It doesn't appear in the Crater blacklist.

[Aaron1011]

Ah, found the issue. Crater is only testing repositories with both a Cargo.lock and a Cargo.toml:

https://github.com/rust-lang/crater/blob/master/src/crates/sources/github.rs#L44-L47

rust-minidump lacks a Cargo.lock, so it got skipped.

[Aaron1011]

I opened rust-lang/crater#548 to address this issue for future Crater runs.

[mitsuhiko]

Going to close this issue as it's working as intended.