Skip to content

Commit 739e5ef

Browse files
Split StableCompare trait out of StableOrd trait.
StableCompare is a companion trait to `StableOrd`. Some types like `Symbol` can be compared in a cross-session stable way, but their `Ord` implementation is not stable. In such cases, a `StableOrd` implementation can be provided to offer a lightweight way for stable sorting. (The more heavyweight option is to sort via `ToStableHashKey`, but then sorting needs to have access to a stable hashing context and `ToStableHashKey` can also be expensive as in the case of `Symbol` where it has to allocate a `String`.)
1 parent 090d5ea commit 739e5ef

File tree

9 files changed

+118
-22
lines changed

9 files changed

+118
-22
lines changed

compiler/rustc_data_structures/src/stable_hasher.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,32 @@ unsafe impl<T: StableOrd> StableOrd for &T {
245245
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
246246
}
247247

248+
/// This is a companion trait to `StableOrd`. Some types like `Symbol` can be
249+
/// compared in a cross-session stable way, but their `Ord` implementation is
250+
/// not stable. In such cases, a `StableOrd` implementation can be provided
251+
/// to offer a lightweight way for stable sorting. (The more heavyweight option
252+
/// is to sort via `ToStableHashKey`, but then sorting needs to have access to
253+
/// a stable hashing context and `ToStableHashKey` can also be expensive as in
254+
/// the case of `Symbol` where it has to allocate a `String`.)
255+
///
256+
/// See the documentation of [StableOrd] for how stable sort order is defined.
257+
/// The same definition applies here. Be careful when implementing this trait.
258+
pub trait StableCompare {
259+
const CAN_USE_UNSTABLE_SORT: bool;
260+
261+
fn stable_cmp(&self, other: &Self) -> std::cmp::Ordering;
262+
}
263+
264+
/// `StableOrd` denotes that the type's `Ord` implementation is stable, so
265+
/// we can implement `StableCompare` by just delegating to `Ord`.
266+
impl<T: StableOrd> StableCompare for T {
267+
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
268+
269+
fn stable_cmp(&self, other: &Self) -> std::cmp::Ordering {
270+
self.cmp(other)
271+
}
272+
}
273+
248274
/// Implement HashStable by just calling `Hash::hash()`. Also implement `StableOrd` for the type since
249275
/// that has the same requirements.
250276
///

compiler/rustc_data_structures/src/unord.rs

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use std::{
1414

1515
use crate::{
1616
fingerprint::Fingerprint,
17-
stable_hasher::{HashStable, StableHasher, StableOrd, ToStableHashKey},
17+
stable_hasher::{HashStable, StableCompare, StableHasher, ToStableHashKey},
1818
};
1919

2020
/// `UnordItems` is the order-less version of `Iterator`. It only contains methods
@@ -134,7 +134,7 @@ impl<'a, T: Copy + 'a, I: Iterator<Item = &'a T>> UnordItems<&'a T, I> {
134134
}
135135
}
136136

