Panic iterating over proc_macro::TokenStream

[dtolnay]

In the following proc macro invocation the compiler passes in the input $crate as a TokenStream but panics when iterating over that TokenStream. Specifically the panic happens in next().

rustc 1.31.0-nightly (de9666f 2018-10-31)

repro/src/lib.rs

extern crate proc_macro;
use self::proc_macro::TokenStream;

#[proc_macro]
pub fn repro(input: TokenStream) -> TokenStream {
    input.into_iter().next();
    unimplemented!()
}

testing/src/lib.rs

macro_rules! m {
    () => {
        repro::repro!($crate);
    };
}

m!();

error: proc macro panicked                                                                                                     
 --> src/main.rs:3:9                                                                                                           
  |                                                                                                                            
3 |         repro::repro!($crate);                                                                                             
  |         ^^^^^^^^^^^^^^^^^^^^^^                                                                                             
...                                                                                                                            
7 | m!();                                                                                                                      
  | ----- in this macro invocation                                                                                             
  |                                                                                                                            
  = help: message: `"$crate"` is not a valid identifier

Originally reported in dtolnay/paste#3. Mentioning @softprops.

[eddyb]

cc @petrochenkov Looks like $crate is a single ident so it need splitting.

[dtolnay]

Don't split it, allow $crate as a single ident. Macro_rules does this correctly.

macro_rules! is_ident {
    ($i:ident) => {};
}

macro_rules! check {
    () => {
        is_ident!($crate);
    };
}

check!();

Otherwise all proc macro parsing code will need separate cases for many places that an ident is parsed or stored to also accept $ crate.

[petrochenkov]

This is a tricky case, yes.
$crate produced by declarative macros is accepted by the ident matcher (including in path segments), so it should somehow act as a single identifier at least for compatibility.

[petrochenkov]

Derive macro infra currently does a horrible thing called eliminate_crate_var replacing the $crate segment either with a crate name or with nothing (depending on the context) before passing tokens to the macro.
Fn-like and attribute macros don't do that (thankfully).

@dtolnay
How derive macros treat the is_ident!($crate) case when the $crate is turned into nothing?
Does it happen in practice?

[dtolnay]

I believe we don't currently have any code in any proc macro library specifically for accommodating $crate.

[petrochenkov]

There's one thing I'm sure in - $crate after processing by the decl-macro it belongs to is no longer two tokens, it's one token - reserved/keyword identifier - in all respects.
It's very similar to crate, but slightly different in terms of resolution (Note: I don't think crate with tweaked span/hygiene can emulate $crate, unfortunately).

If we introduced a new keyword, say dollar_crate, with the same meaning, we could easily replace the "processed $crate" with it.
The obvious solution is to extend the lexical grammar of identifiers, relax validation in Ident::new, and treat "$crate" itself as such keyword.
It just doesn't feel right though, e.g. one issue with it is that such "keyword" can't survive stringification and reparsing.

[dtolnay]

I would be fine with accepting "$crate" in Ident::new. Lots of things don't survive stringification and reparsing so that shouldn't be a requirement.

[petrochenkov]

Some status update: the ICE no longer happens because #49219 removed all the validation for tokens coming to proc macros from the compiler, but the underlying issue is still there.

[eddyb]

@petrochenkov Wait there was validation? Where exactly, I might've missed it?

[petrochenkov]

Ident::new(...)/Punct::new(...) instead of unchecked Ident { /* fields */ }/Punct { /* fields */ } like it's done now in proc_macro_server.rs / fn from_internal.