about summary refs log tree commit diff
path: root/compiler/rustc_data_structures/src
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-06-25 15:51:35 +0000
committerbors <bors@rust-lang.org>2024-06-25 15:51:35 +0000
commitc290e9de32e8ba6a673ef125fde40eadd395d170 (patch)
tree9d40c19ec778317c30ae2770bed4f993373ce60d /compiler/rustc_data_structures/src
parentd929a42a664c026167800801b26d734db925314f (diff)
parent0e73e7095ae7aa7ead69c4ba7ba002560ef118fa (diff)
downloadrust-c290e9de32e8ba6a673ef125fde40eadd395d170.tar.gz
rust-c290e9de32e8ba6a673ef125fde40eadd395d170.zip
Auto merge of #126326 - eggyal:ununsafe-StableOrd, r=michaelwoerister
Un-unsafe the `StableOrd` trait

Whilst incorrect implementations of this trait can cause miscompilation, they cannot cause memory unsafety in rustc.

[Discussed on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Policy.20of.20.60unsafe.60.20within.20the.20compiler).

cc [MCP 533](https://github.com/rust-lang/compiler-team/issues/533), #105175, `@michaelwoerister`

r? `@Nilstrieb`
Diffstat (limited to 'compiler/rustc_data_structures/src')
-rw-r--r--compiler/rustc_data_structures/src/stable_hasher.rs71
1 files changed, 54 insertions, 17 deletions
diff --git a/compiler/rustc_data_structures/src/stable_hasher.rs b/compiler/rustc_data_structures/src/stable_hasher.rs
index b5bdf2e1790..a57f5067dd8 100644
--- a/compiler/rustc_data_structures/src/stable_hasher.rs
+++ b/compiler/rustc_data_structures/src/stable_hasher.rs
@@ -238,12 +238,21 @@ pub trait ToStableHashKey<HCX> {
 /// 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 {
+pub trait StableOrd: Ord {
     const CAN_USE_UNSTABLE_SORT: bool;
+
+    /// Marker to ensure that implementors have carefully considered
+    /// whether their `Ord` implementation obeys this trait's contract.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: ();
 }
 
-unsafe impl<T: StableOrd> StableOrd for &T {
+impl<T: StableOrd> StableOrd for &T {
     const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
+
+    // Ordering of a reference is exactly that of the referent, and since
+    // the ordering of the referet is stable so must be the ordering of the
+    // reference.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 /// This is a companion trait to `StableOrd`. Some types like `Symbol` can be
@@ -290,8 +299,12 @@ macro_rules! impl_stable_traits_for_trivial_type {
             }
         }
 
-        unsafe impl $crate::stable_hasher::StableOrd for $t {
+        impl $crate::stable_hasher::StableOrd for $t {
             const CAN_USE_UNSTABLE_SORT: bool = true;
+
+            // Encoding and decoding doesn't change the bytes of trivial types
+            // and `Ord::cmp` depends only on those bytes.
+            const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
         }
     };
 }
@@ -327,8 +340,12 @@ impl<CTX> HashStable<CTX> for Hash128 {
     }
 }
 
-unsafe impl StableOrd for Hash128 {
+impl StableOrd for Hash128 {
     const CAN_USE_UNSTABLE_SORT: bool = true;
+
+    // Encoding and decoding doesn't change the bytes of `Hash128`
+    // and `Ord::cmp` depends only on those bytes.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<CTX> HashStable<CTX> for ! {
@@ -392,8 +409,12 @@ impl<T1: HashStable<CTX>, T2: HashStable<CTX>, CTX> HashStable<CTX> for (T1, T2)
     }
 }
 
-unsafe impl<T1: StableOrd, T2: StableOrd> StableOrd for (T1, T2) {
+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;
+
+    // Ordering of tuples is a pure function of their elements' ordering, and since
+    // the ordering of each element is stable so must be the ordering of the tuple.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<T1, T2, T3, CTX> HashStable<CTX> for (T1, T2, T3)
@@ -410,9 +431,13 @@ where
     }
 }
 
-unsafe impl<T1: StableOrd, T2: StableOrd, T3: StableOrd> StableOrd for (T1, T2, T3) {
+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;
+
+    // Ordering of tuples is a pure function of their elements' ordering, and since
+    // the ordering of each element is stable so must be the ordering of the tuple.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<T1, T2, T3, T4, CTX> HashStable<CTX> for (T1, T2, T3, T4)
@@ -431,13 +456,15 @@ where
     }
 }
 
-unsafe impl<T1: StableOrd, T2: StableOrd, T3: StableOrd, T4: StableOrd> StableOrd
-    for (T1, T2, T3, T4)
-{
+impl<T1: StableOrd, T2: StableOrd, T3: StableOrd, T4: StableOrd> StableOrd 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;
+
+    // Ordering of tuples is a pure function of their elements' ordering, and since
+    // the ordering of each element is stable so must be the ordering of the tuple.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<T: HashStable<CTX>, CTX> HashStable<CTX> for [T] {
@@ -530,8 +557,12 @@ impl<CTX> HashStable<CTX> for str {
     }
 }
 
-unsafe impl StableOrd for &str {
+impl StableOrd for &str {
     const CAN_USE_UNSTABLE_SORT: bool = true;
+
+    // Encoding and decoding doesn't change the bytes of string slices
+    // and `Ord::cmp` depends only on those bytes.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<CTX> HashStable<CTX> for String {
@@ -541,10 +572,12 @@ impl<CTX> HashStable<CTX> for String {
     }
 }
 
-// Safety: String comparison only depends on their contents and the
-// contents are not changed by (de-)serialization.
-unsafe impl StableOrd for String {
+impl StableOrd for String {
     const CAN_USE_UNSTABLE_SORT: bool = true;
+
+    // String comparison only depends on their contents and the
+    // contents are not changed by (de-)serialization.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<HCX> ToStableHashKey<HCX> for String {
@@ -570,9 +603,11 @@ impl<CTX> HashStable<CTX> for bool {
     }
 }
 
-// Safety: sort order of bools is not changed by (de-)serialization.
-unsafe impl StableOrd for bool {
+impl StableOrd for bool {
     const CAN_USE_UNSTABLE_SORT: bool = true;
+
+    // sort order of bools is not changed by (de-)serialization.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<T, CTX> HashStable<CTX> for Option<T>
@@ -590,9 +625,11 @@ where
     }
 }
 
-// Safety: the Option wrapper does not add instability to comparison.
-unsafe impl<T: StableOrd> StableOrd for Option<T> {
+impl<T: StableOrd> StableOrd for Option<T> {
     const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT;
+
+    // the Option wrapper does not add instability to comparison.
+    const THIS_IMPLEMENTATION_HAS_BEEN_TRIPLE_CHECKED: () = ();
 }
 
 impl<T1, T2, CTX> HashStable<CTX> for Result<T1, T2>