Skip to content

Commit

Permalink
Auto merge of #13012 - Urgau:check-cfg-fingerprint, r=epage
Browse files Browse the repository at this point in the history
Include declared list of features in fingerprint for `-Zcheck-cfg`

This PR include the declared list of features in fingerprint for `-Zcheck-cfg` (#10554)

`--check-cfg` verifies that `#[cfg()]` attributes are valid in Rust code, which includes `--cfg features foo` definitions that came from `[features]` tables in `Cargo.toml`.  For us to correctly re-check cfgs on feature changes,we need to include them in the fingerprint.

For example, if a user deletes an entry from `[features]`, they should then get warnings about any `#[cfg()]` attributes left in the code.  Without this change, that won't happen.  With this change, it now does.
  • Loading branch information
bors committed Dec 1, 2023
2 parents bbd2dcd + f32f43d commit aa6e46e
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/cargo/core/compiler/fingerprint/dirty_reason.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ pub enum DirtyReason {
old: String,
new: String,
},
DeclaredFeaturesChanged {
old: String,
new: String,
},
TargetConfigurationChanged,
PathToSourceChanged,
ProfileConfigurationChanged,
Expand Down Expand Up @@ -141,6 +145,9 @@ impl DirtyReason {
DirtyReason::FeaturesChanged { .. } => {
s.dirty_because(unit, "the list of features changed")
}
DirtyReason::DeclaredFeaturesChanged { .. } => {
s.dirty_because(unit, "the list of declared features changed")
}
DirtyReason::TargetConfigurationChanged => {
s.dirty_because(unit, "the target configuration changed")
}
Expand Down
23 changes: 23 additions & 0 deletions src/cargo/core/compiler/fingerprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
//! Target Name | ✓ | ✓
//! TargetKind (bin/lib/etc.) | ✓ | ✓
//! Enabled Features | ✓ | ✓
//! Declared Features | ✓ |
//! Immediate dependency’s hashes | ✓[^1] | ✓
//! [`CompileKind`] (host/target) | ✓ | ✓
//! __CARGO_DEFAULT_LIB_METADATA[^4] | | ✓
Expand Down Expand Up @@ -572,6 +573,8 @@ pub struct Fingerprint {
rustc: u64,
/// Sorted list of cfg features enabled.
features: String,
/// Sorted list of all the declared cfg features.
declared_features: String,
/// Hash of the `Target` struct, including the target name,
/// package-relative source path, edition, etc.
target: u64,
Expand Down Expand Up @@ -876,6 +879,7 @@ impl Fingerprint {
profile: 0,
path: 0,
features: String::new(),
declared_features: String::new(),
deps: Vec::new(),
local: Mutex::new(Vec::new()),
memoized_hash: Mutex::new(None),
Expand Down Expand Up @@ -922,6 +926,12 @@ impl Fingerprint {
new: self.features.clone(),
};
}
if self.declared_features != old.declared_features {
return DirtyReason::DeclaredFeaturesChanged {
old: old.declared_features.clone(),
new: self.declared_features.clone(),
};
}
if self.target != old.target {
return DirtyReason::TargetConfigurationChanged;
}
Expand Down Expand Up @@ -1200,6 +1210,7 @@ impl hash::Hash for Fingerprint {
let Fingerprint {
rustc,
ref features,
ref declared_features,
target,
path,
profile,
Expand All @@ -1215,6 +1226,7 @@ impl hash::Hash for Fingerprint {
(
rustc,
features,
declared_features,
target,
path,
profile,
Expand Down Expand Up @@ -1431,6 +1443,9 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
allow_features.hash(&mut config);
}
let compile_kind = unit.kind.fingerprint_hash();
let mut declared_features = unit.pkg.summary().features().keys().collect::<Vec<_>>();
declared_features.sort(); // to avoid useless rebuild if the user orders it's features
// differently
Ok(Fingerprint {
rustc: util::hash_u64(&cx.bcx.rustc().verbose_version),
target: util::hash_u64(&unit.target),
Expand All @@ -1439,6 +1454,14 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
// actually affect the output artifact so there's no need to hash it.
path: util::hash_u64(path_args(cx.bcx.ws, unit).0),
features: format!("{:?}", unit.features),
// Note we curently only populate `declared_features` when `-Zcheck-cfg`
// is passed since it's the only user-facing toggle that will make this
// fingerprint relevant.
declared_features: if cx.bcx.config.cli_unstable().check_cfg {
format!("{declared_features:?}")
} else {
"".to_string()
},
deps,
local: Mutex::new(local),
memoized_hash: Mutex::new(None),
Expand Down
71 changes: 71 additions & 0 deletions tests/testsuite/check_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,77 @@ fn features_with_namespaced_features() {
.run();
}

#[cargo_test(nightly, reason = "--check-cfg is unstable")]
fn features_fingerprint() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[features]
f_a = []
f_b = []
"#,
)
.file("src/lib.rs", "#[cfg(feature = \"f_b\")] fn entry() {}")
.build();

p.cargo("check -v -Zcheck-cfg")
.masquerade_as_nightly_cargo(&["check-cfg"])
.with_stderr_contains(x!("rustc" => "cfg" of "feature" with "f_a" "f_b"))
.with_stderr_does_not_contain("[..]unexpected_cfgs[..]")
.run();

p.cargo("check -v -Zcheck-cfg")
.masquerade_as_nightly_cargo(&["check-cfg"])
.with_stderr_does_not_contain("[..]rustc[..]")
.run();

// checking that re-ordering the features does not invalid the fingerprint
p.change_file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[features]
f_b = []
f_a = []
"#,
);

p.cargo("check -v -Zcheck-cfg")
.masquerade_as_nightly_cargo(&["check-cfg"])
.with_stderr_does_not_contain("[..]rustc[..]")
.run();

p.change_file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[features]
f_a = []
"#,
);

p.cargo("check -v -Zcheck-cfg")
.masquerade_as_nightly_cargo(&["check-cfg"])
// we check that the fingerprint is indeed dirty
.with_stderr_contains("[..]Dirty[..]the list of declared features changed")
// that is cause rustc to be called again with the new check-cfg args
.with_stderr_contains(x!("rustc" => "cfg" of "feature" with "f_a"))
// and that we indeed found a new warning from the unexpected_cfgs lint
.with_stderr_contains("[..]unexpected_cfgs[..]")
.run();
}

#[cargo_test(nightly, reason = "--check-cfg is unstable")]
fn well_known_names_values() {
let p = project()
Expand Down

0 comments on commit aa6e46e

Please sign in to comment.