empty :vis metavariable from proc_macro breaks parsing

[ratijas]

Hi.

I filed an issue rust-lang/rust#71422 in the main rust compiler repository, but it also affects syn crate. Please, take a look at the original text. I'll only re-post most relevant parts here.

Issues with macros and :vis meta-variable

I did a lot of experiments, and I think I found three issues at once:

1. Empty :vis meta-variable
   ...

2. Forwarding empty :vis meta-variable

   When rustc forwards empty :vis meta-variable, it creates an empty proc_macro::TokenTree::Group which has no delimiter (proc_macro::Delimiter::None aka Ø) nor inner stream content (empty proc_macro::TokenStream). While None delimiters is a documented feature (quoted) "important to preserve operator priorities", they "may not survive roundtrip of a token stream through a string".

   That leads to a workaround, but at the cost of losing context and spans of tokens:

   let stream: proc_macro2::TokenStream = /* ... */;
   let stream =
       stream.to_string()
           .parse::<proc_macro2::TokenStream>()
           .unwrap();

   Live rust code using this workaround can be found in ./usage/src/main.rs (at the bottom) and ./mac/src/lib.rs (turn the USE_ROUNDTRIP switch on).

   Also, quote! macro does not produce empty groups like that, so it is higly inconsistent behavior.

   Unlike None delimiter, completely empty group is not documented as a useful feature.

3. Parsing empty group with syn crate

   Syn crate is somewhat inconsistent when it comes to parsing empty TokenTree Group.

   Steps to reproduce:

   1. Enable mac_with_ident!(bar); line in ./usage/src/with_ident.rs by commenting out/removing #[cfg!(none)] attribute.
   2. Turn off the USE_ROUNDTRIP switch in proc macro.
   3. Try to build.

   As can be seen from the error message TAG_B: Error("unexpected token"), syn::parse2 has no problems parsing syn::Visibility out of the ParseStream with single empty group, but it fails to "complete" parsing because ParseBuffer is not considered empty (it sees some "tokens" left in the buffer).

Proposal:

1. Rustc macro matching

   ...

2. Rust macro expansion

   Fix rustc macro expansion subsystem to stop forwarding empty vis tokens as an empty groups.

3. Syn parser robustness

   Regardless of fixed in rustc, syn/proc_macro2 crates should handle those empty groups gracefully AND uniformly.
   Probably, skip those blank groups altogether. It's no good that they handle a "blank group" and a "blank group which is followed by stuff" cases differently.

Meta

rustc --version --verbose:

rustc 1.42.0 (b8cedc004 2020-03-09)
binary: rustc
commit-hash: b8cedc00407a4c56a3bda1ed605c6fc166655447
commit-date: 2020-03-09
host: x86_64-unknown-linux-gnu
release: 1.42.0
LLVM version: 9.0

OS: Arch Linux
Kernel: x86_64 Linux 5.6.4-arch1-1

[ratijas]

There's a link to my repository, which may expire in future: https://github.com/ratijas/rustc-issues/tree/vis-parse

These issues are seemingly relevant:

• Public bridge between low and high level parser APIs. #479
• Inherited visibility has a misleading location #780

These lines of code are probably related:

• syn/src/buffer.rs

  Lines 66 to 71 in cac7308

  	TokenTree::Group(g) => {
  	// Record the index of the interesting entry, and store an
  	// `End(null)` there temporarially.
  	seqs.push((entries.len(), g));
  	entries.push(Entry::End(ptr::null()));
  	}

(to be updated)

[ratijas]

Rationale for fixing this in syn in addition to the compiler: crates with minimal stable rust version supported could use newer version of syn which would fix the bug. In other words: keep required minimal stable rustc version.

I'd say, generally empty groups without delimiters should be skipped by the cursor (somewhere in peeking/stepping logic) for both proc_macro and proc_macro2 parsing.

Unless, of course, somebody proves that they are actually useful, and there are other ways to work around.