macros sometimes allow expr followed by ident

[durka]

You can use a repetition to get around the future proofer.

macro_rules! bad {
    ($e:expr $i:ident) => {} //~ERROR
}

macro_rules! sneaky {
    ($($i:ident $e:expr)*) => {} // no error
}

fn main() {
    sneaky!(a b c d);
}

I feel that both of these should be accepted, or neither. BTW, I am using the equivalent of sneaky! in brainmunch (found this while preparing my RustFest talk).

Fixing this by disallowing both macros would require cratering.

cc @jseyfried @LeoTestard

[aturon]

cc @pnkfelix

[nikomatsakis]

The danger here is that contextual keywords become impossible. In particular, consider if we added await f as an expression, which would work with bad! today but fail tomorrow.

[joshtriplett]

One possibility would be to leave this alone and fix it in macros 2.0. The other would be a crater run and evaluation for a fix.

[nikomatsakis]

Some possible routes:

• I don't think we should accept bad!, for the reasons given above.
• We could leave this as is -- we have said in the past that when you break the forwards compatibility rules "in spirit" you may get broken.
• Or, better really, we would do a fix and try to do a crater run.

[nikomatsakis]

triage: P-medium

[nikomatsakis]

I feel like we should at least try the fix -- though it may be hard to ascertain the impact entirely.

[durka]

@nikomatsakis do you have any idea how to fix it?

[estebank]

I'm afraid fixing this is going to be problematic, because even we are using this "feature" in rustc. I'll post a PR with the fix (including changing our mis-uses), but will expect there to be breakage. If that is the case after a crater run, should we still include the fix as a warning?

[durka]

Wow, that's a lot of fallout in the PR! IMO yes, assuming crater shows nonzero breakage, add a warning at the definition site (callers too?) for macros 1.0 and fix it in macros 2.0.

…

On Thu, Oct 25, 2018, 22:14 Esteban Kuber ***@***.***> wrote: I'm afraid fixing this is going to be problematic, because even *we* are using this "feature" in rustc <https://github.com/rust-lang/rust/blob/4bd4e4130ed531a644263db26bf8461704215c77/src/libcore/iter/range.rs#L75>. I'll post a PR with the fix (including changing our mis-uses), but will expect there to be breakage. If that is the case after a crater run, should we still include the fix as a warning? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#44975 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3n0xEXdHT1jR698kC1GUcON_vvt6fks5uom_qgaJpZM4PqiGz> .

[estebank]

(callers too?)

@durka It'd be nice to do, but I'm tempted at leaving devs to deal with the fallout after changing the signature by following the errors they'll get then instead of trying to preempt them, as extending the diagnostic to the callers would be a bigger PR, I fear.