137-
impl<T: Ord, I: Iterator<Item = T>> UnordItems<T, I> {
137+
impl<T, I: Iterator<Item = T>> UnordItems<T, I> {
138138
pub fn into_sorted<HCX>(self, hcx: &HCX) -> Vec<T>
139139
where
140140
T: ToStableHashKey<HCX>,
@@ -147,13 +147,36 @@ impl<T: Ord, I: Iterator<Item = T>> UnordItems<T, I> {
147147
#[inline]
148148
pub fn into_sorted_stable_ord(self) -> Vec<T>
149149
where
150-
T: Ord + StableOrd,
150+
T: StableCompare,
151151
{
152152
let mut items: Vec<T> = self.0.collect();
153153
if !T::CAN_USE_UNSTABLE_SORT {
154-
items.sort();
154+
items.sort_by(T::stable_cmp);
155+
} else {
156+
items.sort_unstable_by(T::stable_cmp)
157+
}
158+
items
159+
}
160+
161+
#[inline]
162+
pub fn into_sorted_stable_ord_by_key<K, C>(self, project_to_key: C) -> Vec<T>
163+
where
164+
K: StableCompare,
165+
C: for<'a> Fn(&'a T) -> &'a K,
166+
{
167+
let mut items: Vec<T> = self.0.collect();
168+
if !K::CAN_USE_UNSTABLE_SORT {
169+
items.sort_by(|a, b| {
170+
let a_key = project_to_key(a);
171+
let b_key = project_to_key(b);
172+
a_key.stable_cmp(b_key)
173+
});
155174
} else {
156-
items.sort_unstable()
175+
items.sort_unstable_by(|a, b| {
176+
let a_key = project_to_key(a);
177+
let b_key = project_to_key(b);
178+
a_key.stable_cmp(b_key)
179+
});
157180
}
158181
items
159182
}
@@ -268,16 +291,30 @@ impl<V: Eq + Hash> UnordSet<V> {
268291
}
269292

270293
/// Returns the items of this set in stable sort order (as defined by
271-
/// `StableOrd`). This method is much more efficient than
294+
/// `StableCompare`). This method is much more efficient than
272295
/// `into_sorted` because it does not need to transform keys to their
273296
/// `ToStableHashKey` equivalent.
274297
#[inline]
275-
pub fn to_sorted_stable_ord(&self) -> Vec<V>
298+
pub fn to_sorted_stable_ord(&self) -> Vec<&V>
276299
where
277-
V: Ord + StableOrd + Clone,
300+
V: StableCompare,
278301
{
279-
let mut items: Vec<V> = self.inner.iter().cloned().collect();
280-
items.sort_unstable();
302+
let mut items: Vec<&V> = self.inner.iter().collect();
303+
items.sort_unstable_by(|a, b| a.stable_cmp(*b));
304+
items
305+
}
306+
307+
/// Returns the items of this set in stable sort order (as defined by
308+
/// `StableCompare`). This method is much more efficient than
309+
/// `into_sorted` because it does not need to transform keys to their
310+
/// `ToStableHashKey` equivalent.
311+
#[inline]
312+
pub fn into_sorted_stable_ord(self) -> Vec<V>
313+
where
314+
V: StableCompare,
315+
{
316+
let mut items: Vec<V> = self.inner.into_iter().collect();
317+
items.sort_unstable_by(V::stable_cmp);
281318
items
282319
}
283320

@@ -483,16 +520,16 @@ impl<K: Eq + Hash, V> UnordMap<K, V> {
483520
to_sorted_vec(hcx, self.inner.iter(), cache_sort_key, |&(k, _)| k)
484521
}
485522

486-
/// Returns the entries of this map in stable sort order (as defined by `StableOrd`).
523+
/// Returns the entries of this map in stable sort order (as defined by `StableCompare`).
487524
/// This method can be much more efficient than `into_sorted` because it does not need
488525
/// to transform keys to their `ToStableHashKey` equivalent.
489526
#[inline]
490-
pub fn to_sorted_stable_ord(&self) -> Vec<(K, &V)>
527+
pub fn to_sorted_stable_ord(&self) -> Vec<(&K, &V)>
491528
where
492-
K: Ord + StableOrd + Copy,
529+
K: StableCompare,
493530
{
494-
let mut items: Vec<(K, &V)> = self.inner.iter().map(|(&k, v)| (k, v)).collect();
495-
items.sort_unstable_by_key(|&(k, _)| k);
531+
let mut items: Vec<_> = self.inner.iter().collect();
532+
items.sort_unstable_by(|(a, _), (b, _)| a.stable_cmp(*b));
496533
items
497534
}
498535

@@ -510,6 +547,19 @@ impl<K: Eq + Hash, V> UnordMap<K, V> {
510547
to_sorted_vec(hcx, self.inner.into_iter(), cache_sort_key, |(k, _)| k)
511548
}
512549

550+
/// Returns the entries of this map in stable sort order (as defined by `StableCompare`).
551+
/// This method can be much more efficient than `into_sorted` because it does not need
552+
/// to transform keys to their `ToStableHashKey` equivalent.
553+
#[inline]
554+
pub fn into_sorted_stable_ord(self) -> Vec<(K, V)>
555+
where
556+
K: StableCompare,
557+
{
558+
let mut items: Vec<(K, V)> = self.inner.into_iter().collect();
559+
items.sort_unstable_by(|a, b| a.0.stable_cmp(&b.0));
560+
items
561+
}
562+
513563
/// Returns the values of this map in stable sort order (as defined by K's
514564
/// `ToStableHashKey` implementation).
515565
///

compiler/rustc_hir_typeck/src/method/suggest.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
913913
for ((span, add_where_or_comma), obligations) in type_params.into_iter() {
914914
restrict_type_params = true;
915915
// #74886: Sort here so that the output is always the same.
916-
let obligations = obligations.to_sorted_stable_ord();
916+
let obligations = obligations.into_sorted_stable_ord();
917917
err.span_suggestion_verbose(
918918
span,
919919
format!(

compiler/rustc_hir_typeck/src/upvar.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -912,7 +912,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
912912
drop_order: bool,
913913
) -> MigrationWarningReason {
914914
MigrationWarningReason {
915-
auto_traits: auto_trait_reasons.to_sorted_stable_ord(),
915+
auto_traits: auto_trait_reasons.into_sorted_stable_ord(),
916916
drop_order,
917917
}
918918
}

compiler/rustc_hir_typeck/src/writeback.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
473473
assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner);
474474

475475
let fcx_coercion_casts = fcx_typeck_results.coercion_casts().to_sorted_stable_ord();
476-
for local_id in fcx_coercion_casts {
476+
for &local_id in fcx_coercion_casts {
477477
self.typeck_results.set_coercion_cast(local_id);
478478
}
479479
}

compiler/rustc_incremental/src/persist/save.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ pub fn save_work_product_index(
103103
// deleted during invalidation. Some object files don't change their
104104
// content, they are just not needed anymore.
105105
let previous_work_products = dep_graph.previous_work_products();
106-
for (id, wp) in previous_work_products.to_sorted_stable_ord().iter() {
106+
for (id, wp) in previous_work_products.to_sorted_stable_ord() {
107107
if !new_work_products.contains_key(id) {
108108
work_product::delete_workproduct_files(sess, wp);
109109
debug_assert!(

compiler/rustc_lint_defs/src/lib.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ pub use self::Level::*;
99
use rustc_ast::node_id::NodeId;
1010
use rustc_ast::{AttrId, Attribute};
1111
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
12-
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
12+
use rustc_data_structures::stable_hasher::{
13+
HashStable, StableCompare, StableHasher, ToStableHashKey,
14+
};
1315
use rustc_error_messages::{DiagnosticMessage, MultiSpan};
1416
use rustc_hir::HashStableContext;
1517
use rustc_hir::HirId;
@@ -541,6 +543,14 @@ impl<HCX> ToStableHashKey<HCX> for LintId {
541543
}
542544
}
543545

546+
impl StableCompare for LintId {
547+
const CAN_USE_UNSTABLE_SORT: bool = true;
548+
549+
fn stable_cmp(&self, other: &Self) -> std::cmp::Ordering {
550+
self.lint_name_raw().cmp(&other.lint_name_raw())
551+
}
552+
}
553+
544554
#[derive(Debug)]
545555
pub struct AmbiguityErrorDiag {
546556
pub msg: String,

compiler/rustc_middle/src/ty/typeck_results.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ impl<'a, V> LocalTableInContext<'a, V> {
527527
}
528528

529529
pub fn items_in_stable_order(&self) -> Vec<(ItemLocalId, &'a V)> {
530-
self.data.to_sorted_stable_ord()
530+
self.data.items().map(|(&k, v)| (k, v)).into_sorted_stable_ord_by_key(|(k, _)| k)
531531
}
532532
}
533533

compiler/rustc_span/src/symbol.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
55
use rustc_arena::DroplessArena;
66
use rustc_data_structures::fx::FxIndexSet;
7-
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
7+
use rustc_data_structures::stable_hasher::{
8+
HashStable, StableCompare, StableHasher, ToStableHashKey,
9+
};
810
use rustc_data_structures::sync::Lock;
911
use rustc_macros::HashStable_Generic;
1012
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
@@ -2103,6 +2105,14 @@ impl<CTX> ToStableHashKey<CTX> for Symbol {
21032105
}
21042106
}
21052107

2108+
impl StableCompare for Symbol {
2109+
const CAN_USE_UNSTABLE_SORT: bool = true;
2110+
2111+
fn stable_cmp(&self, other: &Self) -> std::cmp::Ordering {
2112+
self.as_str().cmp(other.as_str())
2113+
}
2114+
}
2115+
21062116
pub(crate) struct Interner(Lock<InternerInner>);
21072117

21082118
// The `&'static str`s in this type actually point into the arena.

0 commit comments

Comments
 (0)