Removed stable/unstable sort arg from into_sorted_stable_ord, fixed a few misc issues, added collect to UnordItems

This commit is contained in:
Andrew Xie 2023-06-08 00:38:50 -04:00
parent f5f638c124
commit 54d7b327e5
13 changed files with 81 additions and 48 deletions

View file

@ -414,7 +414,9 @@ pub struct Size {
// Safety: Ord is implement as just comparing numerical values and numerical values // Safety: Ord is implement as just comparing numerical values and numerical values
// are not changed by (de-)serialization. // are not changed by (de-)serialization.
#[cfg(feature = "nightly")] #[cfg(feature = "nightly")]
unsafe impl StableOrd for Size {} unsafe impl StableOrd for Size {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
// This is debug-printed a lot in larger structs, don't waste too much space there // This is debug-printed a lot in larger structs, don't waste too much space there
impl fmt::Debug for Size { impl fmt::Debug for Size {

View file

@ -233,7 +233,17 @@ pub trait ToStableHashKey<HCX> {
/// - `DefIndex`, `CrateNum`, `LocalDefId`, because their concrete /// - `DefIndex`, `CrateNum`, `LocalDefId`, because their concrete
/// values depend on state that might be different between /// values depend on state that might be different between
/// compilation sessions. /// compilation sessions.
pub unsafe trait StableOrd: Ord {} ///
/// The associated constant `CAN_USE_UNSTABLE_SORT` denotes whether
/// unstable sorting can be used for this type. Set to true if and
/// only if `a == b` implies `a` and `b` are fully indistinguishable.
pub unsafe trait StableOrd: Ord {
const CAN_USE_UNSTABLE_SORT: bool;
}
unsafe impl<T: StableOrd> StableOrd for &T {
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
}
/// Implement HashStable by just calling `Hash::hash()`. Also implement `StableOrd` for the type since /// Implement HashStable by just calling `Hash::hash()`. Also implement `StableOrd` for the type since
/// that has the same requirements. /// that has the same requirements.
@ -253,7 +263,9 @@ macro_rules! impl_stable_traits_for_trivial_type {
} }
} }
unsafe impl $crate::stable_hasher::StableOrd for $t {} unsafe impl $crate::stable_hasher::StableOrd for $t {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
}; };
} }
@ -339,7 +351,9 @@ impl<T1: HashStable<CTX>, T2: HashStable<CTX>, CTX> HashStable<CTX> for (T1, T2)
} }
} }
unsafe impl<T1: StableOrd, T2: StableOrd> StableOrd for (T1, T2) {} unsafe impl<T1: StableOrd, T2: StableOrd> StableOrd for (T1, T2) {
const CAN_USE_UNSTABLE_SORT: bool = T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT;
}
impl<T1, T2, T3, CTX> HashStable<CTX> for (T1, T2, T3) impl<T1, T2, T3, CTX> HashStable<CTX> for (T1, T2, T3)
where where
@ -355,7 +369,10 @@ where
} }
} }
unsafe impl<T1: StableOrd, T2: StableOrd, T3: StableOrd> StableOrd for (T1, T2, T3) {} unsafe impl<T1: StableOrd, T2: StableOrd, T3: StableOrd> StableOrd for (T1, T2, T3) {
const CAN_USE_UNSTABLE_SORT: bool =
T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT && T3::CAN_USE_UNSTABLE_SORT;
}
impl<T1, T2, T3, T4, CTX> HashStable<CTX> for (T1, T2, T3, T4) impl<T1, T2, T3, T4, CTX> HashStable<CTX> for (T1, T2, T3, T4)
where where
@ -376,6 +393,10 @@ where
unsafe impl<T1: StableOrd, T2: StableOrd, T3: StableOrd, T4: StableOrd> StableOrd unsafe impl<T1: StableOrd, T2: StableOrd, T3: StableOrd, T4: StableOrd> StableOrd
for (T1, T2, T3, T4) for (T1, T2, T3, T4)
{ {
const CAN_USE_UNSTABLE_SORT: bool = T1::CAN_USE_UNSTABLE_SORT
&& T2::CAN_USE_UNSTABLE_SORT
&& T3::CAN_USE_UNSTABLE_SORT
&& T4::CAN_USE_UNSTABLE_SORT;
} }
impl<T: HashStable<CTX>, CTX> HashStable<CTX> for [T] { impl<T: HashStable<CTX>, CTX> HashStable<CTX> for [T] {
@ -468,7 +489,9 @@ impl<CTX> HashStable<CTX> for str {
} }
} }
unsafe impl StableOrd for &str {} unsafe impl StableOrd for &str {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
impl<CTX> HashStable<CTX> for String { impl<CTX> HashStable<CTX> for String {
#[inline] #[inline]
@ -479,7 +502,9 @@ impl<CTX> HashStable<CTX> for String {
// Safety: String comparison only depends on their contents and the // Safety: String comparison only depends on their contents and the
// contents are not changed by (de-)serialization. // contents are not changed by (de-)serialization.
unsafe impl StableOrd for String {} unsafe impl StableOrd for String {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
impl<HCX> ToStableHashKey<HCX> for String { impl<HCX> ToStableHashKey<HCX> for String {
type KeyType = String; type KeyType = String;
@ -505,7 +530,9 @@ impl<CTX> HashStable<CTX> for bool {
} }
// Safety: sort order of bools is not changed by (de-)serialization. // Safety: sort order of bools is not changed by (de-)serialization.
unsafe impl StableOrd for bool {} unsafe impl StableOrd for bool {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
impl<T, CTX> HashStable<CTX> for Option<T> impl<T, CTX> HashStable<CTX> for Option<T>
where where
@ -523,7 +550,9 @@ where
} }
// Safety: the Option wrapper does not add instability to comparison. // Safety: the Option wrapper does not add instability to comparison.
unsafe impl<T: StableOrd> StableOrd for Option<T> {} unsafe impl<T: StableOrd> StableOrd for Option<T> {
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
}
impl<T1, T2, CTX> HashStable<CTX> for Result<T1, T2> impl<T1, T2, CTX> HashStable<CTX> for Result<T1, T2>
where where

View file

@ -140,12 +140,12 @@ impl<T: Ord, I: Iterator<Item = T>> UnordItems<T, I> {
} }
#[inline] #[inline]
pub fn into_sorted_stable_ord(self, use_stable_sort: bool) -> Vec<T> pub fn into_sorted_stable_ord(self) -> Vec<T>
where where
T: Ord + StableOrd, T: Ord + StableOrd,
{ {
let mut items: Vec<T> = self.0.collect(); let mut items: Vec<T> = self.0.collect();
if use_stable_sort { if !T::CAN_USE_UNSTABLE_SORT {
items.sort(); items.sort();
} else { } else {
items.sort_unstable() items.sort_unstable()
@ -161,6 +161,10 @@ impl<T: Ord, I: Iterator<Item = T>> UnordItems<T, I> {
items.sort_by_cached_key(|x| x.to_stable_hash_key(hcx)); items.sort_by_cached_key(|x| x.to_stable_hash_key(hcx));
items items
} }
pub fn collect<C: From<UnordItems<T, I>>>(self) -> C {
self.into()
}
} }
/// This is a set collection type that tries very hard to not expose /// This is a set collection type that tries very hard to not expose

View file

@ -166,7 +166,9 @@ impl ItemLocalId {
// Safety: Ord is implement as just comparing the ItemLocalId's numerical // Safety: Ord is implement as just comparing the ItemLocalId's numerical
// values and these are not changed by (de-)serialization. // values and these are not changed by (de-)serialization.
unsafe impl StableOrd for ItemLocalId {} unsafe impl StableOrd for ItemLocalId {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
/// The `HirId` corresponding to `CRATE_NODE_ID` and `CRATE_DEF_ID`. /// The `HirId` corresponding to `CRATE_NODE_ID` and `CRATE_DEF_ID`.
pub const CRATE_HIR_ID: HirId = pub const CRATE_HIR_ID: HirId =

View file

@ -119,7 +119,7 @@ impl<'tcx> AssertModuleSource<'tcx> {
if !self.available_cgus.contains(&cgu_name) { if !self.available_cgus.contains(&cgu_name) {
let cgu_names: Vec<&str> = let cgu_names: Vec<&str> =
self.available_cgus.items().map(|cgu| cgu.as_str()).into_sorted_stable_ord(true); self.available_cgus.items().map(|cgu| cgu.as_str()).into_sorted_stable_ord();
self.tcx.sess.emit_err(errors::NoModuleNamed { self.tcx.sess.emit_err(errors::NoModuleNamed {
span: attr.span, span: attr.span,
user_path, user_path,

View file

@ -198,7 +198,7 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
let (name, mut auto) = self.auto_labels(item_id, attr); let (name, mut auto) = self.auto_labels(item_id, attr);
let except = self.except(attr); let except = self.except(attr);
let loaded_from_disk = self.loaded_from_disk(attr); let loaded_from_disk = self.loaded_from_disk(attr);
for e in except.items().map(|x| x.as_str()).into_sorted_stable_ord(false) { for e in except.items().map(|x| x.as_str()).into_sorted_stable_ord() {
if !auto.remove(e) { if !auto.remove(e) {
self.tcx.sess.emit_fatal(errors::AssertionAuto { span: attr.span, name, e }); self.tcx.sess.emit_fatal(errors::AssertionAuto { span: attr.span, name, e });
} }
@ -377,16 +377,16 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
continue; continue;
}; };
self.checked_attrs.insert(attr.id); self.checked_attrs.insert(attr.id);
for label in assertion.clean.items().map(|x| x.as_str()).into_sorted_stable_ord(false) { for label in assertion.clean.items().map(|x| x.as_str()).into_sorted_stable_ord() {
let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap(); let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap();
self.assert_clean(item_span, dep_node); self.assert_clean(item_span, dep_node);
} }
for label in assertion.dirty.items().map(|x| x.as_str()).into_sorted_stable_ord(false) { for label in assertion.dirty.items().map(|x| x.as_str()).into_sorted_stable_ord() {
let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap(); let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap();
self.assert_dirty(item_span, dep_node); self.assert_dirty(item_span, dep_node);
} }
for label in for label in
assertion.loaded_from_disk.items().map(|x| x.as_str()).into_sorted_stable_ord(false) assertion.loaded_from_disk.items().map(|x| x.as_str()).into_sorted_stable_ord()
{ {
let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap(); let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap();
self.assert_loaded_from_disk(item_span, dep_node); self.assert_loaded_from_disk(item_span, dep_node);

View file

@ -676,11 +676,8 @@ pub fn garbage_collect_session_directories(sess: &Session) -> io::Result<()> {
// Delete all lock files, that don't have an associated directory. They must // Delete all lock files, that don't have an associated directory. They must
// be some kind of leftover // be some kind of leftover
let lock_file_to_session_dir_iter = lock_file_to_session_dir
.items()
.map(|(file, dir)| (file.as_str(), dir.as_ref().map(|y| y.as_str())));
for (lock_file_name, directory_name) in for (lock_file_name, directory_name) in
lock_file_to_session_dir_iter.into_sorted_stable_ord(false) lock_file_to_session_dir.items().into_sorted_stable_ord()
{ {
if directory_name.is_none() { if directory_name.is_none() {
let Ok(timestamp) = extract_timestamp_from_session_dir(lock_file_name) else { let Ok(timestamp) = extract_timestamp_from_session_dir(lock_file_name) else {
@ -712,10 +709,10 @@ pub fn garbage_collect_session_directories(sess: &Session) -> io::Result<()> {
} }
// Filter out `None` directories // Filter out `None` directories
let lock_file_to_session_dir: UnordMap<String, String> = let lock_file_to_session_dir: UnordMap<String, String> = lock_file_to_session_dir
UnordMap::from(lock_file_to_session_dir.into_items().filter_map( .into_items()
|(lock_file_name, directory_name)| directory_name.map(|n| (lock_file_name, n)), .filter_map(|(lock_file_name, directory_name)| directory_name.map(|n| (lock_file_name, n)))
)); .into();
// Delete all session directories that don't have a lock file. // Delete all session directories that don't have a lock file.
for directory_name in session_directories { for directory_name in session_directories {
@ -821,7 +818,7 @@ pub fn garbage_collect_session_directories(sess: &Session) -> io::Result<()> {
} }
None None
}); });
let deletion_candidates = UnordMap::from(deletion_candidates); let deletion_candidates = deletion_candidates.into();
// Delete all but the most recent of the candidates // Delete all but the most recent of the candidates
all_except_most_recent(deletion_candidates).into_items().all(|(path, lock)| { all_except_most_recent(deletion_candidates).into_items().all(|(path, lock)| {

View file

@ -2,23 +2,16 @@ use super::*;
#[test] #[test]
fn test_all_except_most_recent() { fn test_all_except_most_recent() {
let computed: UnordMap<_, Option<flock::Lock>> = UnordMap::from_iter([ let input: UnordMap<_, Option<flock::Lock>> = UnordMap::from_iter([
((UNIX_EPOCH + Duration::new(4, 0), PathBuf::from("4")), None), ((UNIX_EPOCH + Duration::new(4, 0), PathBuf::from("4")), None),
((UNIX_EPOCH + Duration::new(1, 0), PathBuf::from("1")), None), ((UNIX_EPOCH + Duration::new(1, 0), PathBuf::from("1")), None),
((UNIX_EPOCH + Duration::new(5, 0), PathBuf::from("5")), None), ((UNIX_EPOCH + Duration::new(5, 0), PathBuf::from("5")), None),
((UNIX_EPOCH + Duration::new(3, 0), PathBuf::from("3")), None), ((UNIX_EPOCH + Duration::new(3, 0), PathBuf::from("3")), None),
((UNIX_EPOCH + Duration::new(2, 0), PathBuf::from("2")), None), ((UNIX_EPOCH + Duration::new(2, 0), PathBuf::from("2")), None),
]); ]);
let mut paths = UnordSet::default();
paths.extend_unord(all_except_most_recent(computed).into_items().map(|(path, _)| path));
assert_eq!( assert_eq!(
UnordSet::from(paths), all_except_most_recent(input).into_items().map(|(path, _)| path).into_sorted_stable_ord(),
UnordSet::from_iter([ vec![PathBuf::from("1"), PathBuf::from("2"), PathBuf::from("3"), PathBuf::from("4")]
PathBuf::from("1"),
PathBuf::from("2"),
PathBuf::from("3"),
PathBuf::from("4")
])
); );
assert!(all_except_most_recent(UnordMap::default()).is_empty()); assert!(all_except_most_recent(UnordMap::default()).is_empty());

View file

@ -46,12 +46,7 @@ pub fn copy_cgu_workproduct_to_incr_comp_cache_dir(
/// Removes files for a given work product. /// Removes files for a given work product.
pub fn delete_workproduct_files(sess: &Session, work_product: &WorkProduct) { pub fn delete_workproduct_files(sess: &Session, work_product: &WorkProduct) {
for path in work_product for (_, path) in work_product.saved_files.items().into_sorted_stable_ord() {
.saved_files
.items()
.map(|(_, path)| path.as_str())
.into_sorted_stable_ord(false)
{
let path = in_incr_comp_dir_sess(sess, path); let path = in_incr_comp_dir_sess(sess, path);
if let Err(err) = std_fs::remove_file(&path) { if let Err(err) = std_fs::remove_file(&path) {
sess.emit_warning(errors::DeleteWorkProduct { path: &path, err }); sess.emit_warning(errors::DeleteWorkProduct { path: &path, err });

View file

@ -194,10 +194,15 @@ impl<'tcx> Queries<'tcx> {
let future_opt = self.dep_graph_future()?.steal(); let future_opt = self.dep_graph_future()?.steal();
let dep_graph = future_opt let dep_graph = future_opt
.and_then(|future| { .and_then(|future| {
let (prev_graph, prev_work_products) = let (prev_graph, mut prev_work_products) =
sess.time("blocked_on_dep_graph_loading", || future.open().open(sess)); sess.time("blocked_on_dep_graph_loading", || future.open().open(sess));
let prev_work_products = // Convert from UnordMap to FxIndexMap by sorting
FxIndexMap::from_iter(prev_work_products.into_sorted(&(), false)); let prev_work_product_ids =
prev_work_products.items().map(|x| *x.0).into_sorted_stable_ord();
let prev_work_products = prev_work_product_ids
.into_iter()
.map(|x| (x, prev_work_products.remove(&x).unwrap()))
.collect::<FxIndexMap<_, _>>();
rustc_incremental::build_dep_graph(sess, prev_graph, prev_work_products) rustc_incremental::build_dep_graph(sess, prev_graph, prev_work_products)
}) })
.unwrap_or_else(DepGraph::new_disabled); .unwrap_or_else(DepGraph::new_disabled);

View file

@ -46,7 +46,7 @@ use super::{DepContext, DepKind, FingerprintStyle};
use crate::ich::StableHashingContext; use crate::ich::StableHashingContext;
use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint}; use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableOrd, ToStableHashKey};
use rustc_hir::definitions::DefPathHash; use rustc_hir::definitions::DefPathHash;
use std::fmt; use std::fmt;
use std::hash::Hash; use std::hash::Hash;
@ -254,3 +254,7 @@ impl<HCX> ToStableHashKey<HCX> for WorkProductId {
self.hash self.hash
} }
} }
unsafe impl StableOrd for WorkProductId {
// Fingerprint can use unstable (just a tuple of `u64`s), so WorkProductId can as well
const CAN_USE_UNSTABLE_SORT: bool = true;
}

View file

@ -311,7 +311,9 @@ pub enum OutputType {
} }
// Safety: Trivial C-Style enums have a stable sort order across compilation sessions. // Safety: Trivial C-Style enums have a stable sort order across compilation sessions.
unsafe impl StableOrd for OutputType {} unsafe impl StableOrd for OutputType {
const CAN_USE_UNSTABLE_SORT: bool = true;
}
impl<HCX: HashStableContext> ToStableHashKey<HCX> for OutputType { impl<HCX: HashStableContext> ToStableHashKey<HCX> for OutputType {
type KeyType = Self; type KeyType = Self;

View file

@ -160,7 +160,7 @@ impl LateLintPass<'_> for WildcardImports {
) )
}; };
let mut imports = used_imports.items().map(ToString::to_string).into_sorted_stable_ord(false); let mut imports = used_imports.items().map(ToString::to_string).into_sorted_stable_ord();
let imports_string = if imports.len() == 1 { let imports_string = if imports.len() == 1 {
imports.pop().unwrap() imports.pop().unwrap()
} else if braced_glob { } else if braced_glob {