rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.25k stars 1.61k forks source link

Custom derive attribute stripping is insufficient #8434

Open jonas-schievink opened 3 years ago

jonas-schievink commented 3 years ago

In

#[derive(Derive)]
#[cfg_attr(never, derive(Clone))]
struct S {
    #[cfg(never)]
    invisible_field: u8,
    #[cfg(not(never))]
    visible_field: u8,
}

The Derive macro should be passed the token stream corresponding to

struct S {
    #[cfg(not(never))]
    visible_field: u8,
}

Currently, we pass

#[cfg_attr(never, derive(Clone))]
struct S {
    #[cfg(never)]
    invisible_field: u8,
    #[cfg(not(never))]
    visible_field: u8,
}

Since we have to evalutate cfgs to do this correctly, I propose moving the remove_derive_attrs code to hir_expand.

jonas-schievink commented 3 years ago

Also, we should only be removing #[derive] attributes that come before the currently expanded derive macro, at least when https://github.com/rust-lang/rust/pull/84023 lands (see its tests)

jonas-schievink commented 3 years ago

Another relevant upstream PR: https://github.com/rust-lang/rust/pull/84110

Arjentix commented 1 year ago

Hello! Our team is struggling because of this bug. Do you plan to fix this? It's open for 2 years already.

silver-ymz commented 1 year ago

Same problem. Is there any progress on this issue?

jeff-hiner commented 1 year ago

This got worse for me about 3 or 4 weeks ago, specifically with #[cfg(target_os = ...)] fields. Maybe a regression? It's unfortunately making it impossible to see real errors in VSCode.

smalis-msft commented 1 year ago

Any updates here? This would be a big usability boost IMO.

izderadicka commented 10 months ago

Same (or very similar) problem here with serde derive, it causes no-such-field error , it's caused by field, which is behind inactive feature, line 164,165. If I enable the feature in rust-analyzer settings that it's OK. I'm almost sure that it did work some time ago - few month, so wondering why this old issue demostrated just now.

image

ssrlive commented 9 months ago

OMG, a three-year-age bug.

wyatt-herkamp commented 8 months ago

I have started implementing a fix for this. image

@rustbot claim