Multiple bindings of the same metavariable in a macro rule aren't detected until matching.

[alercah]

The following macro definition compiles:

macro_rules! bad {
  ($a:expr, $a:expr) => {}
}

Any attempt to match against this macro will result in an error, because the metavariable cannot be bound twice. There is no reason for the compiler to accept this definition as a result, since it always represents a bug, except for backwards compatibility. It should be fixed, either by assuring it won't break the ecosystem or by deprecating the behaviour and denying it the next edition.

[alercah]

Example:

macro_rules! ex {
    ($a:expr, $a:expr) => { };
}

fn main() {
    //ex!(1, 2);
}

This compiles fine. But uncommenting the invocation gives an error "duplicated bind name: 'a'". See https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=5990e9c7a1553622afbe0a7fa929d195.

[Centril]

cc @petrochenkov @rust-lang/lang

I'm not sure if this is by design or not, but it seems like a bug to me so I've marked it as such.

[petrochenkov]

Probably an accidental omission.
(Disclaimer: I never touched the code doing MBE matching and transcription.)

[Centril]

@petrochenkov do you know who to cc by any chance?

[petrochenkov]

do who to cc by any chance?

Spirits of ancestors at this point.

@mark-i-m could be familiar with that code though after implementing ?.

[cramertj]

Worth noting that if we fix this it needs a crater run, since this would cause previously-compile-able-but-unusable macro_rules variants to stop compiling, e.g.:

macro_rules! my_macro {
    (one_arg $e:expr) => { ... },
    (two_arg $e:expr, $e:expr) => { ... },
}

my_macro!(one_arg, 5);

This would all work fine, so long as no one was testing the two_arg case.

[nikomatsakis]

My impression is that this is an oversight. We could fix it, or we could just add a lint. The latter seems like a good starting point and is what I personally would do.

[mark-i-m]

I would prefer to fix it over an edition boundary and warn for now.

[mark-i-m]

I opened #57617, for the technical parts, but we need to sort out policy before I can finish it.

[Centril]

Let's do a crater run before setting out any policy. I think this is clearly a bug tho and thus an edition boundary seems excessive.

[alercah]

Waiting for an edition boundary has no value if the goal is to eventually make it deny-by-default across all editions.