Regression: spurious "error: expected ," in serde::Serialize on macro_rules-variable path enum discriminant

Somewhat minimized test case: (still involves serde_derive and syn)

[dependencies]
serde = {version = "1", features = ["derive"]}
syn = { version = "1", features = ["full"] }

mod gl {
    pub const DEPTH_COMPONENT: isize = 0;
}

macro_rules! gl_enums {
    ($mod:ident) => {
        #[derive(serde::Serialize)]
        pub enum TexFormat {
            DepthComponent = $mod::DEPTH_COMPONENT
        }
    }
}

gl_enums!(gl);

Compiles in nightly-2020-05-24, errors in nightly-2020-05-25:

$ cargo +nightly-2020-05-25 rustc -- -Z macro-backtrace
   Compiling proc-macro2 v1.0.17
   Compiling unicode-xid v0.2.0
   Compiling syn v1.0.25
   Compiling serde v1.0.110
   Compiling quote v1.0.6
   Compiling serde_derive v1.0.110
   Compiling aa v0.1.0 (/tmp/aa)
error: expected `,`
  --> src/lib.rs:9:34
   |
5  | / macro_rules! gl_enums {
6  | |     ($mod:ident) => {
7  | |         #[derive(serde::Serialize)]
8  | |         pub enum TexFormat {
9  | |             DepthComponent = $mod::DEPTH_COMPONENT
   | |                                  ^^
10 | |         }
11 | |     }
12 | | }
   | |_- in this expansion of `gl_enums!`
13 | 
14 |   gl_enums!(gl);
   |   -------------- in this macro invocation

error: aborting due to previous error

Any of these changes makes compilation succeed:

Removing use of serde::Serialize
Disabling the syn/full feature
Replacing $mod with a non-variable module name
Replacing the path expression with a { use $mod::DEPTH_COMPONENT; DEPTH_COMPONENT } block

Regression range: 8970e8b...46e85b4. Merges:

Auto merge of Rollup of 5 pull requests #72539 - RalfJung:rollup-8yfidi8, r=RalfJung
Rollup merge of Use sort_unstable_by in its own docs #72535 - saschanaz:patch-1, r=jonas-schievink
Rollup merge of Use dyn trait syntax in more comments and docs #72532 - ratijas:dyn-trait-object-doc, r=jonas-schievink
Rollup merge of Clean up E0602 explanation #72530 - GuillaumeGomez:cleanup-e0602, r=Dylan-DPC
Rollup merge of bootstrap: propagate test-args to miri and clippy test suites #72527 - RalfJung:miri-clippy-test-args, r=Mark-Simulacrum
Rollup merge of Remove all uses of NodeId in ResolverOutputs #72402 - marmeladema:resolver-outputs-def-id, r=ecstatic-morse
Auto merge of bump Miri #72531 - RalfJung:miri-upd, r=RalfJung
Auto merge of Rollup of 3 pull requests #72529 - RalfJung:rollup-ydthv90, r=RalfJung
Rollup merge of Fix typo in doc comment. #72528 - emosenkis:patch-1, r=dtolnay
Rollup merge of Rewrite Parser::collect_tokens #72393 - Aaron1011:feature/rewrite-collect-tokens, r=petrochenkov
Rollup merge of Remove macro_defs map #72284 - Aaron1011:feature/inline-macro-did, r=petrochenkov
Auto merge of Rollup of 2 pull requests #72524 - RalfJung:rollup-s9f1pcc, r=RalfJung
Rollup merge of small select cleanup #72517 - lcnr:refactor-winnowing, r=jonas-schievink
Rollup merge of Recursively expand TokenKind::Interpolated in probably_equal_for_proc_macro #72388 - Aaron1011:fix/deep-tokenstream-equality, r=petrochenkov
Auto merge of Remove ReScope #72362 - matthewjasper:remove-rescope, r=nikomatsakis
Auto merge of Rollup of 5 pull requests #72516 - Dylan-DPC:rollup-cc4w96z, r=Dylan-DPC
Rollup merge of fix discriminant type in generator transform #72502 - RalfJung:generator-discr-ty, r=jonas-schievink
Rollup merge of Fix ice-72487 #72489 - nbdd0121:issue-72487, r=Amanieu
Rollup merge of Add missing ASM arena declarations to librustc_middle #72400 - Aaron1011:fix/asm-incr-ice, r=Amanieu
Rollup merge of Unblock font loading in rustdoc.css #72092 - workingjubilee:patch-2, r=GuillaumeGomez
Rollup merge of Preserve substitutions when making trait obligations for suggestions #71618 - ecstatic-morse:issue-71394, r=nikomatsakis
Auto merge of Revert MSYS2 CI workaround #72474 - mati865:ci-fix, r=pietroalbini

