Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macro_rules! and empty :vis metavariables #71422

Open
ratijas opened this issue Apr 22, 2020 · 1 comment
Open

macro_rules! and empty :vis metavariables #71422

ratijas opened this issue Apr 22, 2020 · 1 comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ratijas
Copy link
Contributor

ratijas commented Apr 22, 2020

Hi.
I founds some bugs, did a research, and created a repository with reproducible examples, README, and instructions. Here is the content of README, and the whole repo is attached as zip archive. Code is highly documented, so please take a look. Code snippets and rust-playground are mostly useless, because we'are dealing with proc_macro multi-crate builds.

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

    According to the documentation, :vis entity in macro_rules! should match empty visibility modifier:
    vis: a possibly empty Visibility qualifier

    While this is the case when visibility modifier is followed by some more stuff, it fails on its own:

    macro_rules! q1 {
        ($visibility:vis) => {}
    }
    
    q1!(); // error: unexpected end of macro invocation

    Followed by stuff:

    macro_rules! q2 {
        ($visibility:vis $item:ident) => {}
    }
    
    q2!(foobar); // ok
    

    Even though, adding ident-only branch still matches the first one (with :vis):

    macro_rules! q3 {
        ($visibility:vis $item:ident) => {};
        ($item:ident) => {
            compile_error!("Ident-only branch chosen instead of branch with empty vis");
        };
    }
    
    q3!(foobar); // ok
    

    Live rust code of these examples can be found in ./mac/src/q.rs.

  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

    Fix rustc macro matching subsystem to accept calls without arguments (e.g. q!()) as matching arms with single :vis meta-variable (e.g. macro_rules! q { ($v:vis) => {} })

  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 ratijas added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 22, 2020
@ratijas
Copy link
Contributor Author

ratijas commented Apr 22, 2020

A link to my repository, which may expire: https://github.com/ratijas/rustc-issues/tree/vis-parse

Also, A-macros label definitely fits here, but I have no permissions to assign them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants