find macro-feature occurrences within macro args

[pnkfelix]

In feature_gate::MacroVisitor, find feature occurrences within macro arguments.

Fix #22234

[rust-highfive]

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

[huonw]

This doesn't quite catch everything, e.g.

macro_rules! foo {
     ($x: ident) => {
         $x!("")
     }
}

foo!(asm)

is valid, AFAIK.

[pnkfelix]

Note: I am not actually thrilled with the approach I used here; it "works", and we may want to include it nonetheless.

But I probably need (for other checks, namely the check for placement-in and overloaded-box) to take a different approach. E.g. inject a marker (saying that the feature had been used) into the generated AST, as outlined in my comment here: #22234 (comment)

(And if I do the latter, then the fix given here has a lower priority.)

[pnkfelix]

@huonw yes, it doesn't catch that. The main thing it catches that my "inject-a-marker" approach would not catch is a scenario where the user has a macro that discards its input. (And that probably is not so important to catch.)

The other thing I have been wondering is if there are legitimate macros that this check would erroneously reject.

[huonw]

Oh, I just thought of an alternate way to do this sort of feature gating: just register the existence of the various macro-related feature(...)s in a struct like the Features one that already exists, and have each macro do the feature gating check internally. That is, change e.g. asms definition to start like:

pub fn expand_quote_tokens<'cx>(cx: &'cx mut ExtCtxt,
                                sp: Span,
                                tts: &[ast::TokenTree])
                                -> Box<base::MacResult+'cx> {

    cx.sess.macro_features.gate(/* name of the feature */ "asm", /* name of the macro */ "asm");

where the macro_featuress field is storing what I mentioned above.

This is then "perfect": any expansion of such a macro will be caught.

The discarding-input macro is a possible downside, as well as being slightly error prone since we have to remember to insert the check (but it seems any scheme has this).

[nikomatsakis]

nit: indentation

[nikomatsakis]

@huonw huh. I always thought of foo! as a single token. I'm...not thrilled that this is not the case. (In particular, it seems to make possible future keyword and macro overlap mildly more troublesome.)

[nikomatsakis]

So, this code seems fine, though the technique @huonw suggests seems more robust.

[pnkfelix]

@nikomatsakis Indeed, I am in the process of implementing a feature much like the one @huonw suggested. (It is necessary for properly feature-gating the desugaring-based box and in.)

(We may still want to do something like this PR, because occurrences of the macro that are hidden under #[cfg(..)] and also under a macro argument will not be caught. But then again, that seems like a sufficiently remote corner case that we can live with it.)

[pnkfelix]

(hein, I'll just close this PR; I think I have something close to working for the other approach, and I am not really convinced by the corner case I outlined above.)