Excluding those that seem unrelated to me leaves:

Let’s see if we can minimize the example and bisect further to find the culprit PR?

@Aaron1011

https://blog.rust-lang.org/inside-rust/2019/12/18/bisecting-rust-compiler.html worked pretty well:

searched nightlies: from nightly-2020-05-24 to nightly-2020-05-26
regressed nightly: nightly-2020-05-25
searched commits: from 8970e8b to 46e85b4
regressed commit: 7726070

bisected with cargo-bisect-rustc v0.5.1

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc

The regression commit is a rollup merge of #72388 and #72517 . The former looks more related. CC @Aaron1011

Replacing serde_derive with this proc macro still reproduces the same error:

use proc_macro::TokenStream;
use syn::{DeriveInput, parse_macro_input};

#[proc_macro_derive(Serialize)]
pub fn derive_serialize(input: TokenStream) -> TokenStream {
    let input = parse_macro_input!(input as DeriveInput);
    unimplemented!()
}

#[proc_macro]
pub fn foo(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
    syn::parse_macro_input!(input as syn::Expr);
    unimplemented!()
}

macro_rules! gl_enums {
    ($mod:ident) => {
        not_serde::foo! {
            $mod::DEPTH_COMPONENT
        }
    }
}
gl_enums!(gl);

Output of dbg!(&input):

TokenStream [
    Group {
        delimiter: None,
        stream: TokenStream [
            Ident {
                ident: "gl",
                span: #0 bytes(136..138),
            },
        ],
        span: #3 bytes(84..88),
    },
    Punct {
        ch: ':',
        spacing: Joint,
        span: #3 bytes(88..90),
    },
    Punct {
        ch: ':',
        spacing: Alone,
        span: #3 bytes(88..90),
    },
    Ident {
        ident: "DEPTH_COMPONENT",
        span: #3 bytes(90..105),
    },
]

And with $mod replaced by a hard-coded gl path:

TokenStream [
    Ident {
        ident: "gl",
        span: #3 bytes(84..86),
    },
    Punct {
        ch: ':',
        spacing: Joint,
        span: #3 bytes(86..88),
    },
    Punct {
        ch: ':',
        spacing: Alone,
        span: #3 bytes(86..88),
    },
    Ident {
        ident: "DEPTH_COMPONENT",
        span: #3 bytes(88..103),
    },
]

Replacing serde_derive with this proc macro still reproduces the same error:

This might be a false lead? The error is identical but it also happen with older rustc where serde succeeds. So I don’t understand what’s going on here. syn::parse_macro_input!(input as syn::DeriveInput) is the first thing that serde::Serialize does: https://github.com/serde-rs/serde/blob/v1.0.110/serde_derive/src/lib.rs#L78

@dtolnay

Could this be a duplicate of or a bug related to #72545? cc @dtolnay

@dtolnay

It does look very similar. I suspect that a change in syn similar to the one discussed at #72545 (comment) in proc-macro-hack could solve this. I tried to find the relevant source code in syn but https://github.com/dtolnay/syn/blob/6d5c90ddf790342947cad5737051f34e715c58af/src/path.rs#L35 only has a definition that a PathSegment contains an Ident. Is the parsing code macro-generated? CC @dtolnay

I released a fix to parse $mod::DEPTH_COMPONENT in syn 1.0.26.

Assigning P-critical as discussed as part of the Prioritization Working Group process and removing I-prioritize.

This seems to be the same issue as #72545 ?. And the culprit being #72388 or am I wrong?

Yeah I think we can close in favor of #72545, it's the same underlying change.

Update Rust crate syn to v2 (#396)

fix(deps): update rust crate syn to v2 (#2512)

Navigation Menu

Repository navigation

Regression: spurious "error: expected `,`" in serde::Serialize on macro_rules-variable path enum discriminant #72608

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Regression: spurious "error: expected ," in serde::Serialize on macro_rules-variable path enum discriminant #72608

Description

Activity

LeSeulArtichaut commented on May 26, 2020

SimonSapin commented on May 26, 2020

SimonSapin commented on May 26, 2020

SimonSapin commented on May 26, 2020

SimonSapin commented on May 26, 2020

SimonSapin commented on May 26, 2020

LeSeulArtichaut commented on May 26, 2020

SimonSapin commented on May 26, 2020

dtolnay commented on May 26, 2020

spastorino commented on May 27, 2020

dtolnay commented on May 27, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Regression: spurious "error: expected `,`" in serde::Serialize on macro_rules-variable path enum discriminant #72608