Repository navigation
Incorrect clippy::derivable_impls in the presence of conditional compilation #13160
Description
Summary
Clippy will happily change the meaning of code using conditional compilation inside a custom Default when using --fix. The code compiles but is now incorrect.
Lint Name
clippy::derivable_impls
Reproducer
I tried this code:
/// Package manager to use
#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
pub enum ConcreteBackend {
#[cfg(feature = "arch_linux")]
Pacman,
#[cfg(feature = "debian")]
Apt,
Flatpak,
}
// Clippy is wrong, this cannot be derived due to the cfg_if
impl Default for ConcreteBackend {
fn default() -> Self {
cfg_if::cfg_if! {
if #[cfg(feature = "arch_linux")] {
ConcreteBackend::Pacman
} else if #[cfg(feature = "debian")] {
ConcreteBackend::Apt
} else {
ConcreteBackend::Flatpak
}
}
}
}I saw this happen:
With --fix it creates the following code that is not equivalent (when no features are enabled, you will get different variants of this depending on what features are enabled)
/// Package manager to use
#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
#[derive(Default)]
pub enum ConcreteBackend {
#[cfg(feature = "arch_linux")]
Pacman,
#[cfg(feature = "debian")]
Apt,
#[default]
Flatpak,
}The reported clippy message without --fix is:
Checking repro v0.1.0 (/home/arvid/src/repro)
warning: this `impl` can be derived
--> src/main.rs:12:1
|
12 | / impl Default for ConcreteBackend {
13 | | fn default() -> Self {
14 | | cfg_if::cfg_if! {
15 | | if #[cfg(feature = "arch_linux")] {
... |
23 | | }
24 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls
= note: `#[warn(clippy::derivable_impls)]` on by default
= help: remove the manual implementation...
help: ...and instead derive it...
|
3 + #[derive(Default)]
4 | pub enum ConcreteBackend {
|
help: ...and mark the default variant
|
8 ~ #[default]
9 ~ Flatpak,
|
warning: `repro` (bin "repro") generated 1 warning (run `cargo clippy --fix --bin "repro"` to apply 1 suggestion)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
I expected to see this happen:
Either Clippy should use cfg_attr here on default, or if that is too complicated it should detect that it breaks the code and refuse to auto-fix it.
Version
rustc 1.80.0 (051478957 2024-07-21)
binary: rustc
commit-hash: 051478957371ee0084a7c0913941d2a8c4757bb9
commit-date: 2024-07-21
host: x86_64-unknown-linux-gnu
release: 1.80.0
LLVM version: 18.1.7
Additional Labels
@rustbot label +I-suggestion-causes-bug
Reacted by lolbinarycat, Hellzbellz123 and Tim Neumann
Activity
estebank commentedon Dec 12, 2024
Because of the way [the macro expansion for cfg-if works], it is not possible to detect post-expansion today, but it will be with rust-lang/rust#133823, which adds a placeholder attribute to every item that has been left behind after a "positively evaluated"
cfg. By looking for the presence of that attribute, clippy can then decide not to touch theimpland avoid linting that case.TimNN commentedon May 4, 2025
As a similar case, I have code that looks similar to
bool_field: cfg!(feature = "hello")in aDefaultimpl, which also triggers a false-positive from this lint.estebank commentedon Jun 3, 2025
Took a quick look at this again, and the issue is that what you have in the lint is the HIR, but I don't think the placeholder attribute makes it past the AST. We might have to change that (on the compiler side, maybe with another side-table? or on clippy's side by collecting these during the run of early-lints?) for this lint to know about
cfgs.ada4a commentedon Aug 16, 2025
@rustbot label S-blocked