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

"Failed to parse macro invocation" when using puffin (regression) #6811

Closed
hrydgard opened this issue Dec 11, 2020 · 6 comments · Fixed by #6886
Closed

"Failed to parse macro invocation" when using puffin (regression) #6811

hrydgard opened this issue Dec 11, 2020 · 6 comments · Fixed by #6886
Assignees
Labels
A-macro macro expansion

Comments

@hrydgard
Copy link

hrydgard commented Dec 11, 2020

Puffin is a nice little instrumented profiling library by @emilk .

It used to work well with rust-analyzer, but doesn't anymore since 0.2.408 or a very nearby version.

image

Complete repro

cargo.toml:

[package]
name = "repro"
[dependencies]
puffin = "0.3.1"

main.rs

fn main() {
    puffin::profile_function!();
}

The offending macro looks like this:

#[macro_export]
macro_rules! profile_function {
    () => {
        let _profiler_scope = if $crate::are_scopes_on() {
            Some($crate::ProfilerScope::new(
                $crate::current_function_name!(),
                $crate::current_file_name!(),
                "",
            ))
        } else {
            None
        };
    };
}

which doesn't look all that complicated but does call other macros from the macro - related to #6788 ? (EDIT: no, doesn't seem to be)

@emilk
Copy link

emilk commented Dec 11, 2020

Direct link to all the macros in puffin: https://github.com/EmbarkStudios/puffin/blob/main/puffin/src/lib.rs#L364

@edwin0cheng
Copy link
Member

edwin0cheng commented Dec 11, 2020

Reproduction :

macro_rules! profile_function {
    () => {
        let _a = if true {
            Some(1)
        } else {
            None
        };
    };
}

fn main() {
    profile_function!();
    // ^^ failed to parse macro invocation
}

@edwin0cheng
Copy link
Member

edwin0cheng commented Dec 11, 2020

The simplest form :

macro_rules! profile_function {
    () => {
        let _a = 1;
    };
}

fn main() {
    profile_function!();
    // ^^ failed to parse macro invocation
}

It seem to be related to this line:

https://github.com/rust-analyzer/rust-analyzer/blob/41321d96789ed918eebda02ada76758765d19d16/crates/hir_expand/src/db.rs#L397-L398

And because it is a statement (let _a = 1;) and then parser reject it as expr.

@edwin0cheng
Copy link
Member

@flodiebold I just checked that FIXME was written by you but I forget what it was meant. Could you shed some light about it ?

@flodiebold
Copy link
Member

IIRC the problem is that during body expansion, we need to detect whether the macro call is in a statement position, and then let it expand to statements instead of an expression and handle that case.

@flodiebold
Copy link
Member

Basically here we are already in an expression context, so we can't just expand to statements:
https://github.com/rust-analyzer/rust-analyzer/blob/41321d96789ed918eebda02ada76758765d19d16/crates/hir_def/src/body/lower.rs#L550

So I guess we'd actually need to add a special case here to see if this is a top-level macro call, so we can let it expand to statements.

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

Successfully merging a pull request may close this issue.

5 participants