Skip to content

Commit

Permalink
Auto merge of #85702 - Aaron1011:no-vec-sort, r=michaelwoerister
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed May 31, 2021
2 parents 6a3dce9 + 605513a commit 657bc01
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 30 deletions.
10 changes: 6 additions & 4 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
36 changes: 10 additions & 26 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand Down
3 changes: 3 additions & 0 deletions src/test/incremental/link_order/auxiliary/my_lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// no-prefer-dynamic
//[cfail1] compile-flags: -lbar -lfoo --crate-type lib
//[cfail2] compile-flags: -lfoo -lbar --crate-type lib
12 changes: 12 additions & 0 deletions src/test/incremental/link_order/main.rs
Original file line number Diff line number Diff line change
@@ -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() {}

0 comments on commit 657bc01

Please sign in to comment.