Auto merge of #13012 - Urgau:check-cfg-fingerprint, r=epage

bors · bors · commit aa6e46e8cabd · 2023-12-01T21:46:28.000Z
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.
diff --git a/src/cargo/core/compiler/fingerprint/dirty_reason.rs b/src/cargo/core/compiler/fingerprint/dirty_reason.rs
@@ -15,6 +15,10 @@ pub enum DirtyReason {
         old: String,
         new: String,
     },
+    DeclaredFeaturesChanged {
+        old: String,
+        new: String,
+    },
     TargetConfigurationChanged,
     PathToSourceChanged,
     ProfileConfigurationChanged,
@@ -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")
             }
diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs
@@ -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]           |             | ✓
@@ -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,
@@ -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),
@@ -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;
         }
@@ -1200,6 +1210,7 @@ impl hash::Hash for Fingerprint {
         let Fingerprint {
             rustc,
             ref features,
+            ref declared_features,
             target,
             path,
             profile,
@@ -1215,6 +1226,7 @@ impl hash::Hash for Fingerprint {
         (
             rustc,
             features,
+            declared_features,
             target,
             path,
             profile,
@@ -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),
@@ -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),
diff --git a/tests/testsuite/check_cfg.rs b/tests/testsuite/check_cfg.rs
@@ -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()
