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

"expected none or () delimited group, but found different token tree (note: do not use the macros from bunt-macros directly, but only through bunt)" #20

Closed
d4h0 opened this issue Feb 25, 2021 · 10 comments

Comments

@d4h0
Copy link

d4h0 commented Feb 25, 2021

Hi,

When macro expansion is activated in rust-analyser in VSCode, I get the following error, every time I use bunt::println!:

expected none or () delimited group, but found different token tree (note: do not use the macros from bunt-macros directly, but only through bunt)

Is this somehow fixable?

Macro expansion is useful for completion of types that are created via proc macros (e.g. typed_builder), but now my code is littered with error messages that the compiler itself doesn't report as errors.

@LukasKalbertodt
Copy link
Owner

LukasKalbertodt commented Feb 25, 2021

I reproduced the problem locally. As to why this happens: good question! I don't know. I don't really know a lot about rust-analyzer and haven't used its proc macro expansion features yet. I would assume it's still not "finished" and that this is just a bug.

So what's happening with bunt is that bunt::println is actually a declarative macro that slightly transforms the input and passes it to the proc macro bunt_macros::writeln. See here. It seems like rust-analyzer ignores this transformation done by the declarative macro and just passes the original input token stream to the proc macro. This causes the error. But I have no idea why that would happen. Does rust-analyzer support declarative macro expansion yet?

So I'm pretty sure this is not a bug or problem in bunt, but if I can change bunt in a way that helps out rust-analyzer, I'm happy to do that.

Please excuse me pinging you directly here, but maybe @jonas-schievink could help us out here?

@jonas-schievink
Copy link

This certainly seems like a bug in rust-analyzer, but I'm not immediately sure what's causing it.

@jonas-schievink
Copy link

It also happens when calling the procedural macro directly:

    bunt_macros::writeln!(
        (bunt::termcolor::StandardStream::stdout(bunt::termcolor::ColorChoice::Auto))
        ["asdasd"]
    );

@LukasKalbertodt
Copy link
Owner

A closer manual invocation would be:

    bunt_macros::writeln!(
        (bunt::termcolor::StandardStream::stdout(bunt::termcolor::ColorChoice::Auto))
        [("hello")]
    );

It's a bit strange, but the [$($format_str)+] (in the definition of bunt::println!) will result in a token stream of:

Group {
    delimiter: Some('['),
    inner: Group {
       delimiter: None,
       inner: ...,
    }
}

Maybe rust-analyzer does not add this "dummy group"? I think the behavior changed recently: rust-lang/rust#73084

But I guess I could also change bunt-macros to optionally accept a None-delimited group there. That's probably the easiest fix.

@jonas-schievink
Copy link

Why is there an additional () around the format string? I only see that as part of the $(...)+ repetition operator, which should not end up in the output

LukasKalbertodt added a commit that referenced this issue Feb 25, 2021
@LukasKalbertodt
Copy link
Owner

I managed to get a fairly minimal example to reproduce this problem. You can see it on this branch. We have three crates: bunt-macros (exporting proc macro), bunt (exporting decl macro) and the simple example.

lib.rs of bunt-macros

use proc_macro::TokenStream;

#[proc_macro]
pub fn writeln(input: TokenStream) -> TokenStream {
    panic!("{:#?}", input);
}

lib.rs of bunt:

pub extern crate bunt_macros;

#[macro_export]
macro_rules! foo {
    ($target:expr) => {
        $crate::bunt_macros::writeln!($target)
    };
}

simple.rs

fn main() {
    bunt::foo!(peter);
}

If you cargo build --example simple, you get this output:

error: proc macro panicked
 --> examples/simple.rs:3:5
  |
3 |     bunt::foo!(peter);
  |     ^^^^^^^^^^^^^^^^^^
  |
  = help: message: TokenStream [
    Group {
        delimiter: None,
        stream: TokenStream [
            Ident {
                ident: "peter",
                span: #0 bytes(28..33),
            },
        ],
        span: #6 bytes(8793927..8793934),
    },
]

However, if I just save simple.rs in VScode with RA proc macro expansion activated, it shows this error (when hovering over the macro call):

proc macro returned error: proc-macro panicked: TokenStream [
    Ident {
        ident: "peter",
        span: 4294967295,
    },
] rust-analyzermacro-error

RA seems to not add this "dummy group" when evaluating the decl macro.

But the most interesting part: if I rename writeln to bar (in both lib.rs), does not show the error anymore! It seems to give up on evaluating the macros. Instead, it only shows the output of cargo check, which also contains the dummy group.
Does RA treat writeln in a special way?


@jonas-schievink Just to be clear, I don't expect you to help out here. Also, if I should just open an issue at the RA repo, let me know!

@jonas-schievink
Copy link

But the most interesting part: if I rename writeln to bar (in both lib.rs), does not show the error anymore! It seems to give up on evaluating the macros. Instead, it only shows the output of cargo check, which also contains the dummy group.

This might just be because we don't reload proc. macros.

Thanks for finding that reproduction! It would be great if you could open a rust-analyzer issue with that.

@LukasKalbertodt
Copy link
Owner

This might just be because we don't reload proc. macros.

Ouch. I regularly restarted RA while building the minimal example, but of course I forgot to restart it after renaming that. So yeah, you seem to be right. Once RA is reloaded, I see the same behavior for bar.

I created the RA issue.

@LukasKalbertodt
Copy link
Owner

I checked the parsing code again and while some of the None delimited groups are not required, others are. So I decided not to implement a workaround in bunt. It would be a bit too much hassle for a fix like that. So we have to wait for the RA fix!

@d4h0
Copy link
Author

d4h0 commented Feb 27, 2021

@LukasKalbertodt: Okay, thank you for investigating this. It looks like I should have reported this directly to RA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants