Skip to content

Commit 0e73e70

Browse files
eggyaleggyal
committed
Ensure careful consideration is given by impls
Added an associated `const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED` to the `StableOrd` trait to ensure that implementors carefully consider whether the trait's contract is upheld, as incorrect implementations can cause miscompilations.
1 parent 114dd20 commit 0e73e70

File tree

6 files changed

+60
-13
lines changed

6 files changed

+60
-13
lines changed

‎compiler/rustc_abi/src/lib.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -425,11 +425,13 @@ pub struct Size {
425425
raw: u64,
426426
}
427427

428-
// Ord is implement as just comparing numerical values and numerical values
429-
// are not changed by (de-)serialization.
430428
#[cfg(feature = "nightly")]
431429
impl StableOrd for Size {
432430
const CAN_USE_UNSTABLE_SORT: bool = true;
431+
432+
// `Ord` is implemented as just comparing numerical values and numerical values
433+
// are not changed by (de-)serialization.
434+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
433435
}
434436

435437
// This is debug-printed a lot in larger structs, don't waste too much space there

‎compiler/rustc_data_structures/src/stable_hasher.rs

+43-7
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,21 @@ pub trait ToStableHashKey<HCX> {
238238
/// The associated constant `CAN_USE_UNSTABLE_SORT` denotes whether
239239
/// unstable sorting can be used for this type. Set to true if and
240240
/// only if `a == b` implies `a` and `b` are fully indistinguishable.
241-
///
242-
/// **Be careful when implementing this trait, as an incorrect
243-
/// implementation can cause miscompilation!**
244241
pub trait StableOrd: Ord {
245242
const CAN_USE_UNSTABLE_SORT: bool;
243+
244+
/// Marker to ensure that implementors have carefully considered
245+
/// whether their `Ord` implementation obeys this trait's contract.
246+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: ();
246247
}
247248

248249
impl<T: StableOrd> StableOrd for &T {
249250
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
251+
252+
// Ordering of a reference is exactly that of the referent, and since
253+
// the ordering of the referet is stable so must be the ordering of the
254+
// reference.
255+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
250256
}
251257

252258
/// This is a companion trait to `StableOrd`. Some types like `Symbol` can be
@@ -295,6 +301,10 @@ macro_rules! impl_stable_traits_for_trivial_type {
295301

296302
impl $crate::stable_hasher::StableOrd for $t {
297303
const CAN_USE_UNSTABLE_SORT: bool = true;
304+
305+
// Encoding and decoding doesn't change the bytes of trivial types
306+
// and `Ord::cmp` depends only on those bytes.
307+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
298308
}
299309
};
300310
}
@@ -332,6 +342,10 @@ impl<CTX> HashStable<CTX> for Hash128 {
332342

333343
impl StableOrd for Hash128 {
334344
const CAN_USE_UNSTABLE_SORT: bool = true;
345+
346+
// Encoding and decoding doesn't change the bytes of `Hash128`
347+
// and `Ord::cmp` depends only on those bytes.
348+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
335349
}
336350

337351
impl<CTX> HashStable<CTX> for ! {
@@ -397,6 +411,10 @@ impl<T1: HashStable<CTX>, T2: HashStable<CTX>, CTX> HashStable<CTX> for (T1, T2)
397411

398412
impl<T1: StableOrd, T2: StableOrd> StableOrd for (T1, T2) {
399413
const CAN_USE_UNSTABLE_SORT: bool = T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT;
414+
415+
// Ordering of tuples is a pure function of their elements' ordering, and since
416+
// the ordering of each element is stable so must be the ordering of the tuple.
417+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
400418
}
401419

402420
impl<T1, T2, T3, CTX> HashStable<CTX> for (T1, T2, T3)
@@ -416,6 +434,10 @@ where
416434
impl<T1: StableOrd, T2: StableOrd, T3: StableOrd> StableOrd for (T1, T2, T3) {
417435
const CAN_USE_UNSTABLE_SORT: bool =
418436
T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT && T3::CAN_USE_UNSTABLE_SORT;
437+
438+
// Ordering of tuples is a pure function of their elements' ordering, and since
439+
// the ordering of each element is stable so must be the ordering of the tuple.
440+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
419441
}
420442

421443
impl<T1, T2, T3, T4, CTX> HashStable<CTX> for (T1, T2, T3, T4)
@@ -439,6 +461,10 @@ impl<T1: StableOrd, T2: StableOrd, T3: StableOrd, T4: StableOrd> StableOrd for (
439461
&& T2::CAN_USE_UNSTABLE_SORT
440462
&& T3::CAN_USE_UNSTABLE_SORT
441463
&& T4::CAN_USE_UNSTABLE_SORT;
464+
465+
// Ordering of tuples is a pure function of their elements' ordering, and since
466+
// the ordering of each element is stable so must be the ordering of the tuple.
467+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
442468
}
443469

444470
impl<T: HashStable<CTX>, CTX> HashStable<CTX> for [T] {
@@ -533,6 +559,10 @@ impl<CTX> HashStable<CTX> for str {
533559

534560
impl StableOrd for &str {
535561
const CAN_USE_UNSTABLE_SORT: bool = true;
562+
563+
// Encoding and decoding doesn't change the bytes of string slices
564+
// and `Ord::cmp` depends only on those bytes.
565+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
536566
}
537567

538568
impl<CTX> HashStable<CTX> for String {
@@ -542,10 +572,12 @@ impl<CTX> HashStable<CTX> for String {
542572
}
543573
}
544574

545-
// String comparison only depends on their contents and the
546-
// contents are not changed by (de-)serialization.
547575
impl StableOrd for String {
548576
const CAN_USE_UNSTABLE_SORT: bool = true;
577+
578+
// String comparison only depends on their contents and the
579+
// contents are not changed by (de-)serialization.
580+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
549581
}
550582

551583
impl<HCX> ToStableHashKey<HCX> for String {
@@ -571,9 +603,11 @@ impl<CTX> HashStable<CTX> for bool {
571603
}
572604
}
573605

574-
// sort order of bools is not changed by (de-)serialization.
575606
impl StableOrd for bool {
576607
const CAN_USE_UNSTABLE_SORT: bool = true;
608+
609+
// sort order of bools is not changed by (de-)serialization.
610+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
577611
}
578612

579613
impl<T, CTX> HashStable<CTX> for Option<T>
@@ -591,9 +625,11 @@ where
591625
}
592626
}
593627

594-
// the Option wrapper does not add instability to comparison.
595628
impl<T: StableOrd> StableOrd for Option<T> {
596629
const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
630+
631+
// the Option wrapper does not add instability to comparison.
632+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
597633
}
598634

599635
impl<T1, T2, CTX> HashStable<CTX> for Result<T1, T2>

‎compiler/rustc_hir/src/hir_id.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,12 @@ impl ItemLocalId {
165165
pub const INVALID: ItemLocalId = ItemLocalId::MAX;
166166
}
167167

168-
// Ord is implement as just comparing the ItemLocalId's numerical
169-
// values and these are not changed by (de-)serialization.
170168
impl StableOrd for ItemLocalId {
171169
const CAN_USE_UNSTABLE_SORT: bool = true;
170+
171+
// `Ord` is implemented as just comparing the ItemLocalId's numerical
172+
// values and these are not changed by (de-)serialization.
173+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
172174
}
173175

174176
/// The `HirId` corresponding to `CRATE_NODE_ID` and `CRATE_DEF_ID`.

‎compiler/rustc_query_system/src/dep_graph/dep_node.rs

+3
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,9 @@ impl<HCX> ToStableHashKey<HCX> for WorkProductId {
304304
impl StableOrd for WorkProductId {
305305
// Fingerprint can use unstable (just a tuple of `u64`s), so WorkProductId can as well
306306
const CAN_USE_UNSTABLE_SORT: bool = true;
307+
308+
// `WorkProductId` sort order is not affected by (de)serialization.
309+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
307310
}
308311

309312
// Some types are used a lot. Make sure they don't unintentionally get bigger.

‎compiler/rustc_session/src/config.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -491,9 +491,11 @@ pub enum OutputType {
491491
DepInfo,
492492
}
493493

494-
// Trivial C-Style enums have a stable sort order across compilation sessions.
495494
impl StableOrd for OutputType {
496495
const CAN_USE_UNSTABLE_SORT: bool = true;
496+
497+
// Trivial C-Style enums have a stable sort order across compilation sessions.
498+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
497499
}
498500

499501
impl<HCX: HashStableContext> ToStableHashKey<HCX> for OutputType {

‎compiler/rustc_span/src/def_id.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,11 @@ impl Default for DefPathHash {
120120
}
121121
}
122122

123-
// `DefPathHash` sort order is not affected (de)serialization.
124123
impl StableOrd for DefPathHash {
125124
const CAN_USE_UNSTABLE_SORT: bool = true;
125+
126+
// `DefPathHash` sort order is not affected by (de)serialization.
127+
const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
126128
}
127129

128130
/// A [`StableCrateId`] is a 64-bit hash of a crate name, together with all

0 commit comments

Comments
 (0)