macro future proofing: extend analysis to detect unfortunate interaction across arms

[pnkfelix]

spawned off of rust-lang/rust#30531 (comment)

(transcribed relevant material from that comment below.)

when we released 1.0, we set some parameters for what sorts of changes we could make to the Rust grammar. In particular, for things like expressions and types, which can be referenced from macro definitions, we established (in RFC 550) a set of legal "follow" tokens that must never be added to the grammar. For example, the follow set for an expression was defined as =>, ,, and ;. These sets were derived by looking at the existing Rust grammar and finding expression terminators we were already committed to: for example, expressions can already be followed by , in a tuple expression or function call. Under those rules, the change to implement type ascription is legal.

RFC 550 also defined a static analysis that was intended to reject macro definitions which might be affected by (legal) changes to the expression grammar. For example, that analysis would reject a macro arm such as $e:expr : $t:ty because, there, $e is followed by :, which is not in the follow set for an expression. This means that if we were to add something like type ascription, the macro would break, and we did not want that to happen. However, as this issue demonstrates, that static anlysis contains a "soundness bug" when it comes to multiple macro arms.

In the medium to long term, we need to patch RFC 550 to correctly account for multiple macro arms. In the short term, however, macro authors will have to look at their own macros to check whether they might be broken by changes to the extension grammar.

[pnkfelix]

cc #550

[durka]

So from the previous issue, it seems the goal is to look at these two macro arms

($e:expr) => { ... }
($i:ident : $t:ty => { ... }

and detect that the same input could be valid for both arms (assuming #![feature(type_ascription)], in this example). Depending on whether you count type ascription syntax as already added or not, this becomes either a future-proofing hazard or a straight-up ambiguity which makes the meaning of the macro depend on the order of the arms.

We have to be careful though, because macro evaluation is already dependent, and usefully so, on the order of the arms, for example this pattern:

(A ...) => { /* case A */ }
(B ...) => { /* case B */ }
($i:ident ...) => { /* other */ }

[durka]

Also, are there ideas on how to do the actual ambiguity checking? Not sure if we know what class of parsers macro_rules! creates, but the problem may not even be decidable...

[pnkfelix]

the problem may not even be decidable

I haven't put much thought into it, but I am willing to use a conservative analysis that just emits a lint style warning to the macro author. (And never make it an error by default in rustc)

(Also, another potential option that has been suggested: expand the macro input matching system so that the expander can "back out" of one arm and try the next. (Ie it becomes a backtracking parser) I am not saying the latter is my preferred solution, but it will be good for us to consider all options when we tackle this problem. )

[DanielKeep]

Backtracking would definitely be the preferred solution. Without it, for example, there's no way to parse generic parameters in macro_rules! even if we did have a lifetime matcher.

[pnkfelix]

cc #1384