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

Shadowed macro causes macro-error #7084

Closed
vallentin opened this issue Dec 30, 2020 · 19 comments
Closed

Shadowed macro causes macro-error #7084

vallentin opened this issue Dec 30, 2020 · 19 comments
Labels
A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now

Comments

@vallentin
Copy link

vallentin commented Dec 30, 2020

The first foo!(1); uses to the subsequent shadowed macro_rules! foo. Causing expected leaf: `,` rust-analyzer(macro-error). Performing "Go to Definition" confirms this, as the cursor jumps to the second macro_rules! foo.

fn main() {
    macro_rules! foo {
        ($a:expr) => {};
    }

    foo!(1);

    macro_rules! foo {
        ($a:expr, $b:expr) => {};
    }

    foo!(1, 2);
}

image

This issue must have been introduced lately, as a few weeks ago this was not an issue.

Issue exists on both stable and the latest nightly release:

  • rust-analyzer version: 2020-12-28 (1d53075)
  • rust-analyzer version: nightly (77ad203)
@lnicola lnicola added A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now labels Dec 30, 2020
@edwin0cheng
Copy link
Member

triage: I can't reproduce in current master 8584d26.

@vallentin Would you mind to try it again ?

@vallentin
Copy link
Author

It still occurs on the latest releases. However, that's of course prior to 8584d26. Can I easily obtain a build, that doesn't require me building rust-analyzer? 😅

rust-analyzer version: 2021-01-04 (5b86ff3)
rust-analyzer version: nightly (a3e5dcc)

@lnicola
Copy link
Member

lnicola commented Jan 8, 2021

@edwin0cheng 8584d26 isn't on master, you meant #7145.

@vallentin not really, you'd need to wait until tomorrow. Building from source isn't that hard, though.

(I can't reproduce it either)

Also, that's a lovely issue report, I wish we had more of these.

@edwin0cheng
Copy link
Member

edwin0cheng commented Jan 8, 2021

Sorry, @lnicola is right and I meant #7145.

However, I tried a3e5dcc and 5b86ff3, I can't reproduce the errors too, so ... What os and ra settings you are using ?

Also, that's a lovely issue report, I wish we had more of these.

Indeed

@vallentin
Copy link
Author

It wasn't the difficulty, it's more that I'm expecting a clean build to take quite a bit of time.

I'm running Windows 10. I tested the above using only default settings, with the exception of "rust-analyzer.updates.channel": "nightly" to test the nightly release.

@flodiebold
Copy link
Member

A clean release build takes about 5 minutes.

@vallentin
Copy link
Author

Just under 3 minutes, nice. Alright, so I've just tried using #7145. Sadly, the issue still persists.

Checking "Show RA Version" show the expected commit. So I sadly don't think that fixes this issue.

rust-analyzer version: 2021-01-04 (76f2b9d)

@flodiebold
Copy link
Member

I can't reproduce the error in current master either, and changing the second macro definition to take just one argument does cause an error only in the second call; though it's worth noting that goto definition, hover etc. refer to the wrong macro for the first definition.

@vallentin
Copy link
Author

vallentin commented Jan 8, 2021

Hmm, that's odd. I assume you aren't using Windows then?

However, I don't at all see how this would be affected by OS, so it's rather confusing.

Is there any more information I can provide?


Here's step-by-step what I do to reproduce the issue (on Windows 10):

git clone https://github.com/rust-analyzer/rust-analyzer
cd rust-analyzer
cargo build --release
cp target/release/rust-analyzer.exe ~/AppData/Roaming/Code/User/globalStorage/matklad.rust-analyzer/rust-analyzer-x86_64-pc-windows-msvc.exe

cd ..
cargo new foo
code foo

Then paste the following code in src/main.rs.

fn main() {
    macro_rules! foo {
        ($a:expr) => {};
    }

    foo!(1);

    macro_rules! foo {
        ($a:expr, $b:expr) => {};
    }

    foo!(1, 2);
}

Checking "Show RA Version" correctly shows:

rust-analyzer version: 2021-01-04 (903d1f8)


Here's a screenshot showing both the issue and the version:

(All default settings.)

image

@flodiebold
Copy link
Member

Oh, putting all that code into main makes a big difference. Local definitions are mostly not supported yet.

@vallentin
Copy link
Author

I literally just had the same thought pop into my head after commenting. Now I feel silly!

How come this worked previously?

@flodiebold
Copy link
Member

It didn't, we just didn't emit those macro errors until recently.

@lnicola
Copy link
Member

lnicola commented Jan 8, 2021

That's #1559.

@vallentin
Copy link
Author

Is there any way to suppress that error?

I have a few files that are affected by this change. Which prevents any other errors from being detected.

@flodiebold
Copy link
Member

flodiebold commented Jan 8, 2021

You can add "macro-error" to the rust-analyzer.diagnostics.disable setting.

This shouldn't prevent other errors from being detected though.

@vallentin
Copy link
Author

Thanks!

The files where the issue occurs in conjunction are quite big. I'll look into making a MCVE, and post it if/when I'm able to.

@jonas-schievink
Copy link
Contributor

Triage: Still happening, presumably because we don't really handle legacy macro scopes in Resolver

@lnicola
Copy link
Member

lnicola commented Sep 1, 2024

This works now AFAICT.

@lnicola lnicola closed this as completed Sep 1, 2024
@Veykril
Copy link
Member

Veykril commented Sep 1, 2024

🤨 oh it does, but why/how?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

6 participants