Skip to content

#[cfg_attr(rust_analyzer, allow(dead_code))] does not work #15612

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

Closed
smalis-msft opened this issue Sep 14, 2023 · 7 comments
Closed

#[cfg_attr(rust_analyzer, allow(dead_code))] does not work #15612

smalis-msft opened this issue Sep 14, 2023 · 7 comments
Labels
C-bug Category: bug

Comments

@smalis-msft
Copy link

smalis-msft commented Sep 14, 2023

rust-analyzer version: 0.3.1657-standalone (326f37e 2023-09-10

rustc version: rustc 1.72.0 (5680fa18f 2023-08-23)

Based on #15528 I assumed this would be possible. My code currently has a rust_analyzer_fix feature we've defined and been using for this purpose, and after trying to replace the feature with cfg(rust_analyzer) this was the only thing that didn't transition cleanly.

Repro code:
lib.rs:

#[cfg_attr(rust_analyzer, allow(dead_code))]
fn foo() {}

What I see:
image

@smalis-msft smalis-msft added the C-bug Category: bug label Sep 14, 2023
@Veykril
Copy link
Member

Veykril commented Sep 14, 2023

that diagnostic comes from rustc, rust_analyzer is not set for cargo checks we invoke though, its only set for r-a's own analysis

@smalis-msft
Copy link
Author

Should it be set? This is a lint ultimately being surfaced by rust-analyzer. As a user I don't particularly care about the mechanism, only that I've asked rust-analyzer to disable this lint and it isn't. Feels weird to me.

@Veykril
Copy link
Member

Veykril commented Sep 14, 2023

r-a invoking cargo check yields the same results as what you get when invoking it manually which is what I would expect. Additionally, if we were to set the cfg for the cargo invocation that would ultimately cause rebuilds whenever you have r-a invoke checks and you building your project yourself.

@smalis-msft
Copy link
Author

smalis-msft commented Sep 14, 2023

I know that's how it's implemented, but the presentation shouldn't necessarily be tied to the implementation IMO. To a user that doesn't know this implementation detail, this presents very weirdly. In a future where r-a doesn't need to invoke cargo check anymore, and everything is librarified, will this suddenly start working? If so, should it be working now?

@flodiebold
Copy link
Member

Maybe this shows that adding the rust_analyzer cfg was a bad idea in the first place 😬 But the main intention, I think, for the cfg was to be able to configure out parts of the code for RA's own analysis. It's not a general "when looking at code in the IDE" toggle.

@smalis-msft
Copy link
Author

Personally I feel like changing cfg(feature = "rust_analyzer_fix") (with said feature enabled in the r-a config) to cfg(rust_analyzer) should be equivalent, and in this case it isn't. I understand why, and our feature works perfectly fine for us for now so we're not unable to use r-a or anything, but it's unfortunate that there's this discrepancy.

@Veykril
Copy link
Member

Veykril commented Sep 14, 2023

Maybe this shows that adding the rust_analyzer cfg was a bad idea in the first place 😬 But the main intention, I think, for the cfg was to be able to configure out parts of the code for RA's own analysis. It's not a general "when looking at code in the IDE" toggle.

I was very reluctant to add it in general because it felt like a bad idea, but it's kind of required because of how bad r-a works in certain scenarios currently... My idea was for it to always be an escape hatch really.

Personally I feel like changing cfg(feature = "rust_analyzer_fix") (with said feature enabled in the r-a config) to cfg(rust_analyzer) should be equivalent, and in this case it isn't. I understand why, and our feature works perfectly fine for us for now so we're not unable to use r-a or anything, but it's unfortunate that there's this discrepancy.

If they were equivalent then there would be no point in even having rust_analyzer as a default cfg in the first place. The entire idea behind it was to only exist for r-a's analysis.

@Veykril Veykril closed this as completed Oct 22, 2024
@Veykril Veykril closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

3 participants