about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-05-31 20:03:18 +0000
committerbors <bors@rust-lang.org>2021-05-31 20:03:18 +0000
commit657bc01888e6297257655585f9c475a0801db6d2 (patch)
treea6f2d29abd336edb01822efe47d4885448d1958a
parent6a3dce99f6ee4fd1ae317fe486b214dad7f4c1ee (diff)
parent605513a513ebe9dfe4d0716c87196d99160981a8 (diff)
downloadrust-657bc01888e6297257655585f9c475a0801db6d2.tar.gz
rust-657bc01888e6297257655585f9c475a0801db6d2.zip
Auto merge of #85702 - Aaron1011:no-vec-sort, r=michaelwoerister
Don't sort a `Vec` before computing its `DepTrackingHash`

Previously, we sorted the vec prior to hashing, making the hash
independent of the original (command-line argument) order. However, the
original vec was still always kept in the original order, so we were
relying on the rest of the compiler always working with it in an
'order-independent' way.

This assumption was not being upheld by the `native_libraries` query -
the order of the entires in its result depends on the order of entries
in `Options.libs`. This lead to an 'unstable fingerprint' ICE when the
`-l` arguments were re-ordered.

This PR removes the sorting logic entirely. Re-ordering command-line
arguments (without adding/removing/changing any arguments) seems like a
really niche use case, and correctly optimizing for it would require
additional work. By always hashing arguments in their original order, we
can entirely avoid a cause of 'unstable fingerprint' errors.
-rw-r--r--compiler/rustc_interface/src/tests.rs10
-rw-r--r--compiler/rustc_session/src/config.rs36
-rw-r--r--src/test/incremental/link_order/auxiliary/my_lib.rs3
-rw-r--r--src/test/incremental/link_order/main.rs12
4 files changed, 31 insertions, 30 deletions
diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs
index bea7d0fb81f..5d8a6084f2e 100644
--- a/compiler/rustc_interface/src/tests.rs
+++ b/compiler/rustc_interface/src/tests.rs
@@ -252,7 +252,8 @@ fn test_lints_tracking_hash_different_construction_order() {
         (String::from("d"), Level::Forbid),
     ];
 
-    assert_same_hash(&v1, &v2);
+    // The hash should be order-dependent
+    assert_different_hash(&v1, &v2);
 }
 
 #[test]
@@ -491,9 +492,10 @@ fn test_native_libs_tracking_hash_different_order() {
         },
     ];
 
-    assert_same_hash(&v1, &v2);
-    assert_same_hash(&v1, &v3);
-    assert_same_hash(&v2, &v3);
+    // The hash should be order-dependent
+    assert_different_hash(&v1, &v2);
+    assert_different_hash(&v1, &v3);
+    assert_different_hash(&v2, &v3);
 }
 
 #[test]
diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs
index 1c6fad2ae8e..52f8b536f4a 100644
--- a/compiler/rustc_session/src/config.rs
+++ b/compiler/rustc_session/src/config.rs
@@ -2427,22 +2427,6 @@ crate mod dep_tracking {
         )+};
     }
 
-    macro_rules! impl_dep_tracking_hash_for_sortable_vec_of {
-        ($($t:ty),+ $(,)?) => {$(
-            impl DepTrackingHash for Vec<$t> {
-                fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
-                    let mut elems: Vec<&$t> = self.iter().collect();
-                    elems.sort();
-                    Hash::hash(&elems.len(), hasher);
-                    for (index, elem) in elems.iter().enumerate() {
-                        Hash::hash(&index, hasher);
-                        DepTrackingHash::hash(*elem, hasher, error_format);
-                    }
-                }
-            }
-        )+};
-    }
-
     impl_dep_tracking_hash_via_hash!(
         bool,
         usize,
@@ -2491,16 +2475,6 @@ crate mod dep_tracking {
         TrimmedDefPaths,
     );
 
-    impl_dep_tracking_hash_for_sortable_vec_of!(
-        String,
-        PathBuf,
-        (PathBuf, PathBuf),
-        CrateType,
-        NativeLib,
-        (String, lint::Level),
-        (String, u64)
-    );
-
     impl<T1, T2> DepTrackingHash for (T1, T2)
     where
         T1: DepTrackingHash,
@@ -2530,6 +2504,16 @@ crate mod dep_tracking {
         }
     }
 
+    impl<T: DepTrackingHash> DepTrackingHash for Vec<T> {
+        fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
+            Hash::hash(&self.len(), hasher);
+            for (index, elem) in self.iter().enumerate() {
+                Hash::hash(&index, hasher);
+                DepTrackingHash::hash(elem, hasher, error_format);
+            }
+        }
+    }
+
     // This is a stable hash because BTreeMap is a sorted container
     crate fn stable_hash(
         sub_hashes: BTreeMap<&'static str, &dyn DepTrackingHash>,
diff --git a/src/test/incremental/link_order/auxiliary/my_lib.rs b/src/test/incremental/link_order/auxiliary/my_lib.rs
new file mode 100644
index 00000000000..57cde5f7c6e
--- /dev/null
+++ b/src/test/incremental/link_order/auxiliary/my_lib.rs
@@ -0,0 +1,3 @@
+// no-prefer-dynamic
+//[cfail1] compile-flags: -lbar -lfoo --crate-type lib
+//[cfail2] compile-flags: -lfoo -lbar --crate-type lib
diff --git a/src/test/incremental/link_order/main.rs b/src/test/incremental/link_order/main.rs
new file mode 100644
index 00000000000..d211c295bc4
--- /dev/null
+++ b/src/test/incremental/link_order/main.rs
@@ -0,0 +1,12 @@
+// aux-build:my_lib.rs
+// error-pattern: error: linking with
+// revisions:cfail1 cfail2
+// compile-flags:-Z query-dep-graph
+
+// Tests that re-ordering the `-l` arguments used
+// when compiling an external dependency does not lead to
+// an 'unstable fingerprint' error.
+
+extern crate my_lib;
+
+fn main() {}